From 93008f01545f47bb6e07f4a6da88f2b5283ce6a0 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Tue, 18 Jul 2017 14:00:19 +0000 Subject: [PATCH] clang-format: [JS] Correctly format JavaScript imports with long module paths Currently the `UnwrappedLineParser` fails to correctly unwrap JavaScript imports where the module path is not on the same line as the `from` keyword. For example: import {A} from 'some/path/longer/than/column/limit/module.js';``` This causes issues when in the middle a list of imports because the formatter thinks it has reached the end of the imports, and therefore will not sort any imports lower in the list. The formatter will, however, split the `from` keyword and the module path if the path exceeds the column limit, which triggers the issue the next time the file is formatted. Patch originally by Jared Neil - thanks! Differential Revision: https://reviews.llvm.org/D34920 llvm-svn: 308306 --- clang/lib/Format/UnwrappedLineParser.cpp | 2 +- clang/unittests/Format/FormatTestJS.cpp | 11 +++++++++++ clang/unittests/Format/SortImportsTestJS.cpp | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 4b57919d1929..faac5a371c26 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -747,7 +747,7 @@ static bool mustBeJSIdent(const AdditionalKeywords &Keywords, Keywords.kw_let, Keywords.kw_var, tok::kw_const, Keywords.kw_abstract, Keywords.kw_extends, Keywords.kw_implements, Keywords.kw_instanceof, Keywords.kw_interface, - Keywords.kw_throws)); + Keywords.kw_throws, Keywords.kw_from)); } static bool mustBeJSIdentOrValue(const AdditionalKeywords &Keywords, diff --git a/clang/unittests/Format/FormatTestJS.cpp b/clang/unittests/Format/FormatTestJS.cpp index 11e386a1c7c7..c256ebe46263 100644 --- a/clang/unittests/Format/FormatTestJS.cpp +++ b/clang/unittests/Format/FormatTestJS.cpp @@ -1464,6 +1464,17 @@ TEST_F(FormatTestJS, ImportWrapping) { " A,\n" "} from 'some/module.js';", Style); + Style.ColumnLimit = 40; + // Using this version of verifyFormat because test::messUp hides the issue. + verifyFormat("import {\n" + " A,\n" + "} from\n" + " 'some/path/longer/than/column/limit/module.js';", + " import { \n" + " A, \n" + " } from\n" + " 'some/path/longer/than/column/limit/module.js' ; ", + Style); } TEST_F(FormatTestJS, TemplateStrings) { diff --git a/clang/unittests/Format/SortImportsTestJS.cpp b/clang/unittests/Format/SortImportsTestJS.cpp index 7e766e1969e1..4208b29702dd 100644 --- a/clang/unittests/Format/SortImportsTestJS.cpp +++ b/clang/unittests/Format/SortImportsTestJS.cpp @@ -283,6 +283,23 @@ TEST_F(SortImportsTestJS, SortCaseInsensitive) { "1;"); } +TEST_F(SortImportsTestJS, SortMultiLine) { + // Reproduces issue where multi-line import was not parsed correctly. + verifySort("import {A} from 'a';\n" + "import {A} from 'b';\n" + "\n" + "1;", + "import\n" + "{\n" + "A\n" + "}\n" + "from\n" + "'b';\n" + "import {A} from 'a';\n" + "\n" + "1;"); +} + } // end namespace } // end namespace format } // end namespace clang