From a56b4c8e3600608069356403058efd0b792d1123 Mon Sep 17 00:00:00 2001 From: Dom Narducci Date: Thu, 13 Oct 2022 15:23:36 -0700 Subject: [PATCH] Add visibility extension to support recursive default_visibility configuration (#783) Under a new /bazel/ subdirectory, this language-agnostic extension allows a directive to control the default_visibility templated out to all BUILD.bazel files in or under the directory. This feature is intended to allow large codebases to define hierarchical visibility defaults. --- .gitignore | 1 + language/BUILD.bazel | 1 + language/bazel/BUILD.bazel | 9 ++++ language/bazel/visibility/BUILD.bazel | 52 ++++++++++++++++++ language/bazel/visibility/config.go | 64 ++++++++++++++++++++++ language/bazel/visibility/lang.go | 55 +++++++++++++++++++ language/bazel/visibility/lang_test.go | 74 ++++++++++++++++++++++++++ language/bazel/visibility/resolve.go | 35 ++++++++++++ 8 files changed, 291 insertions(+) create mode 100644 language/bazel/BUILD.bazel create mode 100644 language/bazel/visibility/BUILD.bazel create mode 100644 language/bazel/visibility/config.go create mode 100644 language/bazel/visibility/lang.go create mode 100644 language/bazel/visibility/lang_test.go create mode 100644 language/bazel/visibility/resolve.go diff --git a/.gitignore b/.gitignore index d9854487f..c2f5a6cda 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ /tests/bcr/bazel-bin /tests/bcr/bazel-out /tests/bcr/bazel-testlogs +.DS_STORE diff --git a/language/BUILD.bazel b/language/BUILD.bazel index 6a7cc6418..e4e7856b1 100644 --- a/language/BUILD.bazel +++ b/language/BUILD.bazel @@ -26,6 +26,7 @@ filegroup( "base.go", "lang.go", "update.go", + "//language/bazel:all_files", "//language/go:all_files", "//language/proto:all_files", ], diff --git a/language/bazel/BUILD.bazel b/language/bazel/BUILD.bazel new file mode 100644 index 000000000..27dcf6417 --- /dev/null +++ b/language/bazel/BUILD.bazel @@ -0,0 +1,9 @@ +filegroup( + name = "all_files", + testonly = True, + srcs = [ + "BUILD.bazel", + "//language/bazel/visibility:all_files", + ], + visibility = ["//visibility:public"], +) diff --git a/language/bazel/visibility/BUILD.bazel b/language/bazel/visibility/BUILD.bazel new file mode 100644 index 000000000..210e311d7 --- /dev/null +++ b/language/bazel/visibility/BUILD.bazel @@ -0,0 +1,52 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "visibility", + srcs = [ + "config.go", + "lang.go", + "resolve.go", + ], + importpath = "github.com/bazelbuild/bazel-gazelle/language/bazel/visibility", + visibility = ["//visibility:public"], + deps = [ + "//config", + "//label", + "//language", + "//repo", + "//resolve", + "//rule", + ], +) + +alias( + name = "go_default_library", + actual = ":visibility", + visibility = ["//visibility:public"], +) + +go_test( + name = "visibility_test", + srcs = ["lang_test.go"], + deps = [ + ":visibility", + "//config", + "//label", + "//language", + "//rule", + "@com_github_stretchr_testify//require:go_default_library", + ], +) + +filegroup( + name = "all_files", + testonly = True, + srcs = [ + "BUILD.bazel", + "config.go", + "lang.go", + "lang_test.go", + "resolve.go", + ], + visibility = ["//visibility:public"], +) diff --git a/language/bazel/visibility/config.go b/language/bazel/visibility/config.go new file mode 100644 index 000000000..64385b3c3 --- /dev/null +++ b/language/bazel/visibility/config.go @@ -0,0 +1,64 @@ +package visibility + +import ( + "flag" + "strings" + + "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/rule" +) + +const ( + _directiveName = "default_visibility" +) + +type visConfig struct { + visibilityTargets []string +} + +// getVisConfig directly returns the internal configuration struct rather +// than a pointer because we explicitly want pass-by-value symantics so +// configurations down a directory tree don't accidentially update upstream. +func getVisConfig(c *config.Config) visConfig { + cfg := c.Exts[_extName] + if cfg == nil { + return visConfig{} + } + return cfg.(visConfig) +} + +// RegisterFlags noops because we only parameterize behavior with a directive. +func (*visibilityExtension) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config) {} + +// CheckFlags noops because no flags are referenced. +func (*visibilityExtension) CheckFlags(fs *flag.FlagSet, c *config.Config) error { + return nil +} + +// KnownDirectives returns the only directive this extension operates on. +func (*visibilityExtension) KnownDirectives() []string { + return []string{_directiveName} +} + +// Configure identifies the visibility targets from the directive value, if it exists. +// +// To set multiple visibility targets, either multiple directives can be used, or a +// list can be provided with comma-separated values. +func (*visibilityExtension) Configure(c *config.Config, _ string, f *rule.File) { + cfg := getVisConfig(c) + if f == nil { + return + } + + for _, d := range f.Directives { + switch d.Key { + case _directiveName: + for _, target := range strings.Split(d.Value, ",") { + cfg.visibilityTargets = append(cfg.visibilityTargets, target) + } + } + } + c.Exts[_extName] = cfg +} + +// /Configurator embed diff --git a/language/bazel/visibility/lang.go b/language/bazel/visibility/lang.go new file mode 100644 index 000000000..d2ea9020a --- /dev/null +++ b/language/bazel/visibility/lang.go @@ -0,0 +1,55 @@ +package visibility + +import ( + "flag" + "strings" + + "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/language" + "github.com/bazelbuild/bazel-gazelle/rule" +) + +type visibilityExtension struct{} + +// NewLanguage constructs a new language.Language modifying visibility. +func NewLanguage() language.Language { + return &visibilityExtension{} +} + +// Kinds instructs gazelle to match any 'package' rule as BUILD files can only have one. +func (*visibilityExtension) Kinds() map[string]rule.KindInfo { + return map[string]rule.KindInfo{ + "package": { + MatchAny: true, + MergeableAttrs: map[string]bool{ + "default_visibility": true, + }, + }, + } +} + +// Loads noops because there are no imports to add +func (*visibilityExtension) Loads() []rule.LoadInfo { + return nil +} + +// GenerateRules does the hard work of setting the default_visibility if a config exists. +func (*visibilityExtension) GenerateRules(args language.GenerateArgs) language.GenerateResult { + res := language.GenerateResult{} + cfg := getVisConfig(args.Config) + + if len(cfg.visibilityTargets) == 0 { + return res + } + + r := rule.NewRule("package", "") + r.SetAttr("default_visibility", cfg.visibilityTargets) + + res.Gen = append(res.Gen, r) + // we have to add a nil to Imports because there is length-matching validation with Gen. + res.Imports = append(res.Imports, nil) + return res +} + +// Fix noop because there is nothing out there to fix yet +func (*visibilityExtension) Fix(c *config.Config, f *rule.File) {} diff --git a/language/bazel/visibility/lang_test.go b/language/bazel/visibility/lang_test.go new file mode 100644 index 000000000..a827a63d0 --- /dev/null +++ b/language/bazel/visibility/lang_test.go @@ -0,0 +1,74 @@ +package visibility_test + +import ( + "fmt" + "testing" + + "github.com/bazelbuild/bazel-gazelle/language/bazel/visibility" + "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/label" + "github.com/bazelbuild/bazel-gazelle/language" + "github.com/bazelbuild/bazel-gazelle/rule" + "github.com/stretchr/testify/require" +) + +func TestNoopsBecauseILoveCoverage(t *testing.T) { + ext := visibility.NewLanguage() + + ext.RegisterFlags(nil /* flagset */, "command", nil /* config */) + ext.Resolve(nil /* config */, nil /* RuleIndex */, nil /* RemoteCache */, nil /* Rule */, nil /* imports */, label.New("repo", "pkg", "name")) + ext.Fix(nil /* config */, nil /* File */) + require.Nil(t, ext.CheckFlags(nil /* flagset */, nil /* config */)) + require.Nil(t, ext.Imports(nil /* rule */, nil /* rule */, nil /* file */)) + require.Nil(t, ext.Embeds(nil /* rule */, label.New("repo", "pkg", "name"))) + require.Nil(t, ext.Loads()) + require.NotNil(t, ext.KnownDirectives()) + require.NotNil(t, ext.Name()) +} + +func Test_NoDirective(t *testing.T) { + cfg := config.New() + file := rule.EmptyFile("path", "pkg") + + ext := visibility.NewLanguage() + ext.Configure(cfg, "rel", file) + res := ext.GenerateRules(language.GenerateArgs{Config: cfg}) + + require.Len(t, res.Imports, 0) + require.Len(t, res.Gen, 0) +} + +func Test_NewDirective(t *testing.T) { + testVis := "//src:__subpackages__" + cfg := config.New() + file, err := rule.LoadData("path", "pkg", []byte(fmt.Sprintf("# gazelle:default_visibility %s", testVis))) + require.Nil(t, err) + + ext := visibility.NewLanguage() + ext.Configure(cfg, "rel", file) + res := ext.GenerateRules(language.GenerateArgs{Config: cfg}) + + require.Len(t, res.Gen, 1) + require.Len(t, res.Imports, 1) + require.Len(t, res.Gen[0].AttrStrings("default_visibility"), 1) + require.Equal(t, testVis, res.Gen[0].AttrStrings("default_visibility")[0]) +} + +func Test_ReplacementDirective(t *testing.T) { + testVis := "//src:__subpackages__" + cfg := config.New() + file, err := rule.LoadData("path", "pkg", []byte(fmt.Sprintf(` +# gazelle:default_visibility %s + +package(default_visibility = "//not-src:__subpackages__") +`, testVis))) + require.Nil(t, err) + + ext := visibility.NewLanguage() + ext.Configure(cfg, "rel", file) + res := ext.GenerateRules(language.GenerateArgs{Config: cfg}) + + require.Len(t, res.Gen, 1) + require.Len(t, res.Imports, 1) + require.Len(t, res.Gen[0].AttrStrings("default_visibility"), 1) + require.Equal(t, testVis, res.Gen[0].AttrStrings("default_visibility")[0]) diff --git a/language/bazel/visibility/resolve.go b/language/bazel/visibility/resolve.go new file mode 100644 index 000000000..46fa9c885 --- /dev/null +++ b/language/bazel/visibility/resolve.go @@ -0,0 +1,35 @@ +package visibility + +import ( + "flag" + "strings" + + "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/label" + "github.com/bazelbuild/bazel-gazelle/repo" + "github.com/bazelbuild/bazel-gazelle/resolve" + "github.com/bazelbuild/bazel-gazelle/rule" +) + +const ( + _extName = "visibility_extension" +) + +// Name returns the extension name. +func (*visibilityExtension) Name() string { + return _extName +} + +// Imports noops because no imports are needed to leverage this functionality. +func (*visibilityExtension) Imports(c *config.Config, r *rule.Rule, f *rule.File) []resolve.ImportSpec { + return nil +} + +// Embeds noops because we are not a traditional rule, and cannot be embedded. +func (*visibilityExtension) Embeds(r *rule.Rule, from label.Label) []label.Label { + return nil +} + +// Resolve noops because we don't have deps=[] to resolve. +func (*visibilityExtension) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.RemoteCache, r *rule.Rule, imports interface{}, from label.Label) { +}