Add support for gazelle's `default_visibility` directive

[dougthor42]

🚀 feature request

Relevant Rules

• gazelle

Description

Python's gazelle doesn't currently support the default_visibility directive - it's hard-coded as https://github.com/bazelbuild/rules_python/blob/3f40e98d3901068aa25fa7ff2573f6bdb5b1fe81/gazelle/python/generate.go#L215

If you try and use it, you get this error:

gazelle: /c/dev/bazel-python-src-tests-example/BUILD: unknown directive: gazelle:default_visibility

Support for such should be added.

Describe the solution you'd like

Implement default_visibility 🙃. The same as how build-gazelle defines it:

Comma-separated list of visibility specifications. This directive adds the visibility specifications for this and descendant packages.

There are 4 questions that I've been able to come up with. I elaborate on them below.

1. How to handle python_root?
2. Should the directive be specific to just python?
3. Should the current default be kept?
4. Should we support "clearing/resetting" visibility?

Open Question 1: How to handle python_root?

rules_python's gazelle supports the python_root directive, which modifies the default visibility: https://github.com/bazelbuild/rules_python/blob/3f40e98d3901068aa25fa7ff2573f6bdb5b1fe81/gazelle/python/generate.go#L215

Do we want to support users injecting python_root or do we want them to hard code things?

Examples:

# Example of users injecting python_root
# gazelle:default_visibility //$python_root:__pkg__,//bar:__pkg__,//$python_root/foo:__subpackages

# assuming python_root = "./src", renders targets as:
py_library(
    ...,
    visibility = [
        "//bar:__pkg__",  # ordered alphabetically
        "//src:__pkg__",
        "//src/foo:__subpackages__",
    ],
    ...,
)

How would we support such injection? The above example is a linux var style, but we could also do %s formatting or {{ python_root }} templating or something else.

Or we could just say "Do it yourself, users!":

# Example of users injecting python_root
# gazelle:default_visibility //src:__pkg__,//bar:__pkg__,//src/foo:__subpackages

Open Question 2: Specific to just python?

I see 3 paths forward:

1. Add only default_visibility
   • Same as the core bazel-gazelle project, affects all targets.
2. Add only python_default_visibility
   • Only affects py_* targets.
3. Add both default_visibility and python_default_visibility.
   • allows more customization and would be beneficial for multi-language projects, but is more complex for both gazelle code and user BUILD files.
   • python_default_visibility overrides default_visibility for py_* targets

Which should we strive for?

Open Question 3: Should the current default be kept?

Should the current default visivility be kept as fmt.Sprintf("//%s:__subpackages__", pythonProjectRoot)?

If not, what should it change to?

Open Question 4: Support for "clearing/resetting" visibility?

Do we want to add the ability to clear/reset visibility? Perhaps by supporting special labels NONE and DEFAULT?

Example:

# No directive set, use the default. Assuming that python_root is unset.
py_library(
    visibility = ["//:__subpackages__"],
)

# Set default vis.
# gazelle:default_visibility //src:__pkg__
py_library(
    visibility = ["//src:__pkg__"],
)

go_library(
    visibility = ["//src:__pkg__"],
)

# python_default_visibility overrides default_visibility for py_* targets only
# gazelle:python_default_visibility //foo:bar,//tests:baz
py_library(
    visibility = [
        "//foo:bar",
        "//tests:baz",
    ],
)

go_library(
    visibility = ["//src:__pkg__"],
)

# Here's the "reset"
# gazelle:python_default_visibility DEFAULT
py_library(
    visibility = ["//:__subpackages__"],
)

# gazelle:python_default_visibility NONE
py_library(
    visibility = [],  # in reality, this attribute would just be missing altogether
)

Describe alternatives you've considered

N/A

[dougthor42]

I've opened #1784 for this, making the following assumptions:

1. How to handle python_root? Allow users to manually inject it via special string
2. Should the directive be specific to just python? Just python
3. Should the current default be kept? Yes
4. Should we support "clearing/resetting" visibility? Yes

[dougthor42]

Fixed in #1784 and #1787.