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

[mprobst]

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.

[mprobst]

Friendly ping.

[djasper]

Looks good.

cfe/trunk/lib/Format/SortJavaScriptImports.cpp

private:
   // affected lines of module references, nullptr otherwise.
   std::pair<SmallVector<JsModuleReference, 16>, AnnotatedLine*>
   parseModuleReferences(const AdditionalKeywords &Keywords,
                         SmallVectorImpl<AnnotatedLine *> &AnnotatedLines) {
     SmallVector<JsModuleReference, 16> References;
     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();
       if (Start.isInvalid() || References.empty())
         // After the first file level comment, consider line comments to be part
         // of the import that immediately follows them by using the previously
         // set Start.
         Start = Line->First->Tok.getLocation();
       if (!Current)
         continue; // Only comments on this line.
       FoundLines = true;
       JsModuleReference Reference;
       Reference.Range.setBegin(Start);
       if (!parseModuleReference(Keywords, Reference)) {
         FirstNonImportLine = Line;
         break;
       }
+      AnyImportAffected = AnyImportAffected || Line->Affected;
       Reference.Range.setEnd(LineEnd->Tok.getEndLoc());
       DEBUG({
         llvm::dbgs() << "JsModuleReference: {"
                      << "is_export: " << Reference.IsExport
                      << ", cat: " << Reference.Category
                      << ", url: " << Reference.URL
                      << ", prefix: " << Reference.Prefix;
         for (size_t i = 0; i < Reference.Symbols.size(); ++i)
           llvm::dbgs() << ", " << Reference.Symbols[i].Symbol << " as "
                        << Reference.Symbols[i].Alias;
         llvm::dbgs() << ", text: " << getSourceText(Reference.Range);
         llvm::dbgs() << "}\n";
       });
       References.push_back(Reference);
       Start = SourceLocation();
     }
+    // Sort imports if any import line was affected.
+    if (!AnyImportAffected)
+      References.clear();
     return std::make_pair(References, FirstNonImportLine);
   }
 
   // Parses a JavaScript/ECMAScript 6 module reference.
   // See http://www.ecma-international.org/ecma-262/6.0/#sec-scripts-and-modules
   // for grammar EBNF (production ModuleItem).
   bool parseModuleReference(const AdditionalKeywords &Keywords,
                             JsModuleReference &Reference) {

cfe/trunk/unittests/Format/SortImportsTestJS.cpp

verifySort("import 'ZZside-effect';\n"
              "import {R} from './relative';\n",
              "import {R} from './relative';\n"
              "import 'ZZside-effect';\n"
              "import {A} from 'absolute';\n"
              "import 'AAside-effect';\n");
 }
 
 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) {
   // Sort excluding a suffix.
   verifySort("import {B} from 'a';\n"
              "import {A} from 'b';\n"
              "\n"
              "1;",