r280485 - clang-format: [JS] Sort all JavaScript imports if any changed.

[Martin Probst via cfe-commits]

Author: mprobst
Date: Fri Sep  2 09:01:17 2016
New Revision: 280485

URL: http://llvm.org/viewvc/llvm-project?rev=280485&view=rev
Log:
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

Modified:
    cfe/trunk/lib/Format/SortJavaScriptImports.cpp
    cfe/trunk/unittests/Format/SortImportsTestJS.cpp

Modified: cfe/trunk/lib/Format/SortJavaScriptImports.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/SortJavaScriptImports.cpp?rev=280485&r1=280484&r2=280485&view=diff
==============================================================================
--- cfe/trunk/lib/Format/SortJavaScriptImports.cpp (original)
+++ cfe/trunk/lib/Format/SortJavaScriptImports.cpp Fri Sep  2 09:01:17 2016
@@ -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);
   }
 

Modified: cfe/trunk/unittests/Format/SortImportsTestJS.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortImportsTestJS.cpp?rev=280485&r1=280484&r2=280485&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/SortImportsTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/SortImportsTestJS.cpp Fri Sep  2 09:01:17 2016
@@ -190,40 +190,29 @@ TEST_F(SortImportsTestJS, SideEffectImpo
 }
 
 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) {