[clang] d45df0d - clang-format: [JS] merge import lines.
Martin Probst via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 14 08:20:33 PDT 2021
Author: Martin Probst
Date: 2021-04-14T17:20:07+02:00
New Revision: d45df0d29f7005d3c25357f3982002eaf339f875
URL: https://github.com/llvm/llvm-project/commit/d45df0d29f7005d3c25357f3982002eaf339f875
DIFF: https://github.com/llvm/llvm-project/commit/d45df0d29f7005d3c25357f3982002eaf339f875.diff
LOG: clang-format: [JS] merge import lines.
Multiple lines importing from the same URL can be merged:
import {X} from 'a';
import {Y} from 'a';
Merge to:
import {X, Y} from 'a';
This change implements this merge operation. It takes care not to merge in
various corner case situations (default imports, star imports).
Differential Revision: https://reviews.llvm.org/D100466
Added:
Modified:
clang/lib/Format/SortJavaScriptImports.cpp
clang/unittests/Format/SortImportsTestJS.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/SortJavaScriptImports.cpp b/clang/lib/Format/SortJavaScriptImports.cpp
index db2b65b088988..c1cf3de153256 100644
--- a/clang/lib/Format/SortJavaScriptImports.cpp
+++ b/clang/lib/Format/SortJavaScriptImports.cpp
@@ -83,8 +83,13 @@ struct JsModuleReference {
// Prefix from "import * as prefix". Empty for symbol imports and `export *`.
// Implies an empty names list.
StringRef Prefix;
+ // Default import from "import DefaultName from '...';".
+ StringRef DefaultImport;
// Symbols from `import {SymbolA, SymbolB, ...} from ...;`.
SmallVector<JsImportedSymbol, 1> Symbols;
+ // The source location just after { and just before } in the import.
+ // Extracted eagerly to allow modification of Symbols later on.
+ SourceLocation SymbolsStart, SymbolsEnd;
// Textual position of the import/export, including preceding and trailing
// comments.
SourceRange Range;
@@ -146,6 +151,41 @@ class JavaScriptImportSorter : public TokenAnalyzer {
});
bool ReferencesInOrder = llvm::is_sorted(Indices);
+ // Merge module references:
+ // After sorting, find all references that import named symbols from the
+ // same URL and merge their names. E.g.
+ // import {X} from 'a';
+ // import {Y} from 'a';
+ // should be rewritten to:
+ // import {X, Y} from 'a';
+ // Note: this removes merged imports from Indices, but not from References.
+ // The loop below iterates Indices, so things work out.
+ JsModuleReference *PreviousReference = &References[Indices[0]];
+ auto *it = std::next(Indices.begin());
+ bool ImportsMerged = false;
+ while (it != std::end(Indices)) {
+ JsModuleReference *Reference = &References[*it];
+ // Skip:
+ // import 'foo';
+ // import * as foo from 'foo'; on either previous or this.
+ // import Default from 'foo'; on either previous or this.
+ // mismatching
+ if (Reference->Category == JsModuleReference::SIDE_EFFECT ||
+ !PreviousReference->Prefix.empty() || !Reference->Prefix.empty() ||
+ !PreviousReference->DefaultImport.empty() ||
+ !Reference->DefaultImport.empty() || Reference->Symbols.empty() ||
+ PreviousReference->URL != Reference->URL) {
+ PreviousReference = Reference;
+ ++it;
+ continue;
+ }
+ // Merge symbols from identical imports.
+ PreviousReference->Symbols.append(Reference->Symbols);
+ ImportsMerged = true;
+ // Remove the merged import.
+ it = Indices.erase(it);
+ }
+
std::string ReferencesText;
bool SymbolsInOrder = true;
for (unsigned i = 0, e = Indices.size(); i != e; ++i) {
@@ -164,7 +204,7 @@ class JavaScriptImportSorter : public TokenAnalyzer {
}
}
- if (ReferencesInOrder && SymbolsInOrder)
+ if (ReferencesInOrder && SymbolsInOrder && !ImportsMerged)
return {Result, 0};
SourceRange InsertionPoint = References[0].Range;
@@ -245,20 +285,18 @@ class JavaScriptImportSorter : public TokenAnalyzer {
// Sort the individual symbols within the import.
// E.g. `import {b, a} from 'x';` -> `import {a, b} from 'x';`
SmallVector<JsImportedSymbol, 1> Symbols = Reference.Symbols;
- llvm::stable_sort(
- Symbols, [&](const JsImportedSymbol &LHS, const JsImportedSymbol &RHS) {
- return LHS.Symbol.compare_lower(RHS.Symbol) < 0;
- });
- if (Symbols == Reference.Symbols) {
- // No change in symbol order.
+ if (Symbols.empty()) {
+ // No symbol imports, just emit the entire module reference.
StringRef ReferenceStmt = getSourceText(Reference.Range);
Buffer += ReferenceStmt;
return false;
}
+ llvm::stable_sort(
+ Symbols, [&](const JsImportedSymbol &LHS, const JsImportedSymbol &RHS) {
+ return LHS.Symbol.compare_lower(RHS.Symbol) < 0;
+ });
// Stitch together the module reference start...
- SourceLocation SymbolsStart = Reference.Symbols.front().Range.getBegin();
- SourceLocation SymbolsEnd = Reference.Symbols.back().Range.getEnd();
- Buffer += getSourceText(Reference.Range.getBegin(), SymbolsStart);
+ Buffer += getSourceText(Reference.Range.getBegin(), Reference.SymbolsStart);
// ... then the references in order ...
for (auto I = Symbols.begin(), E = Symbols.end(); I != E; ++I) {
if (I != Symbols.begin())
@@ -266,7 +304,7 @@ class JavaScriptImportSorter : public TokenAnalyzer {
Buffer += getSourceText(I->Range);
}
// ... followed by the module reference end.
- Buffer += getSourceText(SymbolsEnd, Reference.Range.getEnd());
+ Buffer += getSourceText(Reference.SymbolsEnd, Reference.Range.getEnd());
return true;
}
@@ -280,7 +318,7 @@ class JavaScriptImportSorter : public TokenAnalyzer {
SourceLocation Start;
AnnotatedLine *FirstNonImportLine = nullptr;
bool AnyImportAffected = false;
- for (auto Line : AnnotatedLines) {
+ for (auto *Line : AnnotatedLines) {
Current = Line->First;
LineEnd = Line->Last;
skipComments();
@@ -393,7 +431,9 @@ class JavaScriptImportSorter : public TokenAnalyzer {
bool parseNamedBindings(const AdditionalKeywords &Keywords,
JsModuleReference &Reference) {
+ // eat a potential "import X, " prefix.
if (Current->is(tok::identifier)) {
+ Reference.DefaultImport = Current->TokenText;
nextToken();
if (Current->is(Keywords.kw_from))
return true;
@@ -405,6 +445,7 @@ class JavaScriptImportSorter : public TokenAnalyzer {
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))
@@ -432,6 +473,11 @@ class JavaScriptImportSorter : public TokenAnalyzer {
if (!Current->isOneOf(tok::r_brace, tok::comma))
return false;
}
+ Reference.SymbolsEnd = Current->Tok.getLocation();
+ // For named imports with a trailing comma ("import {X,}"), consider the
+ // comma to be the end of the import list, so that it doesn't get removed.
+ if (Current->Previous->is(tok::comma))
+ Reference.SymbolsEnd = Current->Previous->Tok.getLocation();
nextToken(); // consume r_brace
return true;
}
diff --git a/clang/unittests/Format/SortImportsTestJS.cpp b/clang/unittests/Format/SortImportsTestJS.cpp
index 72c79ac71828e..9f4904426cc83 100644
--- a/clang/unittests/Format/SortImportsTestJS.cpp
+++ b/clang/unittests/Format/SortImportsTestJS.cpp
@@ -307,6 +307,52 @@ TEST_F(SortImportsTestJS, SortDefaultImports) {
"import {A} from 'a';\n");
}
+TEST_F(SortImportsTestJS, MergeImports) {
+ // basic operation
+ verifySort("import {X, Y} from 'a';\n"
+ "import {Z} from 'z';\n"
+ "\n"
+ "X + Y + Z;\n",
+ "import {X} from 'a';\n"
+ "import {Z} from 'z';\n"
+ "import {Y} from 'a';\n"
+ "\n"
+ "X + Y + Z;\n");
+
+ // empty imports
+ verifySort("import {A} from 'foo';\n", "import {} from 'foo';\n"
+ "import {A} from 'foo';");
+
+ // ignores import *
+ verifySort("import * as foo from 'foo';\n"
+ "import {A} from 'foo';\n",
+ "import * as foo from 'foo';\n"
+ "import {A} from 'foo';\n");
+
+ // ignores default import
+ verifySort("import X from 'foo';\n"
+ "import {A} from 'foo';\n",
+ "import X from 'foo';\n"
+ "import {A} from 'foo';\n");
+
+ // keeps comments
+ // known issue: loses the 'also a' comment.
+ verifySort("// a\n"
+ "import {/* x */ X, /* y */ Y} from 'a';\n"
+ "// z\n"
+ "import {Z} from 'z';\n"
+ "\n"
+ "X + Y + Z;\n",
+ "// a\n"
+ "import {/* y */ Y} from 'a';\n"
+ "// z\n"
+ "import {Z} from 'z';\n"
+ "// also a\n"
+ "import {/* x */ X} from 'a';\n"
+ "\n"
+ "X + Y + Z;\n");
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
More information about the cfe-commits
mailing list