[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