Author: mprobst Date: Fri Sep 2 09:01:17 2016 New Revision: 280485 URL: http://llvm.org/viewvc/llvm-project?rev=280485&view=rev Log: clang-format: [JS] Sort all JavaScript imports if any changed.
Summary: User feedback is that they expect *all* imports to be sorted if any import was affected by a change, not just imports up to the first non-affected line, as clang-format currently does. Reviewers: djasper Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D23972 Modified: cfe/trunk/lib/Format/SortJavaScriptImports.cpp cfe/trunk/unittests/Format/SortImportsTestJS.cpp Modified: cfe/trunk/lib/Format/SortJavaScriptImports.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/SortJavaScriptImports.cpp?rev=280485&r1=280484&r2=280485&view=diff ============================================================================== --- cfe/trunk/lib/Format/SortJavaScriptImports.cpp (original) +++ cfe/trunk/lib/Format/SortJavaScriptImports.cpp Fri Sep 2 09:01:17 2016 @@ -284,14 +284,8 @@ private: SourceLocation Start; bool FoundLines = false; AnnotatedLine *FirstNonImportLine = nullptr; + bool AnyImportAffected = false; for (auto Line : AnnotatedLines) { - if (!Line->Affected) { - // Only sort the first contiguous block of affected lines. - if (FoundLines) - break; - else - continue; - } Current = Line->First; LineEnd = Line->Last; skipComments(); @@ -309,6 +303,7 @@ private: FirstNonImportLine = Line; break; } + AnyImportAffected = AnyImportAffected || Line->Affected; Reference.Range.setEnd(LineEnd->Tok.getEndLoc()); DEBUG({ llvm::dbgs() << "JsModuleReference: {" @@ -325,6 +320,9 @@ private: References.push_back(Reference); Start = SourceLocation(); } + // Sort imports if any import line was affected. + if (!AnyImportAffected) + References.clear(); return std::make_pair(References, FirstNonImportLine); } Modified: cfe/trunk/unittests/Format/SortImportsTestJS.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortImportsTestJS.cpp?rev=280485&r1=280484&r2=280485&view=diff ============================================================================== --- cfe/trunk/unittests/Format/SortImportsTestJS.cpp (original) +++ cfe/trunk/unittests/Format/SortImportsTestJS.cpp Fri Sep 2 09:01:17 2016 @@ -190,40 +190,29 @@ TEST_F(SortImportsTestJS, SideEffectImpo } TEST_F(SortImportsTestJS, AffectedRange) { - // Sort excluding a suffix. - verifySort("import {sym} from 'b';\n" + // Affected range inside of import statements. + verifySort("import {sym} from 'a';\n" + "import {sym} from 'b';\n" "import {sym} from 'c';\n" - "import {sym} from 'a';\n" + "\n" "let x = 1;", "import {sym} from 'c';\n" "import {sym} from 'b';\n" "import {sym} from 'a';\n" "let x = 1;", 0, 30); - // Sort excluding a prefix. + // Affected range outside of import statements. verifySort("import {sym} from 'c';\n" - "import {sym} from 'a';\n" - "import {sym} from 'b';\n" - "\n" - "let x = 1;", - "import {sym} from 'c';\n" "import {sym} from 'b';\n" "import {sym} from 'a';\n" "\n" "let x = 1;", - 30, 0); - // Sort a range within imports. - verifySort("import {sym} from 'c';\n" - "import {sym} from 'a';\n" - "import {sym} from 'b';\n" - "import {sym} from 'c';\n" - "let x = 1;", "import {sym} from 'c';\n" "import {sym} from 'b';\n" "import {sym} from 'a';\n" - "import {sym} from 'c';\n" + "\n" "let x = 1;", - 24, 30); + 70, 1); } TEST_F(SortImportsTestJS, SortingCanShrink) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
