r271400 - clang-format: [JS] Sort imported symbols.
Martin Probst via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 1 08:19:53 PDT 2016
Author: mprobst
Date: Wed Jun 1 10:19:53 2016
New Revision: 271400
URL: http://llvm.org/viewvc/llvm-project?rev=271400&view=rev
Log:
clang-format: [JS] Sort imported symbols.
Summary: E.g. sort `import {b, a} from 'x';` into `import {a, b} from 'x';`.
Reviewers: djasper
Subscribers: cfe-commits, klimek
Differential Revision: http://reviews.llvm.org/D20798
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/SortJavaScriptImports.cpp
cfe/trunk/unittests/Format/SortImportsTestJS.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=271400&r1=271399&r2=271400&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Jun 1 10:19:53 2016
@@ -34,6 +34,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
#include "llvm/Support/YAMLTraits.h"
+#include <algorithm>
#include <memory>
#include <queue>
#include <string>
@@ -1225,14 +1226,7 @@ static void sortCppIncludes(const Format
// If the #includes are out of order, we generate a single replacement fixing
// the entire block. Otherwise, no replacement is generated.
- bool OutOfOrder = false;
- for (unsigned i = 1, e = Indices.size(); i != e; ++i) {
- if (Indices[i] != i) {
- OutOfOrder = true;
- break;
- }
- }
- if (!OutOfOrder)
+ if (std::is_sorted(Indices.begin(), Indices.end()))
return;
std::string result;
Modified: cfe/trunk/lib/Format/SortJavaScriptImports.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/SortJavaScriptImports.cpp?rev=271400&r1=271399&r2=271400&view=diff
==============================================================================
--- cfe/trunk/lib/Format/SortJavaScriptImports.cpp (original)
+++ cfe/trunk/lib/Format/SortJavaScriptImports.cpp Wed Jun 1 10:19:53 2016
@@ -25,6 +25,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Debug.h"
+#include <algorithm>
#include <string>
#define DEBUG_TYPE "format-formatter"
@@ -40,6 +41,13 @@ using clang::format::FormatStyle;
struct JsImportedSymbol {
StringRef Symbol;
StringRef Alias;
+ SourceRange Range;
+
+ bool operator==(const JsImportedSymbol &RHS) const {
+ // Ignore Range for comparison, it is only used to stitch code together,
+ // but imports at different code locations are still conceptually the same.
+ return Symbol == RHS.Symbol && Alias == RHS.Alias;
+ }
};
// An ES6 module reference.
@@ -139,23 +147,14 @@ public:
[&](unsigned LHSI, unsigned RHSI) {
return References[LHSI] < References[RHSI];
});
- // FIXME: Pull this into a common function.
- bool OutOfOrder = false;
- for (unsigned i = 0, e = Indices.size(); i != e; ++i) {
- if (i != Indices[i]) {
- OutOfOrder = true;
- break;
- }
- }
- if (!OutOfOrder)
- return Result;
+ bool ReferencesInOrder = std::is_sorted(Indices.begin(), Indices.end());
- // Replace all existing import/export statements.
std::string ReferencesText;
+ bool SymbolsInOrder = true;
for (unsigned i = 0, e = Indices.size(); i != e; ++i) {
JsModuleReference Reference = References[Indices[i]];
- StringRef ReferenceStmt = getSourceText(Reference.Range);
- ReferencesText += ReferenceStmt;
+ if (appendReference(ReferencesText, Reference))
+ SymbolsInOrder = false;
if (i + 1 < e) {
// Insert breaks between imports and exports.
ReferencesText += "\n";
@@ -167,6 +166,10 @@ public:
ReferencesText += "\n";
}
}
+
+ if (ReferencesInOrder && SymbolsInOrder)
+ return Result;
+
// Separate references from the main code body of the file.
if (FirstNonImportLine && FirstNonImportLine->First->NewlinesBefore < 2)
ReferencesText += "\n";
@@ -211,10 +214,45 @@ private:
}
StringRef getSourceText(SourceRange Range) {
+ return getSourceText(Range.getBegin(), Range.getEnd());
+ }
+
+ StringRef getSourceText(SourceLocation Begin, SourceLocation End) {
const SourceManager &SM = Env.getSourceManager();
- return FileContents.substr(SM.getFileOffset(Range.getBegin()),
- SM.getFileOffset(Range.getEnd()) -
- SM.getFileOffset(Range.getBegin()));
+ return FileContents.substr(SM.getFileOffset(Begin),
+ SM.getFileOffset(End) - SM.getFileOffset(Begin));
+ }
+
+ // Appends ``Reference`` to ``Buffer``, returning true if text within the
+ // ``Reference`` changed (e.g. symbol order).
+ bool appendReference(std::string &Buffer, JsModuleReference &Reference) {
+ // 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;
+ std::stable_sort(
+ Symbols.begin(), Symbols.end(),
+ [&](const JsImportedSymbol &LHS, const JsImportedSymbol &RHS) {
+ return LHS.Symbol < RHS.Symbol;
+ });
+ if (Symbols == Reference.Symbols) {
+ // No change in symbol order.
+ StringRef ReferenceStmt = getSourceText(Reference.Range);
+ Buffer += ReferenceStmt;
+ return false;
+ }
+ // 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);
+ // ... then the references in order ...
+ for (auto *I = Symbols.begin(), *E = Symbols.end(); I != E; ++I) {
+ if (I != Symbols.begin())
+ Buffer += ",";
+ Buffer += getSourceText(I->Range);
+ }
+ // ... followed by the module reference end.
+ Buffer += getSourceText(SymbolsEnd, Reference.Range.getEnd());
+ return true;
}
// Parses module references in the given lines. Returns the module references,
@@ -350,6 +388,9 @@ private:
JsImportedSymbol Symbol;
Symbol.Symbol = Current->TokenText;
+ // Make sure to include any preceding comments.
+ Symbol.Range.setBegin(
+ Current->getPreviousNonComment()->Next->WhitespaceRange.getBegin());
nextToken();
if (Current->is(Keywords.kw_as)) {
@@ -359,6 +400,7 @@ private:
Symbol.Alias = Current->TokenText;
nextToken();
}
+ Symbol.Range.setEnd(Current->Tok.getLocation());
Reference.Symbols.push_back(Symbol);
if (Current->is(tok::r_brace))
Modified: cfe/trunk/unittests/Format/SortImportsTestJS.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortImportsTestJS.cpp?rev=271400&r1=271399&r2=271400&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/SortImportsTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/SortImportsTestJS.cpp Wed Jun 1 10:19:53 2016
@@ -67,6 +67,21 @@ TEST_F(SortImportsTestJS, BasicSorting)
"let x = 1;");
}
+TEST_F(SortImportsTestJS, WrappedImportStatements) {
+ verifySort("import {sym1, sym2} from 'a';\n"
+ "import {sym} from 'b';\n"
+ "\n"
+ "1;",
+ "import\n"
+ " {sym}\n"
+ " from 'b';\n"
+ "import {\n"
+ " sym1,\n"
+ " sym2\n"
+ "} from 'a';\n"
+ "1;");
+}
+
TEST_F(SortImportsTestJS, SeparateMainCodeBody) {
verifySort("import {sym} from 'a';"
"\n"
@@ -101,6 +116,18 @@ TEST_F(SortImportsTestJS, AliasesSymbols
"import {sym1 as alias1} from 'b';\n");
}
+TEST_F(SortImportsTestJS, SortSymbols) {
+ verifySort("import {sym1, sym2 as a, sym3} from 'b';\n",
+ "import {sym2 as a, sym1, sym3} from 'b';\n");
+ verifySort("import {sym1 /* important! */, /*!*/ sym2 as a} from 'b';\n",
+ "import {/*!*/ sym2 as a, sym1 /* important! */} from 'b';\n");
+ verifySort("import {sym1, sym2} from 'b';\n", "import {\n"
+ " sym2 \n"
+ ",\n"
+ " sym1 \n"
+ "} from 'b';\n");
+}
+
TEST_F(SortImportsTestJS, GroupImports) {
verifySort("import {a} from 'absolute';\n"
"\n"
More information about the cfe-commits
mailing list