maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Andrew Smith 2021-12-15 23:08:33 +00:00 committed by mydeveloperday
parent a94aab12a4
commit 63a565768e
2 changed files with 27 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
clang/tools/clang-format/ClangFormat.cpp
View File

@ -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) {

28
clang/unittests/Format/FormatTestJson.cpp
View File

@ -27,7 +27,7 @@ protected:
// 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 @@ protected:
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