From 1f41f1002a9b334cf104c2913a992f594bf4c008 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Fri, 2 Sep 2016 14:01:17 +0000 Subject: [PATCH] clang-format: [JS] Sort all JavaScript imports if any changed. Summary: User feedback is that they expect *all* imports to be sorted if any import was affected by a change, not just imports up to the first non-affected line, as clang-format currently does. Reviewers: djasper Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D23972 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@280485 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/SortJavaScriptImports.cpp | 12 +++++------- unittests/Format/SortImportsTestJS.cpp | 25 +++++++------------------ 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/lib/Format/SortJavaScriptImports.cpp b/lib/Format/SortJavaScriptImports.cpp index e800007f79..72da52bf16 100644 --- a/lib/Format/SortJavaScriptImports.cpp +++ b/lib/Format/SortJavaScriptImports.cpp @@ -284,14 +284,8 @@ private: SourceLocation Start; bool FoundLines = false; AnnotatedLine *FirstNonImportLine = nullptr; + bool AnyImportAffected = false; for (auto Line : AnnotatedLines) { - if (!Line->Affected) { - // Only sort the first contiguous block of affected lines. - if (FoundLines) - break; - else - continue; - } Current = Line->First; LineEnd = Line->Last; skipComments(); @@ -309,6 +303,7 @@ private: FirstNonImportLine = Line; break; } + AnyImportAffected = AnyImportAffected || Line->Affected; Reference.Range.setEnd(LineEnd->Tok.getEndLoc()); DEBUG({ llvm::dbgs() << "JsModuleReference: {" @@ -325,6 +320,9 @@ private: References.push_back(Reference); Start = SourceLocation(); } + // Sort imports if any import line was affected. + if (!AnyImportAffected) + References.clear(); return std::make_pair(References, FirstNonImportLine); } diff --git a/unittests/Format/SortImportsTestJS.cpp b/unittests/Format/SortImportsTestJS.cpp index 77c37e337d..e43844c49c 100644 --- a/unittests/Format/SortImportsTestJS.cpp +++ b/unittests/Format/SortImportsTestJS.cpp @@ -190,40 +190,29 @@ TEST_F(SortImportsTestJS, SideEffectImports) { } TEST_F(SortImportsTestJS, AffectedRange) { - // Sort excluding a suffix. - verifySort("import {sym} from 'b';\n" + // Affected range inside of import statements. + verifySort("import {sym} from 'a';\n" + "import {sym} from 'b';\n" "import {sym} from 'c';\n" - "import {sym} from 'a';\n" + "\n" "let x = 1;", "import {sym} from 'c';\n" "import {sym} from 'b';\n" "import {sym} from 'a';\n" "let x = 1;", 0, 30); - // Sort excluding a prefix. + // Affected range outside of import statements. verifySort("import {sym} from 'c';\n" - "import {sym} from 'a';\n" - "import {sym} from 'b';\n" - "\n" - "let x = 1;", - "import {sym} from 'c';\n" "import {sym} from 'b';\n" "import {sym} from 'a';\n" "\n" "let x = 1;", - 30, 0); - // Sort a range within imports. - verifySort("import {sym} from 'c';\n" - "import {sym} from 'a';\n" - "import {sym} from 'b';\n" - "import {sym} from 'c';\n" - "let x = 1;", "import {sym} from 'c';\n" "import {sym} from 'b';\n" "import {sym} from 'a';\n" - "import {sym} from 'c';\n" + "\n" "let x = 1;", - 24, 30); + 70, 1); } TEST_F(SortImportsTestJS, SortingCanShrink) { -- 2.40.0