Skip to content
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

Jump to bottom

clang-format adds spurious binding to JSON file with DisableFormat: true #52705

Closed
Andrew-William-Smith opened this issue Dec 15, 2021 · 1 comment
Closed

clang-format adds spurious binding to JSON file with DisableFormat: true #52705

Andrew-William-Smith opened this issue Dec 15, 2021 · 1 comment
Labels
bug Indicates an unexpected problem or unintended behavior clang-format good first issue https://github.com/llvm/llvm-project/contribute

Comments

@Andrew-William-Smith
Copy link
Contributor

Andrew-William-Smith commented Dec 15, 2021 •
edited
Loading

With clang-format built from the head of main and the following .clang-format file, which should fully disable formatting for JSON files:

---
Language: Json
DisableFormat: true

The following file:

{
  "key": "value"
}

Is reformatted as follows, with a spurious binding that results in invalid JSON:

x = {
  "key": "value"
}

With DisableFormat: false or no .clang-format file at all, the binding is not added to the file.

Research note: This issue is a result of the way that clang-format implements JSON formatting by adding a binding to the top-level object in order to force clang::format to treat the code as plain JavaScript. I have already tracked down the issue and am currently preparing a patch.
@Andrew-William-Smith
Copy link
Contributor Author

Here's an initial diff that fixes this issue: https://reviews.llvm.org/D115769

@mydeveloperday mydeveloperday added bug Indicates an unexpected problem or unintended behavior good first issue https://github.com/llvm/llvm-project/contribute and removed new issue labels Dec 15, 2021
@mydeveloperday mydeveloperday added the awaiting-review Has pending Phabricator review label Dec 15, 2021
Andrew-William-Smith added a commit to Andrew-William-Smith/llvm-project that referenced this issue Dec 15, 2021
@Andrew-William-Smith
[clang-format] Remove spurious JSON binding when DisableFormat: true
fb8988e 
Relevant issue: llvm#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:
```json
{
  "key": "value"
}
```
Is reformatted to:
```javascript
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.

Differential Revision: https://reviews.llvm.org/D115769
@asl asl removed the awaiting-review Has pending Phabricator review label Jan 27, 2022
@JohnC32 JohnC32 mentioned this issue Apr 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior clang-format good first issue https://github.com/llvm/llvm-project/contribute
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@asl @mkurdej @Andrew-William-Smith @mydeveloperday