Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 15, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a736d84
[clang-format] Disable string breaking in JS for now
sstwcw Sep 14, 2023
84eec1c
Address comments
sstwcw Sep 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions clang/docs/ClangFormatStyleOptions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
C# and JavaScript interpolated strings are not broken.
C# interpolated strings are not broken.

Expand Down
12 changes: 6 additions & 6 deletions clang/include/clang/Format/Format.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2062,19 +2062,19 @@ 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
///
/// C# and JavaScript interpolated strings are not broken.
/// C# interpolated strings are not broken.
///
/// In Verilog:
/// \code
Expand Down
13 changes: 4 additions & 9 deletions clang/lib/Format/ContinuationIndenter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 cannot be broken. Breaking strings in JavaScript is
// disabled for now.
if (Style.isJson() || Style.isJavaScript() || !Style.BreakStringLiterals ||
!AllowBreak) {
return nullptr;
}

Expand Down
114 changes: 2 additions & 112 deletions clang/unittests/Format/FormatTestJS.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down