jankuehle created this revision.
jankuehle added a project: clang-format.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
jankuehle requested review of this revision.

Users can choose to only import/export the type of the symbol (not value nor 
namespace) by adding a `type` keyword, e.g.:

  import type {x} from 'y';
  import {type x} from 'y';
  export type {x};
  export {type x};

Previously, this was not handled and would:

- Terminate import sorting
- Remove the space before the curly bracket in `export type {`

With this change, both formatting and import sorting work as expected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150116

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

Index: clang/unittests/Format/SortImportsTestJS.cpp
===================================================================
--- clang/unittests/Format/SortImportsTestJS.cpp
+++ clang/unittests/Format/SortImportsTestJS.cpp
@@ -465,6 +465,46 @@
              "console.log(Z);\n");
 }
 
+TEST_F(SortImportsTestJS, ImportExportType) {
+  verifySort("import type {sym} from 'a';\n"
+             "import {type sym} from 'b';\n"
+             "import {sym} from 'c';\n"
+             "import type sym from 'd';\n"
+             "import type * as sym from 'e';\n"
+             "\n"
+             "let x = 1;",
+             "import {sym} from 'c';\n"
+             "import type {sym} from 'a';\n"
+             "import type * as sym from 'e';\n"
+             "import type sym from 'd';\n"
+             "import {type sym} from 'b';\n"
+             "let x = 1;");
+
+  // Symbols within import statement
+  verifySort("import {type sym1, type sym2 as a, sym3} from 'b';\n",
+             "import {type sym2 as a, type sym1, sym3} from 'b';\n");
+
+  // Merging
+  verifySort("import {X, type Z} from 'a';\n"
+             "import type {Y} from 'a';\n"
+             "\n"
+             "X + Y + Z;\n",
+             "import {X} from 'a';\n"
+             "import {type Z} from 'a';\n"
+             "import type {Y} from 'a';\n"
+             "\n"
+             "X + Y + Z;\n");
+
+  // Merging: empty imports
+  verifySort("import type {A} from 'foo';\n", "import type {} from 'foo';\n"
+                                              "import type {A} from 'foo';");
+
+  // Merging: exports
+  verifySort("export {A, type B} from 'foo';\n",
+             "export {A} from 'foo';\n"
+             "export   {type B} from 'foo';");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/unittests/Format/FormatTestJS.cpp
===================================================================
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -1476,6 +1476,17 @@
                " export class Y {}");
 }
 
+TEST_F(FormatTestJS, ImportExportType) {
+  verifyFormat("import type {x, y} from 'y';\n"
+               "import type * as x from 'y';\n"
+               "import type x from 'y';\n"
+               "import {x, type yu, z} from 'y';\n");
+  verifyFormat("export type {x, y} from 'y';\n"
+               "export {x, type yu, z} from 'y';\n"
+               "export type {x, y};\n"
+               "export {x, type yu, z};\n");
+}
+
 TEST_F(FormatTestJS, ClosureStyleCasts) {
   verifyFormat("var x = /** @type {foo} */ (bar);");
 }
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -4068,7 +4068,9 @@
   // parsing the structural element, i.e. the declaration or expression for
   // `export default`.
   if (!IsImport && !FormatTok->isOneOf(tok::l_brace, tok::star) &&
-      !FormatTok->isStringLiteral()) {
+      !FormatTok->isStringLiteral() &&
+      !(FormatTok->is(Keywords.kw_type) &&
+        Tokens->peekNextToken()->isOneOf(tok::l_brace, tok::star))) {
     return;
   }
 
Index: clang/lib/Format/SortJavaScriptImports.cpp
===================================================================
--- clang/lib/Format/SortJavaScriptImports.cpp
+++ clang/lib/Format/SortJavaScriptImports.cpp
@@ -72,6 +72,7 @@
 struct JsModuleReference {
   bool FormattingOff = false;
   bool IsExport = false;
+  bool IsTypeOnly = false;
   // Module references are sorted into these categories, in order.
   enum ReferenceCategory {
     SIDE_EFFECT,     // "import 'something';"
@@ -306,6 +307,7 @@
       if (Reference->Category == JsModuleReference::SIDE_EFFECT ||
           PreviousReference->Category == JsModuleReference::SIDE_EFFECT ||
           Reference->IsExport != PreviousReference->IsExport ||
+          Reference->IsTypeOnly != PreviousReference->IsTypeOnly ||
           !PreviousReference->Prefix.empty() || !Reference->Prefix.empty() ||
           !PreviousReference->DefaultImport.empty() ||
           !Reference->DefaultImport.empty() || Reference->Symbols.empty() ||
@@ -488,6 +490,11 @@
   bool parseStarBinding(const AdditionalKeywords &Keywords,
                         JsModuleReference &Reference) {
     // * as prefix from '...';
+    if (Current->is(Keywords.kw_type) && Current->Next &&
+        Current->Next->is(tok::star)) {
+      Reference.IsTypeOnly = true;
+      nextToken();
+    }
     if (Current->isNot(tok::star))
       return false;
     nextToken();
@@ -503,6 +510,12 @@
 
   bool parseNamedBindings(const AdditionalKeywords &Keywords,
                           JsModuleReference &Reference) {
+    if (Current->is(Keywords.kw_type) && Current->Next &&
+        Current->Next->isOneOf(tok::identifier, tok::l_brace)) {
+      Reference.IsTypeOnly = true;
+      nextToken();
+    }
+
     // eat a potential "import X, " prefix.
     if (Current->is(tok::identifier)) {
       Reference.DefaultImport = Current->TokenText;
@@ -535,14 +548,19 @@
       nextToken();
       if (Current->is(tok::r_brace))
         break;
-      if (!Current->isOneOf(tok::identifier, tok::kw_default))
+      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))
         return false;
 
       JsImportedSymbol Symbol;
-      Symbol.Symbol = Current->TokenText;
       // 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)) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to