mprobst created this revision.
mprobst added a reviewer: krasimir.
Herald added a subscriber: jeroen.dobbelaere.
mprobst requested review of this revision.
Herald added a project: clang.

Users can define aliases for long symbols using import aliases:

  import X = A.B.C;

Previously, these were unhandled and would terminate import sorting.
With this change, aliases sort as their own group, coming last after all
other imports.

Aliases are not sorted within their group, as they may reference each
other, so order is significant. Aliases sort before ES module exports,
as exports may reference aliases.

  import {A} from 'foo';
  
  import X = A.B.C;
  
  export {X};


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118361

Files:
  clang/lib/Format/SortJavaScriptImports.cpp
  clang/unittests/Format/SortImportsTestJS.cpp

Index: clang/unittests/Format/SortImportsTestJS.cpp
===================================================================
--- clang/unittests/Format/SortImportsTestJS.cpp
+++ clang/unittests/Format/SortImportsTestJS.cpp
@@ -446,6 +446,25 @@
              "const x =   1;");
 }
 
+TEST_F(SortImportsTestJS, ImportEqAliases) {
+  verifySort("import {B} from 'bar';\n"
+             "import {A} from 'foo';\n"
+             "\n"
+             "import C = A.C;\n"
+             "import Z = B.C.Z;\n"
+             "\n"
+             "export {C};\n"
+             "\n"
+             "console.log(Z);\n",
+             "import {A} from 'foo';\n"
+             "import C = A.C;\n"
+             "export {C};\n"
+             "import {B} from 'bar';\n"
+             "import Z = B.C.Z;\n"
+             "\n"
+             "console.log(Z);\n");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/SortJavaScriptImports.cpp
===================================================================
--- clang/lib/Format/SortJavaScriptImports.cpp
+++ clang/lib/Format/SortJavaScriptImports.cpp
@@ -71,8 +71,14 @@
 // required for sorting module references.
 struct JsModuleReference {
   bool FormattingOff = false;
-  bool IsExport = false;
-  // Module references are sorted into these categories, in order.
+  // Module references are sorted coarsely into these three groups, in order.
+  enum ReferenceKind {
+    IMPORT,  // import ... from or side-effect imports.
+    ALIAS,   // import A = B.C;
+    EXPORT,  // export ...
+  };
+  ReferenceKind Kind = ReferenceKind::IMPORT;
+  // Module references are then sorted by these categories, in order.
   enum ReferenceCategory {
     SIDE_EFFECT,     // "import 'something';"
     ABSOLUTE,        // from 'something'
@@ -101,14 +107,16 @@
 };
 
 bool operator<(const JsModuleReference &LHS, const JsModuleReference &RHS) {
-  if (LHS.IsExport != RHS.IsExport)
-    return LHS.IsExport < RHS.IsExport;
+  if (LHS.Kind != RHS.Kind)
+    return LHS.Kind < RHS.Kind;
   if (LHS.Category != RHS.Category)
     return LHS.Category < RHS.Category;
-  if (LHS.Category == JsModuleReference::ReferenceCategory::SIDE_EFFECT)
-    // Side effect imports might be ordering sensitive. Consider them equal so
-    // that they maintain their relative order in the stable sort below.
-    // This retains transitivity because LHS.Category == RHS.Category here.
+  if (LHS.Category == JsModuleReference::ReferenceCategory::SIDE_EFFECT ||
+      LHS.Kind == JsModuleReference::ReferenceKind::ALIAS)
+    // Side effect imports and aliases might be ordering sensitive. Consider
+    // them equal so that they maintain their relative order in the stable sort
+    // below. This retains transitivity because LHS.Category == RHS.Category
+    // here.
     return false;
   // Empty URLs sort *last* (for export {...};).
   if (LHS.URL.empty() != RHS.URL.empty())
@@ -162,9 +170,10 @@
         // Insert breaks between imports and exports.
         ReferencesText += "\n";
         // Separate imports groups with two line breaks, but keep all exports
-        // in a single group.
-        if (!Reference.IsExport &&
-            (Reference.IsExport != References[I + 1].IsExport ||
+        // and aliases in a single group.
+        // NB: exports sort last, so there is no need to check the next.
+        if (Reference.Kind != JsModuleReference::ReferenceKind::EXPORT &&
+            (Reference.Kind != References[I + 1].Kind ||
              Reference.Category != References[I + 1].Category))
           ReferencesText += "\n";
       }
@@ -298,7 +307,7 @@
       //   mismatching
       if (Reference->Category == JsModuleReference::SIDE_EFFECT ||
           PreviousReference->Category == JsModuleReference::SIDE_EFFECT ||
-          Reference->IsExport != PreviousReference->IsExport ||
+          Reference->Kind != PreviousReference->Kind ||
           !PreviousReference->Prefix.empty() || !Reference->Prefix.empty() ||
           !PreviousReference->DefaultImport.empty() ||
           !Reference->DefaultImport.empty() || Reference->Symbols.empty() ||
@@ -398,6 +407,8 @@
       JsModuleReference Reference;
       Reference.FormattingOff = FormattingOff;
       Reference.Range.setBegin(Start);
+      // References w/o a URL, e.g. export {A}, groups with RELATIVE.
+      Reference.Category = JsModuleReference::ReferenceCategory::RELATIVE;
       if (!parseModuleReference(Keywords, Reference)) {
         if (!FirstNonImportLine)
           FirstNonImportLine = Line; // if no comment before.
@@ -409,7 +420,7 @@
       LLVM_DEBUG({
         llvm::dbgs() << "JsModuleReference: {"
                      << "formatting_off: " << Reference.FormattingOff
-                     << ", is_export: " << Reference.IsExport
+                     << ", kind: " << Reference.Kind
                      << ", cat: " << Reference.Category
                      << ", url: " << Reference.URL
                      << ", prefix: " << Reference.Prefix;
@@ -434,10 +445,12 @@
                             JsModuleReference &Reference) {
     if (!Current || !Current->isOneOf(Keywords.kw_import, tok::kw_export))
       return false;
-    Reference.IsExport = Current->is(tok::kw_export);
+    if (Current->is(tok::kw_export))
+      Reference.Kind = JsModuleReference::ReferenceKind::EXPORT;
 
     nextToken();
-    if (Current->isStringLiteral() && !Reference.IsExport) {
+    if (Current->isStringLiteral() &&
+        Reference.Kind != JsModuleReference::ReferenceKind::EXPORT) {
       // "import 'side-effect';"
       Reference.Category = JsModuleReference::ReferenceCategory::SIDE_EFFECT;
       Reference.URL =
@@ -463,9 +476,6 @@
         Reference.Category = JsModuleReference::ReferenceCategory::RELATIVE;
       else
         Reference.Category = JsModuleReference::ReferenceCategory::ABSOLUTE;
-    } else {
-      // w/o URL groups with "empty".
-      Reference.Category = JsModuleReference::ReferenceCategory::RELATIVE;
     }
     return true;
   }
@@ -501,6 +511,21 @@
       nextToken();
       if (Current->is(Keywords.kw_from))
         return true;
+      // import X = A.B.C;
+      if (Current->is(tok::equal)) {
+        Reference.Kind = JsModuleReference::ReferenceKind::ALIAS;
+        nextToken();
+        while (Current->is(tok::identifier)) {
+          nextToken();
+          if (Current->is(tok::semi)) {
+            nextToken();
+            return true;
+          }
+          if (!Current->is(tok::period))
+            return false;
+          nextToken();
+        }
+      }
       if (Current->isNot(tok::comma))
         return false;
       nextToken(); // eat comma.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to