Allowing to disable buildozer on specific targets via #buildozer: leave-alone

[oreflow]

• Moving inherited comments to parsing instead of live-checks (note, this will miss comments added in the same change)
• Moving comment parsing to separate file, reused by both Buildifier and Buildozer
• Adding logic for Buildozer to ignore making changes to exprs tagged with #buildozer: leave-alone comments

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the parsing and manipulation of BUILD files by introducing a new mechanism for comment inheritance within the AST and centralizing comment-related utilities. The primary impact is the ability to mark specific expressions with a #buildozer: leave-alone comment, which will prevent buildozer from making any changes to those elements. This provides users with greater control over automated tooling, allowing them to protect critical or intentionally formatted code sections from unintended modifications.

Highlights

• Comment Inheritance in AST: Introduced an Inherited field to the Comments struct in the Abstract Syntax Tree (AST), allowing comments from parent expressions to be propagated to child expressions during parsing. This enables more robust comment-based directives.
• Centralized Comment Checking Logic: Moved and centralized comment checking functionality into a new comments.go file, providing a HasCommentContaining utility function. This function efficiently checks for specific comment prefixes, including those inherited from parent nodes, for both Buildifier and Buildozer.
• Buildozer 'leave-alone' Directive: Implemented a new buildozer: leave-alone comment directive. Expressions tagged with this comment will now be ignored by Buildozer, preventing it from modifying or deleting them. This provides fine-grained control over automated formatting and refactoring.
• Updated Buildifier Rewrite Functions: Refactored Buildifier's rewrite functions (leaveAlone, doNotSort, keepSorted) to utilize the new HasCommentContaining utility and rewriteComment constants, streamlining comment-based logic.
• Comprehensive Testing: Added extensive test cases for the new comment inheritance mechanism and the buildozer: leave-alone functionality, ensuring correct behavior for various scenarios including rule deletion, attribute setting, and empty package handling.

Changelog

• build/BUILD.bazel
  • Added comments.go and comments_test.go to the go_library and go_test source lists respectively.
• build/comments.go
  • Added new file comments.go containing checkSelfOrInheritedComment and HasCommentContaining functions for centralized comment checking.
• build/comments_test.go
  • Added new file comments_test.go with unit tests for the HasCommentContaining function, covering various comment placements and expression types.
• build/lex.go
  • Introduced assignInheritedComments function to populate the Inherited field of Comments for expressions based on parent comments.
  • Called assignInheritedComments within the main assignComments function to enable comment inheritance during parsing.
• build/parse_test.go
  • Added a new test case to parseTests to verify correct inheritance of suffix comments by child expressions.
• build/rewrite.go
  • Defined rewriteComment type and constants for various comment directives, including rewriteCommentLeaveAlone.
  • Refactored leaveAlone, doNotSort, and keepSorted functions to use the new HasCommentContaining utility.
  • Removed deprecated hasComment and leaveAlone1 functions.
  • Updated calls to comment-checking functions across the file to use the new HasCommentContaining.
• build/syntax.go
  • Added Inherited []Comment field to the Comments struct to store comments inherited from parent expressions.
  • Corrected comment style for Copy methods from //Copy to // Copy.
• buildozer/buildozer_test.sh
  • Added new shell tests (test_buildozer_leave_alone_delete_is_not_deleted, test_buildozer_leave_alone_empty_package_is_not_deleted, test_buildozer_leave_alone_set_is_not_updated, test_buildozer_leave_alone_set_targeting_leave_alone_returns_error) to validate the buildozer: leave-alone functionality.
• edit/buildozer.go
  • Defined BuildozerComment type and BuildozerCommentLeaveAlone constant.
  • Modified filterRules to skip rules marked with BuildozerCommentLeaveAlone.
• edit/edit.go
  • Modified RemoveEmptyPackage and RemoveEmptyUseRepoCalls to prevent removal of statements marked with BuildozerCommentLeaveAlone.
  • Updated IndexOfRuleByName to exclude rules marked with BuildozerCommentLeaveAlone from search results.
• edit/edit_test.go
  • Added a test case to TestIndexOfRuleByName to confirm that rules with buildozer: leave-alone are not found.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a useful feature to disable buildozer on specific targets using a #buildozer: leave-alone comment. The implementation correctly filters out tagged rules from modifications. The changes also include a significant and valuable refactoring of comment handling, moving comment inheritance logic into the parsing phase and centralizing it in a new comments.go file. This improves code clarity and maintainability.

I have a suggestion regarding the new comment inheritance logic to improve its clarity and ensure consistency with intended behavior.

[gemini-code-assist]

The condition suffixComment.Start.Line <= eStart.Line || suffixComment.Start.Line <= eEnd.Line is redundant and can be simplified to suffixComment.Start.Line <= eEnd.Line.

However, the existing comment on line 919 states that suffix comments are inherited for expressions that overlap with the comment line number. An overlap check would be eStart.Line <= suffixComment.Start.Line && suffixComment.Start.Line <= eEnd.Line.

This creates a discrepancy between the code's logic and its documentation. Please clarify the intended behavior. If the 'overlap' behavior (as described in the existing comment) was the intentional previous behavior, the code should be adjusted to implement that. If the suffixComment.Start.Line <= eEnd.Line behavior is the intended one, then the code can be simplified as noted, and the comment on line 919 should be updated to reflect this logic.

Please ensure the chosen behavior is consistent with previous intentional behavior.

References

1. When modifying code, ensure that changes are consistent with previous behavior, especially if the previous behavior was intentional.