Exclude function doc strings from external-path and canonical-repository lints

[Silic0nS0ldier]

Addresses some feedback in #1386 (comment)

This is not exhaustive, the following are not handled;

• File doc strings.
  Could be simplified to skipping any triple-quote string that is unassigned.
• doc attributes on rule, macro, repository_rule, attr.___, etc.
  Or for sake of simplicity, any named argument assignment for doc.
• Provider doc strings.

external-path lint doc has also been updated with examples are more illustrative of what may be found in the wild (srcs and data are normally label_list attributes which are immune to the problems external-path is trying to guard against).

[gemini-code-assist]

Summary of Changes

Hello @Silic0nS0ldier, 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 refines the behavior of the external-path and canonical-repository lints by ensuring they do not flag strings found within function docstrings. This change improves the accuracy of the lints by avoiding unnecessary warnings in documentation, making the linter more intelligent about context. Additionally, the documentation for the external-path lint has been updated with clearer examples to better reflect real-world scenarios.

Highlights

• Lint Exclusion for Doc Strings: Function doc strings are now excluded from triggering external-path and canonical-repository lints, preventing false positives in documentation.
• Updated Lint Documentation: The external-path lint documentation in WARNINGS.md has been updated with more relevant and illustrative examples.
• New Helper Function: A new helper function, isDocStringContext, has been introduced in warn/warn_bazel.go to identify if a string expression is part of a function doc string.
• Enhanced Test Coverage: New test cases have been added to warn/warn_bazel_test.go to specifically verify that doc strings do not trigger the external-path and canonical-repository warnings.

Changelog

• WARNINGS.md
  • Updated examples for the external-path lint to use copts and args instead of srcs and data, providing more illustrative cases for the warning.
• warn/warn_bazel.go
  • Added a new function isDocStringContext to determine if a given expression stack is within a function doc string context.
  • Modified externalPathWarning to skip linting strings identified as function doc strings using isDocStringContext.
  • Modified canonicalRepositoryWarning to skip linting strings identified as function doc strings using isDocStringContext.
• warn/warn_bazel_test.go
  • Added new test cases for external-path to ensure that strings within function doc strings do not trigger warnings.
  • Added new test cases for canonical-repository to ensure that strings within function doc strings do not trigger warnings.

Activity

• This pull request addresses specific feedback received on a previous pull request (#1386).
• The author notes that while this change handles function doc strings, it does not yet cover file doc strings, doc attributes on rules/macros, or provider doc strings, indicating potential future enhancements.

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 aims to prevent external-path and canonical-repository lints from firing on function docstrings. The changes include updating the linter logic to recognize docstring contexts and adding corresponding test cases. The documentation for external-path is also improved with more relevant examples.

My main feedback is regarding the implementation of isDocStringContext, which is a bit too broad and could lead to missed lint errors. I've provided a detailed comment with a suggested fix that not only corrects the logic but also extends it to handle file-level docstrings. Overall, this is a valuable improvement, and with the suggested change, it will be even more robust.

[gemini-code-assist]

Code Review

This pull request aims to exclude function docstrings from external-path and canonical-repository lints. The approach is to add a new helper isDocStringContext to detect when a string is a docstring and skip the linting. The changes also include updated examples in WARNINGS.md and new tests.

The implementation of isDocStringContext is slightly incorrect as it considers any top-level string in a function as a docstring, not just the first one. I've suggested a fix to make this check more precise. With that change, this PR looks good.

[Silic0nS0ldier]

/gemini review

[gemini-code-assist]

Code Review

This pull request aims to prevent external-path and canonical-repository lints from firing on function docstrings. The documentation for external-path is also improved with better examples. The core logic for identifying docstrings is a good start, but it's currently too broad and will incorrectly treat any triple-quoted string inside a function as a docstring. My review provides a fix to make this check more precise, ensuring only actual docstrings are ignored by the linter.

[Silic0nS0ldier]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively addresses the issue of false positives from external-path and canonical-repository lints within function docstrings. The introduction of the isDocStringContext helper function provides a clean and targeted way to identify function docstrings, ensuring they are correctly excluded from these specific lint checks. The implementation is robust, correctly handling cases where a string is not a docstring (i.e., not the first statement in a function). The accompanying test updates are thorough, covering both the cases that should be ignored and those that should still trigger a warning. Additionally, the update to WARNINGS.md with more illustrative examples for the external-path warning is a valuable improvement to the documentation. The changes are well-implemented and improve the linter's accuracy.