search

bazelbuild / intellij

IntelliJ plugin for Bazel projects
https://ij.bazel.build/
Apache License 2.0
761 stars 304 forks source link

Formatting buildfiles from IntelliJ can yield incorrect results #4496

Closed Duhemm closed 1 year ago

Duhemm commented 1 year ago

Description of the bug:

When formatting a file, buildifier is called without passing the --path argument. Because buildifier will receive the content of the file to format via stdin, buildifier cannot determine to which package the file being formatted belongs.

Unfortunately, this can lead to unexpected results during formatting. For instance, if StripLabelLeadingSlashes and ShortenAbsoluteLabelsToRelative are enabled, then the absolute label of a target at the root of the workspace such as //:label will be rewritten to the relative label :label, which has a different meaning if the build file being formatted is not at the root of the workspace.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

  1. Install buildifier
  2. Clone https://github.com/Duhemm/buildifier-repro and import in IntelliJ
  3. Edit my-buildifier to make sure it points to your install of buildifier
  4. Open subpkg/BUILD
  5. Run "Reformat code"
  6. Notice the actual label changed from actual = "//:example", to actual = ":example",.

Which Intellij IDE are you using? Please provide the specific version.

IntelliJ IDEA 2022.3.1 (Community Edition) Build #IC-223.8214.52

What programming languages and tools are you using? Please provide specific versions.

No response

What Bazel plugin version are you using?

2022.12.13.0.1-api-version-223

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

Duhemm commented 1 year ago

I have a fix available in https://github.com/bazelbuild/intellij/compare/master...Duhemm:intellij:buildifier-path?expand=1.

I'd be happy to submit a PR if the maintainers are okay with it.

mai93 commented 1 year ago

Thank you for reporting this and for the fix, I noticed that the problem still happens on file save (with your fix patched). I think we need to pass the --path also here which is called by the onSaveHandler. I'm also looking into refactoring this part so that buildifier invocations are done from a single place to not run in similar cases like this.

Duhemm commented 1 year ago

Thank you for taking a look @mai93. You're right, we also need to pass --path when saving. I've updated the patch and opened a pull request in #4550 .