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.

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

Add rule.AttrAssignExpr function #1390

Closed
wants to merge 4 commits into from

Conversation

pcj
Copy link
Contributor

@pcj pcj commented Dec 9, 2022

What type of PR is this?

Feature

What package or component does this PR mostly affect?

all

What does this PR do? Why is it needed?

Allows programmatic access to the assignment expressions of a rule.

Which issues(s) does this PR fix?

Fixes #1389

@achew22
Copy link
Member

achew22 commented Dec 9, 2022

Seems like a reasonable path forward. Could you explain why defining directives as a part of your plugin doesn't achieve what you want? Maybe there is a control surface that we should be adding to Gazelle instead of pushing the complexity on plugin implementors

@pcj
Copy link
Contributor Author

pcj commented Dec 9, 2022

@achew22 directives are fine way of reading configuration data from users in the build file.

However, if plugin authors want to add comments to generated rules, they have limited options: Use the rule.AddComment() API, which makes it really hard to avoid duplicate comments when the rule already exists (and they run gazelle multiple times), or put it on the RHS of the assigment, which is where nobody ever actually writes comments.

Moreover, when you add comments to the RHS of an expression, the formatted output from gazelle differs from buildifier. If a repo has a buildifier check it will fail from the freshly generated gazelle output:

foo(
    srcs = 
    # what gazelle outputs
   []
)
foo(
    srcs = 
+     # what buildifier reformats to
+    []
)

With this PR, it's more natural to add comments:

foo(
+   # ooh so nice
+   srcs = []
-     # no more ugly
-     []
)

@pcj
Copy link
Contributor Author

pcj commented Dec 9, 2022

I could entertain a different function rule.AttrComments(name string) *bzl.Comments if that sounds better.

@pcj
Copy link
Contributor Author

pcj commented Dec 15, 2022

Closing in favor of #1394.

@pcj pcj closed this Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule API: can't access the assigment statements
2 participants