[clang-format] Fix a JavaScript import order bug.

[owenpan]

Summary

When the imported symbol is exactly "template" the sorting is disabled.
"import {template} from x;" is not recognized as an import.

[djasper]

Generally, upload patches with the full file as context (that will prevent Phabricator's "Context not available")
But this change looks good. Thank you.

[MyDeveloperDay]

Are you waiting for someone to land this?

[shafik]

@MyDeveloperDay this looks like it still could be relevant.

[owenpan]

The test case doesn't pass.

[owenpan]

Make the test case pass.

clang/lib/Format/SortJavaScriptImports.cpp

if (Current->isNot(tok::l_brace))
       return false;
 
     // {sym as alias, sym2 as ...} from '...';
     Reference.SymbolsStart = Current->Tok.getEndLoc();
     while (Current->isNot(tok::r_brace)) {
       nextToken();
       if (Current->is(tok::r_brace))
         break;
-      bool isTypeOnly =
-          Current->is(Keywords.kw_type) && Current->Next &&
-          Current->Next->isOneOf(tok::identifier, tok::kw_default);
-      if (!isTypeOnly && !Current->isOneOf(tok::identifier, tok::kw_default))
+      auto IsIdentifier = [](const auto *Tok) {
+        return Tok->isOneOf(tok::identifier, tok::kw_default, tok::kw_template);
+      };
+      bool isTypeOnly = Current->is(Keywords.kw_type) && Current->Next &&
+                        IsIdentifier(Current->Next);
+      if (!isTypeOnly && !IsIdentifier(Current))
         return false;
 
       JsImportedSymbol Symbol;
       // Make sure to include any preceding comments.
       Symbol.Range.setBegin(
           Current->getPreviousNonComment()->Next->WhitespaceRange.getBegin());
       if (isTypeOnly)
         nextToken();
       Symbol.Symbol = Current->TokenText;
       nextToken();
 
       if (Current->is(Keywords.kw_as)) {
         nextToken();
-        if (!Current->isOneOf(tok::identifier, tok::kw_default))
+        if (!IsIdentifier(Current))
           return false;
         Symbol.Alias = Current->TokenText;
         nextToken();
       }
       Symbol.Range.setEnd(Current->Tok.getLocation());
       Reference.Symbols.push_back(Symbol);
 
       if (!Current->isOneOf(tok::r_brace, tok::comma))

clang/unittests/Format/SortImportsTestJS.cpp

TEST_F(SortImportsTestJS, ImportExportType) {
   verifySort("export type A = B;\n"
              "export {X};\n"
              "export {Y};\n",
              "export type A = B;\n"
              "export {X};\n"
              "export {Y};\n");
 }
 
+TEST_F(SortImportsTestJS, TemplateKeyword) {
+  // Reproduces issue where importing "template" disables imports sorting.
+  verifySort("import {template} from './a';\n"
+             "import {b} from './b';\n",
+             "import {b} from './b';\n"
+             "import {template} from './a';\n");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang