From: Krasimir Georgiev Date: Thu, 25 Oct 2018 07:39:30 +0000 (+0000) Subject: [clang-format] Break before next parameter after a formatted multiline raw string... X-Git-Tag: llvmorg-8.0.0-rc1~5826 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=128fcffb062789a77d65ce2966ffa0ef718e3d47;p=platform%2Fupstream%2Fllvm.git [clang-format] Break before next parameter after a formatted multiline raw string parameter Summary: Currently clang-format breaks before the next parameter after multiline parameters (also recursively for the parent expressions of multiline parameters). However, it fails to do so for formatted multiline raw string literals: ``` $ cat test.cc // Examples // Regular multiline tokens int x = f(R"(multi line)", 2); } int y = g(h(R"(multi line)"), 2); // Formatted multiline tokens int z = f(R"pb(multi: 1 # line: 2)pb", 2); int w = g(h(R"pb(multi: 1 # line: 2)pb"), 2); $ clang-format -style=google test.cc // Examples // Regular multiline tokens int x = f(R"(multi line)", 2); } int y = g(h(R"(multi line)"), 2); // Formatted multiline tokens int z = f(R"pb(multi: 1 # line: 2)pb", 2); int w = g(h(R"pb(multi: 1 # line: 2)pb"), 2); ``` This patch addresses this inconsistency by forcing breaking after multiline formatted raw string literals. This requires a little tweak to the indentation chosen for the contents of a formatted raw string literal: in case when that's a parameter and not the last one, the indentation is based off of the uniform indentation of all of the parameters. Reviewers: sammccall Reviewed By: sammccall Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D52448 llvm-svn: 345242 --- diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index c9c9645..89d346d 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -1502,10 +1502,25 @@ unsigned ContinuationIndenter::reformatRawStringLiteral( // violate the rectangle rule and visually flows within the surrounding // source. bool ContentStartsOnNewline = Current.TokenText[OldPrefixSize] == '\n'; - unsigned NextStartColumn = - ContentStartsOnNewline - ? State.Stack.back().NestedBlockIndent + Style.IndentWidth - : FirstStartColumn; + // If this token is the last parameter (checked by looking if it's followed by + // `)`, the base the indent off the line's nested block indent. Otherwise, + // base the indent off the arguments indent, so we can achieve: + // fffffffffff(1, 2, 3, R"pb( + // key1: 1 # + // key2: 2)pb"); + // + // fffffffffff(1, 2, 3, + // R"pb( + // key1: 1 # + // key2: 2 + // )pb", + // 5); + unsigned CurrentIndent = (Current.Next && Current.Next->is(tok::r_paren)) + ? State.Stack.back().NestedBlockIndent + : State.Stack.back().Indent; + unsigned NextStartColumn = ContentStartsOnNewline + ? CurrentIndent + Style.IndentWidth + : FirstStartColumn; // The last start column is the column the raw string suffix starts if it is // put on a newline. @@ -1517,7 +1532,7 @@ unsigned ContinuationIndenter::reformatRawStringLiteral( // indent. unsigned LastStartColumn = Current.NewlinesBefore ? FirstStartColumn - NewPrefixSize - : State.Stack.back().NestedBlockIndent; + : CurrentIndent; std::pair Fixes = internal::reformat( RawStringStyle, RawText, {tooling::Range(0, RawText.size())}, @@ -1527,8 +1542,7 @@ unsigned ContinuationIndenter::reformatRawStringLiteral( auto NewCode = applyAllReplacements(RawText, Fixes.first); tooling::Replacements NoFixes; if (!NewCode) { - State.Column += Current.ColumnWidth; - return 0; + return addMultilineToken(Current, State); } if (!DryRun) { if (NewDelimiter != OldDelimiter) { @@ -1577,6 +1591,13 @@ unsigned ContinuationIndenter::reformatRawStringLiteral( unsigned PrefixExcessCharacters = StartColumn + NewPrefixSize > Style.ColumnLimit ? StartColumn + NewPrefixSize - Style.ColumnLimit : 0; + bool IsMultiline = + ContentStartsOnNewline || (NewCode->find('\n') != std::string::npos); + if (IsMultiline) { + // Break before further function parameters on all levels. + for (unsigned i = 0, e = State.Stack.size(); i != e; ++i) + State.Stack[i].BreakBeforeParameter = true; + } return Fixes.second + PrefixExcessCharacters * Style.PenaltyExcessCharacter; } diff --git a/clang/unittests/Format/FormatTestRawStrings.cpp b/clang/unittests/Format/FormatTestRawStrings.cpp index 3073943..2a8a43d 100644 --- a/clang/unittests/Format/FormatTestRawStrings.cpp +++ b/clang/unittests/Format/FormatTestRawStrings.cpp @@ -576,10 +576,13 @@ auto S = auto S = R"pb(item_1:1 item_2:2)pb"+rest;)test", getRawStringPbStyleWithColumns(40))); + // `rest` fits on the line after )pb", but forced on newline since the raw + // string literal is multiline. expect_eq(R"test( auto S = R"pb(item_1: 1, item_2: 2, - item_3: 3)pb" + rest;)test", + item_3: 3)pb" + + rest;)test", format(R"test( auto S = R"pb(item_1:1,item_2:2,item_3:3)pb"+rest;)test", getRawStringPbStyleWithColumns(40))); @@ -615,7 +618,8 @@ auto S = first+R"pb(item_1:1 item_2:2)pb";)test", expect_eq(R"test( auto S = R"pb(item_1: 1, item_2: 2, - item_3: 3)pb" + rest;)test", + item_3: 3)pb" + + rest;)test", format(R"test( auto S = R"pb(item_1:1,item_2:2,item_3:3)pb"+rest;)test", getRawStringPbStyleWithColumns(40))); @@ -889,6 +893,95 @@ item { Style)); } +TEST_F(FormatTestRawStrings, + BreaksBeforeNextParamAfterMultilineRawStringParam) { + FormatStyle Style = getRawStringPbStyleWithColumns(60); + expect_eq(R"test( +int f() { + int a = g(x, R"pb( + key: 1 # + key: 2 + )pb", + 3, 4); +} +)test", + format(R"test( +int f() { + int a = g(x, R"pb( + key: 1 # + key: 2 + )pb", 3, 4); +} +)test", + Style)); + + // Breaks after a parent of a multiline param. + expect_eq(R"test( +int f() { + int a = g(x, h(R"pb( + key: 1 # + key: 2 + )pb"), + 3, 4); +} +)test", + format(R"test( +int f() { + int a = g(x, h(R"pb( + key: 1 # + key: 2 + )pb"), 3, 4); +} +)test", + Style)); + + expect_eq(R"test( +int f() { + int a = g(x, + h(R"pb( + key: 1 # + key: 2 + )pb", + 2), + 3, 4); +} +)test", + format(R"test( +int f() { + int a = g(x, h(R"pb( + key: 1 # + key: 2 + )pb", 2), 3, 4); +} +)test", + Style)); + // Breaks if formatting introduces a multiline raw string. + expect_eq(R"test( +int f() { + int a = g(x, R"pb(key1: value111111111 + key2: value2222222222)pb", + 3, 4); +} +)test", + format(R"test( +int f() { + int a = g(x, R"pb(key1: value111111111 key2: value2222222222)pb", 3, 4); +} +)test", + Style)); + // Does not force a break after an original multiline param that is + // reformatterd as on single line. + expect_eq(R"test( +int f() { + int a = g(R"pb(key: 1)pb", 2); +})test", + format(R"test( +int f() { + int a = g(R"pb(key: + 1)pb", 2); +})test", Style)); +} + } // end namespace } // end namespace format } // end namespace clang