Index: cfe/trunk/lib/Format/Format.cpp =================================================================== --- cfe/trunk/lib/Format/Format.cpp +++ cfe/trunk/lib/Format/Format.cpp @@ -34,6 +34,7 @@ #include "llvm/Support/Path.h" #include "llvm/Support/Regex.h" #include "llvm/Support/YAMLTraits.h" +#include #include #include #include @@ -1225,14 +1226,7 @@ // 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; Index: cfe/trunk/lib/Format/SortJavaScriptImports.cpp =================================================================== --- cfe/trunk/lib/Format/SortJavaScriptImports.cpp +++ cfe/trunk/lib/Format/SortJavaScriptImports.cpp @@ -25,6 +25,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/Debug.h" +#include #include #define DEBUG_TYPE "format-formatter" @@ -40,6 +41,13 @@ 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 @@ [&](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 @@ 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 @@ } 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 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 @@ 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 @@ Symbol.Alias = Current->TokenText; nextToken(); } + Symbol.Range.setEnd(Current->Tok.getLocation()); Reference.Symbols.push_back(Symbol); if (Current->is(tok::r_brace)) Index: cfe/trunk/unittests/Format/SortImportsTestJS.cpp =================================================================== --- cfe/trunk/unittests/Format/SortImportsTestJS.cpp +++ cfe/trunk/unittests/Format/SortImportsTestJS.cpp @@ -67,6 +67,21 @@ "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 @@ "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"