[PATCH] D105087: [clang-format] PR49960 clang-format doesn't handle ASI after "return" on JavaScript

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, mprobst.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://bugs.llvm.org/show_bug.cgi?id=49960

clang-format can mutate legal javascript code due to missing semicolons, but clang format pulls the next line into the first causing incorrect grammar.

  function t() {
    if (true) return
    const v = 42
  }

clang-format result:

  function t() {
    if (true)
      return const v = 42
  }

This was already somewhat addressed by the work of @mprobst  in https://reviews.llvm.org/rG1dcbbcfc5cf06d2eacc68fbe9b6fc1fb12168d6f back in 2016, but this revision aims to identify other possible places so clang-format wouldn't be quite so destructive as people develop.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105087

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestJS.cpp


Index: clang/unittests/Format/FormatTestJS.cpp
===================================================================
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2604,5 +2604,41 @@
       "}\n");
 }
 
+TEST_F(FormatTestJS, ASIAfterReturn) {
+  verifyFormat("if (true) return\n"
+               "const v = 42");
+  verifyFormat("if (true) return\n"
+               "let v = 42");
+  verifyFormat("if (true) return\n"
+               "var v = 42");
+  verifyFormat("if (true) break\n"
+               "let v = 42");
+  verifyFormat("if (true) break\n"
+               "const v = 42");
+  verifyFormat("if (true) break\n"
+               "v = 42");
+  verifyFormat("if (true) continue\n"
+               "let v = 42");
+  verifyFormat("if (true) continue\n"
+               "var v = 42");
+  verifyFormat("if (true) continue\n"
+               "const v = 42");
+  verifyFormat("if (true) yield\n"
+               "let v = 42");
+  verifyFormat("if (true) yield\n"
+               "var v = 42");
+  verifyFormat("if (true) yield\n"
+               "const v = 42");
+
+  verifyFormat("if (true) break\n"
+               "var v = 42");
+  verifyFormat("if (true) return\n"
+               "v = 42");
+  verifyFormat("if (true) continue\n"
+               "v = 42");
+  verifyFormat("if (true) yield\n"
+               "v = 42");
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1006,6 +1006,23 @@
   readToken();
   FormatToken *Next = FormatTok;
 
+  if (Previous->isOneOf(tok::kw_return, tok::kw_break, tok::kw_continue,
+                        Keywords.kw_yield)) {
+    if (Next->isOneOf(Keywords.kw_let, tok::kw_const, Keywords.kw_var))
+      return addUnwrappedLine();
+    // Don't go past the end of the file.
+    if (!eof()) {
+      if (Next->is(tok::identifier)) {
+        // Peek the next token.
+        unsigned StoredPosition = Tokens->getPosition();
+        FormatToken *NextNext = Tokens->getNextToken();
+        Tokens->setPosition(StoredPosition);
+        if (NextNext && NextNext->is(tok::equal))
+          return addUnwrappedLine();
+      }
+    }
+  }
+
   bool IsOnSameLine =
       CommentsBeforeNextToken.empty()
           ? Next->NewlinesBefore == 0


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105087.355133.patch
Type: text/x-patch
Size: 2440 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210629/c4010b38/attachment.bin>