[clang] [clang-format] Disable string breaking in JS for now (PR #66372)

via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 15 05:35:51 PDT 2023


https://github.com/sstwcw updated https://github.com/llvm/llvm-project/pull/66372

>From a736d84df61e3e7c1a4c8b22c7cd1e7524499d64 Mon Sep 17 00:00:00 2001
From: sstwcw <su3e8a96kzlver at posteo.net>
Date: Thu, 14 Sep 2023 13:04:56 +0000
Subject: [PATCH 1/2] [clang-format] Disable string breaking in JS for now

See the discussion
[here](https://github.com/llvm/llvm-project/pull/66168#issuecomment-1719038797).

The functionality is not mature enough.
---
 clang/docs/ClangFormatStyleOptions.rst    |  10 +-
 clang/include/clang/Format/Format.h       |  10 +-
 clang/lib/Format/ContinuationIndenter.cpp |  13 +--
 clang/unittests/Format/FormatTestJS.cpp   | 114 +---------------------
 4 files changed, 16 insertions(+), 131 deletions(-)

diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index d5d9faa5c78cffb..4ab0b3a207270dc 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -2783,17 +2783,17 @@ the configuration (without a prefix: ``Auto``).
      const char* x =
          "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
 
-  In C#, Java, and JavaScript:
+  In C# and Java:
 
   .. code-block:: c++
 
      true:
-     var x = "veryVeryVeryVeryVeryVe" +
-             "ryVeryVeryVeryVeryVery" +
-             "VeryLongString";
+     string x = "veryVeryVeryVeryVeryVe" +
+                "ryVeryVeryVeryVeryVery" +
+                "VeryLongString";
 
      false:
-     var x =
+     string x =
          "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
 
   C# and JavaScript interpolated strings are not broken.
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index c7bd356b7faeded..3bf70838d059086 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2062,15 +2062,15 @@ struct FormatStyle {
   ///        "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
   /// \endcode
   ///
-  /// In C#, Java, and JavaScript:
+  /// In C# and Java:
   /// \code
   ///    true:
-  ///    var x = "veryVeryVeryVeryVeryVe" +
-  ///            "ryVeryVeryVeryVeryVery" +
-  ///            "VeryLongString";
+  ///    string x = "veryVeryVeryVeryVeryVe" +
+  ///               "ryVeryVeryVeryVeryVery" +
+  ///               "VeryLongString";
   ///
   ///    false:
-  ///    var x =
+  ///    string x =
   ///        "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
   /// \endcode
   ///
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 6673b5c703b835f..15b9ad41a37af59 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -2237,15 +2237,10 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current,
                                            LineState &State, bool AllowBreak) {
   unsigned StartColumn = State.Column - Current.ColumnWidth;
   if (Current.isStringLiteral()) {
-    // Strings in JSON can not be broken.
-    if (Style.isJson() || !Style.BreakStringLiterals || !AllowBreak)
-      return nullptr;
-
-    // Strings in TypeScript types and dictionary keys can not be broken.
-    if (Style.isJavaScript() &&
-        (Current.is(TT_SelectorName) ||
-         State.Line->startsWith(Keywords.kw_type) ||
-         State.Line->startsWith(tok::kw_export, Keywords.kw_type))) {
+    // Strings in JSON can not be broken. Breaking strings in JavaScript is
+    // disabled for now.
+    if (Style.isJson() || Style.isJavaScript() || !Style.BreakStringLiterals ||
+        !AllowBreak) {
       return nullptr;
     }
 
diff --git a/clang/unittests/Format/FormatTestJS.cpp b/clang/unittests/Format/FormatTestJS.cpp
index 51543d0a54d8561..23b010dbc982684 100644
--- a/clang/unittests/Format/FormatTestJS.cpp
+++ b/clang/unittests/Format/FormatTestJS.cpp
@@ -1506,121 +1506,11 @@ TEST_F(FormatTestJS, StringLiteralConcatenation) {
   verifyFormat("var literal = 'hello ' +\n"
                "    'world';");
 
-  // Long strings should be broken.
+  // String breaking is disabled for now.
   verifyFormat("var literal =\n"
-               "    'xxxxxxxx ' +\n"
-               "    'xxxxxxxx';",
+               "    'xxxxxxxx xxxxxxxx';",
                "var literal = 'xxxxxxxx xxxxxxxx';",
                getGoogleJSStyleWithColumns(17));
-  verifyFormat("var literal =\n"
-               "    'xxxxxxxx ' +\n"
-               "    'xxxxxxxx';",
-               "var literal = 'xxxxxxxx xxxxxxxx';",
-               getGoogleJSStyleWithColumns(18));
-  verifyFormat("var literal =\n"
-               "    'xxxxxxxx' +\n"
-               "    ' xxxxxxxx';",
-               "var literal = 'xxxxxxxx xxxxxxxx';",
-               getGoogleJSStyleWithColumns(16));
-  // The quotes should be correct.
-  for (char OriginalQuote : {'\'', '"'}) {
-    auto VerifyQuotes = [=](FormatStyle::JavaScriptQuoteStyle StyleQuote,
-                            char TargetQuote) {
-      auto Style = getGoogleJSStyleWithColumns(17);
-      Style.JavaScriptQuotes = StyleQuote;
-      std::string Target{"var literal =\n"
-                         "    \"xxxxxxxx \" +\n"
-                         "    \"xxxxxxxx\";"};
-      std::string Original{"var literal = \"xxxxxxxx xxxxxxxx\";"};
-      std::replace(Target.begin(), Target.end(), '"', TargetQuote);
-      std::replace(Original.begin(), Original.end(), '"', OriginalQuote);
-      verifyFormat(Target, Original, Style);
-    };
-    VerifyQuotes(FormatStyle::JSQS_Leave, OriginalQuote);
-    VerifyQuotes(FormatStyle::JSQS_Single, '\'');
-    VerifyQuotes(FormatStyle::JSQS_Double, '"');
-  }
-  // Parentheses should be added when necessary.
-  verifyFormat("var literal =\n"
-               "    ('xxxxxxxx ' +\n"
-               "     'xxxxxx')[0];",
-               "var literal = 'xxxxxxxx xxxxxx'[0];",
-               getGoogleJSStyleWithColumns(18));
-  auto Style = getGoogleJSStyleWithColumns(20);
-  Style.SpacesInParens = FormatStyle::SIPO_Custom;
-  Style.SpacesInParensOptions.Other = true;
-  verifyFormat("var literal =\n"
-               "    ( 'xxxxxxxx ' +\n"
-               "      'xxxxxx' )[0];",
-               "var literal = 'xxxxxxxx xxxxxx'[0];", Style);
-  // FIXME: When the part before the string literal is shorter than the
-  // continuation indentation, and the option AlignAfterOpenBracket is set to
-  // AlwaysBreak which is the default for the Google style, the unbroken string
-  // does not get to a new line while the broken string does due to the added
-  // parentheses. The formatter does not do it in one pass.
-  EXPECT_EQ(
-      "x = ('xxxxxxxx ' +\n"
-      "     'xxxxxx')[0];",
-      format("x = 'xxxxxxxx xxxxxx'[0];", getGoogleJSStyleWithColumns(18)));
-  verifyFormat("x =\n"
-               "    ('xxxxxxxx ' +\n"
-               "     'xxxxxx')[0];",
-               getGoogleJSStyleWithColumns(18));
-  // Breaking of template strings and regular expressions is not implemented.
-  verifyFormat("var literal =\n"
-               "    `xxxxxxxx xxxxxxxx`;",
-               getGoogleJSStyleWithColumns(18));
-  verifyFormat("var literal =\n"
-               "    /xxxxxxxx xxxxxxxx/;",
-               getGoogleJSStyleWithColumns(18));
-  // There can be breaks in the code inside a template string.
-  verifyFormat("var literal = `xxxxxx ${\n"
-               "    xxxxxxxxxx} xxxxxx`;",
-               "var literal = `xxxxxx ${xxxxxxxxxx} xxxxxx`;",
-               getGoogleJSStyleWithColumns(14));
-  verifyFormat("var literal = `xxxxxx ${\n"
-               "    xxxxxxxxxx} xxxxxx`;",
-               "var literal = `xxxxxx ${xxxxxxxxxx} xxxxxx`;",
-               getGoogleJSStyleWithColumns(15));
-  // Identifiers inside the code inside a template string should not be broken
-  // even if the column limit is exceeded.  This following behavior is not
-  // optimal.  The part after the closing brace which exceeds the column limit
-  // can be put on a new line.  Change this test when it is implemented.
-  verifyFormat("var literal = `xxxxxx ${\n"
-               "    xxxxxxxxxxxxxxxxxxxxxx} xxxxxx`;",
-               "var literal = `xxxxxx ${xxxxxxxxxxxxxxxxxxxxxx} xxxxxx`;",
-               getGoogleJSStyleWithColumns(14));
-  verifyFormat("var literal = `xxxxxx ${\n"
-               "    xxxxxxxxxxxxxxxxxxxxxx +\n"
-               "    xxxxxxxxxxxxxxxxxxxxxx} xxxxxx`;",
-               "var literal = `xxxxxx ${xxxxxxxxxxxxxxxxxxxxxx + "
-               "xxxxxxxxxxxxxxxxxxxxxx} xxxxxx`;",
-               getGoogleJSStyleWithColumns(14));
-
-  // Strings in a TypeScript type declaration can't be broken.
-  verifyFormat("type x =\n"
-               "    'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx';",
-               getGoogleJSStyleWithColumns(20));
-  verifyFormat("/* type */ type x =\n"
-               "    'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx';",
-               getGoogleJSStyleWithColumns(20));
-  verifyFormat("export type x =\n"
-               "    'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx';",
-               getGoogleJSStyleWithColumns(20));
-  // Dictionary keys can't be broken. Values can be broken.
-  verifyFormat("var w = {\n"
-               "  'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx':\n"
-               "      'xxxxxxxxxx' +\n"
-               "      'xxxxxxxxxx' +\n"
-               "      'xxxxxxxxxx' +\n"
-               "      'xxxxxxxxxx' +\n"
-               "      'xxx',\n"
-               "};",
-               "var w = {\n"
-               "  'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx':\n"
-               "      'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',\n"
-               "};",
-               getGoogleJSStyleWithColumns(20));
 }
 
 TEST_F(FormatTestJS, RegexLiteralClassification) {

>From 84eec1c60ca4760bf3c25133fc2eb4157970c305 Mon Sep 17 00:00:00 2001
From: sstwcw <su3e8a96kzlver at posteo.net>
Date: Fri, 15 Sep 2023 12:35:32 +0000
Subject: [PATCH 2/2] Address comments

---
 clang/include/clang/Format/Format.h       | 2 +-
 clang/lib/Format/ContinuationIndenter.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 3bf70838d059086..e78aa62f93cffe1 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2074,7 +2074,7 @@ struct FormatStyle {
   ///        "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
   /// \endcode
   ///
-  /// C# and JavaScript interpolated strings are not broken.
+  /// C# interpolated strings are not broken.
   ///
   /// In Verilog:
   /// \code
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 15b9ad41a37af59..17567572f030542 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -2237,7 +2237,7 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current,
                                            LineState &State, bool AllowBreak) {
   unsigned StartColumn = State.Column - Current.ColumnWidth;
   if (Current.isStringLiteral()) {
-    // Strings in JSON can not be broken. Breaking strings in JavaScript is
+    // Strings in JSON cannot be broken. Breaking strings in JavaScript is
     // disabled for now.
     if (Style.isJson() || Style.isJavaScript() || !Style.BreakStringLiterals ||
         !AllowBreak) {



More information about the cfe-commits mailing list