[PATCH] D24708: clang-format: [JS] Fix line breaks before comments when sorting imports.

[Martin Probst via cfe-commits]

mprobst created this revision.
mprobst added a reviewer: djasper.
mprobst added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

Previously, clang-format would always insert an additional line break after the
import block if the main body started with a comment, due to loosing track of
the first non-import line.

https://reviews.llvm.org/D24708

Files:
  lib/Format/SortJavaScriptImports.cpp
  unittests/Format/SortImportsTestJS.cpp

Index: unittests/Format/SortImportsTestJS.cpp
===================================================================
--- unittests/Format/SortImportsTestJS.cpp
+++ unittests/Format/SortImportsTestJS.cpp
@@ -121,6 +121,16 @@
              "import {sym} from 'b';  // from //foo:bar\n"
              "// A very important import follows.\n"
              "import {sym} from 'a';  /* more comments */\n");
+  verifySort("import {sym} from 'a';\n"
+             "import {sym} from 'b';\n"
+             "\n"
+             "/** Comment on variable. */\n"
+             "const x = 1;\n",
+             "import {sym} from 'b';\n"
+             "import {sym} from 'a';\n"
+             "\n"
+             "/** Comment on variable. */\n"
+             "const x = 1;\n");
 }
 
 TEST_F(SortImportsTestJS, SortStar) {
Index: lib/Format/SortJavaScriptImports.cpp
===================================================================
--- lib/Format/SortJavaScriptImports.cpp
+++ lib/Format/SortJavaScriptImports.cpp
@@ -293,14 +293,19 @@
         // of the import that immediately follows them by using the previously
         // set Start.
         Start = Line->First->Tok.getLocation();
-      if (!Current)
-        continue; // Only comments on this line.
+      if (!Current) {
+        // Only comments on this line. Could be the first non-import line.
+        FirstNonImportLine = Line;
+        continue;
+      }
       JsModuleReference Reference;
       Reference.Range.setBegin(Start);
       if (!parseModuleReference(Keywords, Reference)) {
-        FirstNonImportLine = Line;
+        if (!FirstNonImportLine)
+          FirstNonImportLine = Line; // if no comment before.
         break;
       }
+      FirstNonImportLine = nullptr;
       AnyImportAffected = AnyImportAffected || Line->Affected;
       Reference.Range.setEnd(LineEnd->Tok.getEndLoc());
       DEBUG({



Index: unittests/Format/SortImportsTestJS.cpp
===================================================================
--- unittests/Format/SortImportsTestJS.cpp
+++ unittests/Format/SortImportsTestJS.cpp
@@ -121,6 +121,16 @@
              "import {sym} from 'b';  // from //foo:bar\n"
              "// A very important import follows.\n"
              "import {sym} from 'a';  /* more comments */\n");
+  verifySort("import {sym} from 'a';\n"
+             "import {sym} from 'b';\n"
+             "\n"
+             "/** Comment on variable. */\n"
+             "const x = 1;\n",
+             "import {sym} from 'b';\n"
+             "import {sym} from 'a';\n"
+             "\n"
+             "/** Comment on variable. */\n"
+             "const x = 1;\n");
 }
 
 TEST_F(SortImportsTestJS, SortStar) {
Index: lib/Format/SortJavaScriptImports.cpp
===================================================================
--- lib/Format/SortJavaScriptImports.cpp
+++ lib/Format/SortJavaScriptImports.cpp
@@ -293,14 +293,19 @@
         // of the import that immediately follows them by using the previously
         // set Start.
         Start = Line->First->Tok.getLocation();
-      if (!Current)
-        continue; // Only comments on this line.
+      if (!Current) {
+        // Only comments on this line. Could be the first non-import line.
+        FirstNonImportLine = Line;
+        continue;
+      }
       JsModuleReference Reference;
       Reference.Range.setBegin(Start);
       if (!parseModuleReference(Keywords, Reference)) {
-        FirstNonImportLine = Line;
+        if (!FirstNonImportLine)
+          FirstNonImportLine = Line; // if no comment before.
         break;
       }
+      FirstNonImportLine = nullptr;
       AnyImportAffected = AnyImportAffected || Line->Affected;
       Reference.Range.setEnd(LineEnd->Tok.getEndLoc());
       DEBUG({

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits