Author: Andrew Smith Date: 2021-12-15T23:09:28Z New Revision: 63a565768e8fdbb3260a88f584aa2c11a58fea93
URL: https://github.com/llvm/llvm-project/commit/63a565768e8fdbb3260a88f584aa2c11a58fea93 DIFF: https://github.com/llvm/llvm-project/commit/63a565768e8fdbb3260a88f584aa2c11a58fea93.diff LOG: [clang-format] Remove spurious JSON binding when DisableFormat = true Relevant issue: https://github.com/llvm/llvm-project/issues/52705 When the `DisableFormat` option of `clang-format` is set to `true` and a JSON file is formatted, the ephemeral variable binding that is added to the top-level object is not removed from the formatted file. For example, this JSON: ``` { "key": "value" } ``` Is reformatted to: ``` x = { "key": "value" } ``` Which is not valid JSON syntax. This fix avoids the addition of this binding when `DisableFormat` is set to `true`, ensuring that it cannot be left behind when formatting is disabled. Reviewed By: MyDeveloperDay, HazardyKnusperkeks Differential Revision: https://reviews.llvm.org/D115769 Fixes #52705 Added: Modified: clang/tools/clang-format/ClangFormat.cpp clang/unittests/Format/FormatTestJson.cpp Removed: ################################################################################ diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp index 56dc628869a4d..893c17d917082 100644 --- a/clang/tools/clang-format/ClangFormat.cpp +++ b/clang/tools/clang-format/ClangFormat.cpp @@ -460,7 +460,7 @@ static bool format(StringRef FileName) { // To format JSON insert a variable to trick the code into thinking its // JavaScript. - if (FormatStyle->isJson()) { + if (FormatStyle->isJson() && !FormatStyle->DisableFormat) { auto Err = Replaces.add(tooling::Replacement( tooling::Replacement(AssumedFileName, 0, 0, "x = "))); if (Err) { diff --git a/clang/unittests/Format/FormatTestJson.cpp b/clang/unittests/Format/FormatTestJson.cpp index a1680d80b92b2..14a48182f9dfe 100644 --- a/clang/unittests/Format/FormatTestJson.cpp +++ b/clang/unittests/Format/FormatTestJson.cpp @@ -27,7 +27,7 @@ class FormatTestJson : public ::testing::Test { // Mock up what ClangFormat.cpp will do for JSON by adding a variable // to trick JSON into being JavaScript - if (Style.isJson()) { + if (Style.isJson() && !Style.DisableFormat) { auto Err = Replaces.add( tooling::Replacement(tooling::Replacement("", 0, 0, "x = "))); if (Err) { @@ -60,10 +60,15 @@ class FormatTestJson : public ::testing::Test { return Style; } + static void verifyFormatStable(llvm::StringRef Code, + const FormatStyle &Style) { + EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable"; + } + static void verifyFormat(llvm::StringRef Code, const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Json)) { - EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable"; + verifyFormatStable(Code, Style); EXPECT_EQ(Code.str(), format(test::messUp(Code), Style)); } }; @@ -193,5 +198,24 @@ TEST_F(FormatTestJson, JsonNoStringSplit) { Style); } +TEST_F(FormatTestJson, DisableJsonFormat) { + FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json); + verifyFormatStable("{}", Style); + verifyFormatStable("{\n" + " \"name\": 1\n" + "}", + Style); + + // Since we have to disable formatting to run this test, we shall refrain from + // calling test::messUp lest we change the unformatted code and cannot format + // it back to how it started. + Style.DisableFormat = true; + verifyFormatStable("{}", Style); + verifyFormatStable("{\n" + " \"name\": 1\n" + "}", + Style); +} + } // namespace format } // end namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
