Skip to content

Disable bzl-visibility lint if the target file has a visibility() declaration#1439

Merged
oreflow merged 3 commits intobazelbuild:mainfrom
haberman:warn-visibility
Jan 19, 2026
Merged

Disable bzl-visibility lint if the target file has a visibility() declaration#1439
oreflow merged 3 commits intobazelbuild:mainfrom
haberman:warn-visibility

Conversation

@haberman
Copy link
Contributor

@haberman haberman commented Jan 14, 2026

The official docs for the bzl-visibility lint say:

This lint predates the load visibility feature and is unnecessary in workspaces where .bzl files declare visibilities.

This statement is correct; load visibility is a strictly superior mechanism, because it is more explicit and more strictly enforced. All code should move towards visibility() and away from this legacy private/ naming scheme.

However, this lint currently fires even in cases where visibility has been explicitly specified. This is unfortunate, because the usage may be intended, in which case the warning is pure noise. The only way to disable the warning in that case is to put the following comment on the code performing the import:

# Currently buildifier forces me to put the following comment on the load() statement
# even if foo.bzl has added me to its visibility list:

load("//some/package/private:foo.bzl", "some_symbol") # buildifier: disable=bzl-visibility

If foo.bzl has explicitly added the caller to its visibility() list, this extra comment is pure noise that adds no value. Worse, it's likely to get copied and pasted around, whether or not it is really needed.

We should fix this by making the warning notice that the target file has an explicit visibility() definition. If so, there is no need to emit the legacy warning.

MARKDOWN=true

@haberman
Disable bzl-visibility lint if the target file has a visibility() d…
003ef26 
…eclaration.

The official docs for this lint say:

> This lint predates the load visibility feature and is unnecessary in workspaces where .bzl files declare visibilities.

However the lint can sometimes get in the way, by warning about instances where the usage is actually intended.

Since `visibility()` is more explicit, and more strictly enforced, there is no need for the lint to return any warnings if the target file has a visibility declaration.

MARKDOWN=true
@haberman haberman requested a review from a team as a code owner January 14, 2026 22:01
@haberman haberman requested review from oreflow and removed request for a team January 14, 2026 22:01
@gemini-code-assist
Copy link
Contributor

gemini-code-assist bot commented Jan 14, 2026

Summary of Changes

Hello @haberman, I'm Gemini Code Assist1! 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 bzl-visibility lint within buildifier. The primary goal is to eliminate redundant warnings that occur when .bzl files explicitly define their visibility using the visibility() function, which is the recommended and more robust mechanism. By checking for these explicit declarations, the lint becomes smarter, reducing noise and improving the developer experience for those adopting modern Bazel visibility controls.

Highlights

  • Lint Relocation: The bzl-visibility lint has been moved from FileWarningMap to MultiFileWarningMap in warn/warn.go. This change signifies that the lint now requires context from multiple files to operate, specifically to check the visibility declarations of loaded .bzl files.
  • New Visibility Check Function: A new function, hasVisibilityStatement, has been introduced in warn/warn_visibility.go. This function determines if a target .bzl file explicitly declares its visibility using a visibility() statement, which is a more explicit and strongly-enforced mechanism than the legacy private/ naming scheme.
  • Conditional Lint Suppression: The bzlVisibilityWarning function has been updated to utilize the new hasVisibilityStatement function. If the loaded .bzl file contains an explicit visibility() declaration, the bzl-visibility warning will now be suppressed, preventing unnecessary noise for users who are already adhering to modern visibility practices.
  • New Test Case: A new test case, TestBzlVisibilityWithVisibilityStatement, has been added to warn/warn_visibility_test.go. This test specifically verifies that the bzl-visibility warning is not issued when the loaded .bzl module explicitly defines its visibility.
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[bot]
gemini-code-assist bot reviewed Jan 14, 2026
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly implements the logic to disable the bzl-visibility lint when a visibility() declaration is present in the target file. The change to move the lint from FileWarningMap to MultiFileWarningMap is appropriate, as it now requires reading other files. The added test case is also good for verifying the new behavior. However, I've found a critical issue in the implementation that could lead to a panic due to an unsafe type assertion. Please address this to ensure the stability of the linter.

@haberman @gemini-code-assist
Addressed edge case where call function is not an ident.
02bc1da 
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
oreflow
oreflow reviewed Jan 16, 2026
View reviewed changes
oreflow
oreflow approved these changes Jan 16, 2026
View reviewed changes
Copy link
Collaborator

@oreflow oreflow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just the mentioned note on that it would be great to have a test case where the file exists but does not have a visibility statement

@haberman
Copy link
Contributor Author

haberman commented Jan 16, 2026

Thanks for the review! PTAL.

@oreflow oreflow merged commit 9bdafcf into bazelbuild:main Jan 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oreflow oreflow oreflow approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@haberman @oreflow