From 2090e579af470cfad579ef3d1e82a62841fd784b Mon Sep 17 00:00:00 2001 From: Sebastian Birunt Date: Fri, 22 Sep 2023 23:04:08 +0200 Subject: [PATCH] Fix kinds and stmt (#1613) * fix: correct rule's kind & stmt Created rule with `NewRule()` is different to the rule created with `ruleFromExpr()` (when parsing input data). In the first case (`NewRule()`) kind is set to `build.Ident{Name: string}` while it's parsed expression in the second case (`ruleFromExpr()`). For rules packed in a struct like `selects.config_setting_group` there is no way to create/generate the same rule as the one parsed from the file as both will have different kinds. This change unifies kinds for two ways of creating rules and correctly sets the identifier while fixing loads (`FixLoads()`). * chore: fix load tests Change adds test to verify fixing load statements while generating packed rules in struct e.g. `selects.config_setting_group`. * chore: update MODULE.bazel.lock --- internal/go_repository_tools_srcs.bzl | 2 + internal/language/BUILD.bazel | 1 + .../test_load_for_packed_rules/BUILD.bazel | 32 +++++ .../test_load_for_packed_rules/lang.go | 124 ++++++++++++++++++ rule/rule.go | 48 ++++++- tests/BUILD.bazel | 12 +- tests/bcr/MODULE.bazel.lock | 2 +- tests/fix_load_for_packed_rules/BUILD.in | 0 tests/fix_load_for_packed_rules/BUILD.out | 9 ++ tests/fix_load_for_packed_rules/README.md | 3 + tests/fix_load_for_packed_rules/WORKSPACE | 0 tests/fix_load_for_packed_rules/arguments.txt | 0 .../expectedStdout.txt | 0 tests/tools.bzl | 23 ++++ 14 files changed, 252 insertions(+), 4 deletions(-) create mode 100644 internal/language/test_load_for_packed_rules/BUILD.bazel create mode 100644 internal/language/test_load_for_packed_rules/lang.go create mode 100644 tests/fix_load_for_packed_rules/BUILD.in create mode 100644 tests/fix_load_for_packed_rules/BUILD.out create mode 100644 tests/fix_load_for_packed_rules/README.md create mode 100644 tests/fix_load_for_packed_rules/WORKSPACE create mode 100644 tests/fix_load_for_packed_rules/arguments.txt create mode 100644 tests/fix_load_for_packed_rules/expectedStdout.txt create mode 100644 tests/tools.bzl diff --git a/internal/go_repository_tools_srcs.bzl b/internal/go_repository_tools_srcs.bzl index e551c951e..42dc72320 100644 --- a/internal/go_repository_tools_srcs.bzl +++ b/internal/go_repository_tools_srcs.bzl @@ -37,6 +37,8 @@ GO_REPOSITORY_TOOLS_SRCS = [ Label("//internal/language:BUILD.bazel"), Label("//internal/language/test_filegroup:BUILD.bazel"), Label("//internal/language/test_filegroup:lang.go"), + Label("//internal/language/test_load_for_packed_rules:BUILD.bazel"), + Label("//internal/language/test_load_for_packed_rules:lang.go"), Label("//internal/language/test_loads_from_flag:BUILD.bazel"), Label("//internal/language/test_loads_from_flag:lang.go"), Label("//internal:list_repository_tools_srcs.go"), diff --git a/internal/language/BUILD.bazel b/internal/language/BUILD.bazel index 673292554..c98e4a341 100644 --- a/internal/language/BUILD.bazel +++ b/internal/language/BUILD.bazel @@ -4,6 +4,7 @@ filegroup( srcs = [ "BUILD.bazel", "//internal/language/test_filegroup:all_files", + "//internal/language/test_load_for_packed_rules:all_files", "//internal/language/test_loads_from_flag:all_files", ], visibility = ["//visibility:public"], diff --git a/internal/language/test_load_for_packed_rules/BUILD.bazel b/internal/language/test_load_for_packed_rules/BUILD.bazel new file mode 100644 index 000000000..cb832c5b6 --- /dev/null +++ b/internal/language/test_load_for_packed_rules/BUILD.bazel @@ -0,0 +1,32 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "test_load_for_packed_rules", + srcs = ["lang.go"], + importpath = "github.com/bazelbuild/bazel-gazelle/internal/language/test_load_for_packed_rules", + visibility = ["//visibility:public"], + deps = [ + "//config", + "//label", + "//language", + "//repo", + "//resolve", + "//rule", + ], +) + +filegroup( + name = "all_files", + testonly = True, + srcs = [ + "BUILD.bazel", + "lang.go", + ], + visibility = ["//visibility:public"], +) + +alias( + name = "go_default_library", + actual = ":test_load_for_packed_rules", + visibility = ["//:__subpackages__"], +) diff --git a/internal/language/test_load_for_packed_rules/lang.go b/internal/language/test_load_for_packed_rules/lang.go new file mode 100644 index 000000000..fad7774c3 --- /dev/null +++ b/internal/language/test_load_for_packed_rules/lang.go @@ -0,0 +1,124 @@ +/* Copyright 2023 The Bazel Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package `test_load_for_packed_rules` generates packed +// rule of `selects.config_setting_group`. +// +// This extension is experimental and subject to change. It is not included +// in the default Gazelle binary. +package test_load_for_packed_rules + +import ( + "context" + + "github.com/bazelbuild/bazel-gazelle/config" + "github.com/bazelbuild/bazel-gazelle/label" + "github.com/bazelbuild/bazel-gazelle/language" + "github.com/bazelbuild/bazel-gazelle/repo" + "github.com/bazelbuild/bazel-gazelle/resolve" + "github.com/bazelbuild/bazel-gazelle/rule" +) + +const testLoadForPackedRulesName = "test_load_for_packed_rules" + +type testLoadForPackedRulesLang struct { + language.BaseLang + + Initialized, RulesGenerated, DepsResolved bool +} + +var ( + _ language.Language = (*testLoadForPackedRulesLang)(nil) + _ language.LifecycleManager = (*testLoadForPackedRulesLang)(nil) +) + +func NewLanguage() language.Language { + return &testLoadForPackedRulesLang{} +} + +var kinds = map[string]rule.KindInfo{ + "selects.config_setting_group": { + NonEmptyAttrs: map[string]bool{"name": true}, + MergeableAttrs: map[string]bool{ + "match_all": true, + "match_any": true, + }, + }, +} + +var loads = []rule.LoadInfo{ + { + Name: "@bazel_skylib//lib:selects.bzl", + Symbols: []string{ + "selects", + }, + }, +} + +func (*testLoadForPackedRulesLang) Name() string { + return testLoadForPackedRulesName +} + +func (*testLoadForPackedRulesLang) Kinds() map[string]rule.KindInfo { + return kinds +} + +func (*testLoadForPackedRulesLang) Loads() []rule.LoadInfo { + return loads +} + +func (l *testLoadForPackedRulesLang) Before(ctx context.Context) { + l.Initialized = true +} + +func (l *testLoadForPackedRulesLang) GenerateRules(args language.GenerateArgs) language.GenerateResult { + if !l.Initialized { + panic("GenerateRules must not be called before Before") + } + if l.RulesGenerated { + panic("GenerateRules must not be called after DoneGeneratingRules") + } + + r := rule.NewRule("selects.config_setting_group", "all_configs_group") + + match := []string{ + "//:config_a", + "//:config_b", + } + + r.SetAttr("match_all", match) + + return language.GenerateResult{ + Gen: []*rule.Rule{r}, + Imports: []interface{}{nil}, + } +} + +func (l *testLoadForPackedRulesLang) DoneGeneratingRules() { + l.RulesGenerated = true +} + +func (l *testLoadForPackedRulesLang) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.RemoteCache, r *rule.Rule, imports interface{}, from label.Label) { + if !l.RulesGenerated { + panic("Expected a call to DoneGeneratingRules before Resolve") + } + if l.DepsResolved { + panic("Resolve must be called before calling AfterResolvingDeps") + } +} + +func (l *testLoadForPackedRulesLang) AfterResolvingDeps(ctx context.Context) { + l.DepsResolved = true +} diff --git a/rule/rule.go b/rule/rule.go index a7972ac61..abd0b5863 100644 --- a/rule/rule.go +++ b/rule/rule.go @@ -730,8 +730,9 @@ type attrValue struct { // NewRule creates a new, empty rule with the given kind and name. func NewRule(kind, name string) *Rule { - kindIdent := &bzl.Ident{Name: kind} + kindIdent := createDotExpr(kind) call := &bzl.CallExpr{X: kindIdent} + r := &Rule{ stmt: stmt{expr: call}, kind: kindIdent, @@ -752,6 +753,47 @@ func NewRule(kind, name string) *Rule { return r } +// Creates `bzl.Expr` for a kind which +// is either `*bzl.DotExpr` or `*bzl.Ident`. +// +// For `myKind` kind it returns: +// &bzl.Ident{ +// Name: "myKind" +// } +// +// For `myKind.inner` kind it returns: +// &bzl.DotExpr{ +// Name: "inner", +// X: &bzl.Ident { +// Name: "myKind" +// } +// } +func createDotExpr(kind string) bzl.Expr { + var expr bzl.Expr + parts := strings.Split(kind, ".") + + if len(parts) > 1 { + // last `parts` element is the main bzl.DotExpr + var dotExpr *bzl.DotExpr = &bzl.DotExpr{Name: parts[len(parts)-1]} + + _pDot := dotExpr + + for idx := len(parts) - 2; idx > 0; idx-- { + d := &bzl.DotExpr{Name: parts[idx]} + _pDot.X = d + _pDot = d + } + + // first `parts` element is the identifier + _pDot.X = &bzl.Ident{Name: parts[0]} + expr = dotExpr + } else { + expr = &bzl.Ident{Name: kind} + } + + return expr +} + func isNestedDotOrIdent(expr bzl.Expr) bool { if _, ok := expr.(*bzl.Ident); ok { return true @@ -999,7 +1041,9 @@ func (r *Rule) sync() { } call := r.expr.(*bzl.CallExpr) - call.X = r.kind + + // update `call.X` (e.g.: "# gazelle:map_kind") + call.X = createDotExpr(r.Kind()) if len(r.attrs) > 1 { call.ForceMultiLine = true diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index d81f39fa5..e2ebfeaf6 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -4,6 +4,8 @@ load( "gazelle_generation_test", ) +load("//tests:tools.bzl", "get_binary") + # Exclude this entire directly from having anything gnerated by Gazelle. That # way the test cases won't be fixed by `bazel run //:gazelle` when run in this # repository. @@ -26,10 +28,18 @@ gazelle_binary( visibility = ["//visibility:private"], ) +gazelle_binary( + name = "gazelle_with_language_load_for_packed_rules", + languages = [ + "//internal/language/test_load_for_packed_rules", + ], + visibility = ["//visibility:private"], +) + [gazelle_generation_test( # Name the test the path to the directory containing the WORKSPACE file. name = file[0:-len("/WORKSPACE")], - gazelle_binary = ":gazelle" if "loads_from_flag" not in file else ":gazelle_with_language_loads_from_flag", + gazelle_binary = get_binary(file), # This is a noop as the default is False. However, it does confirm that # gazelle_generation_test accepts setting common test attributes. local = False, diff --git a/tests/bcr/MODULE.bazel.lock b/tests/bcr/MODULE.bazel.lock index c5ebd7ed7..cb3049cbf 100644 --- a/tests/bcr/MODULE.bazel.lock +++ b/tests/bcr/MODULE.bazel.lock @@ -966,7 +966,7 @@ } }, "@gazelle~override//internal/bzlmod:non_module_deps.bzl%non_module_deps": { - "bzlTransitiveDigest": "wdirRXKNPbbln3dfsr2tlza6Y8itKEhUmqgQx8l634g=", + "bzlTransitiveDigest": "AjbsH9WZCj0ipLarbbkp25YBRrRhWYvO7OIiTcHyyok=", "accumulatedFileDigests": {}, "envVariables": {}, "generatedRepoSpecs": { diff --git a/tests/fix_load_for_packed_rules/BUILD.in b/tests/fix_load_for_packed_rules/BUILD.in new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fix_load_for_packed_rules/BUILD.out b/tests/fix_load_for_packed_rules/BUILD.out new file mode 100644 index 000000000..f8672c4af --- /dev/null +++ b/tests/fix_load_for_packed_rules/BUILD.out @@ -0,0 +1,9 @@ +load("@bazel_skylib//lib:selects.bzl", "selects") + +selects.config_setting_group( + name = "all_configs_group", + match_all = [ + "//:config_a", + "//:config_b", + ], +) diff --git a/tests/fix_load_for_packed_rules/README.md b/tests/fix_load_for_packed_rules/README.md new file mode 100644 index 000000000..fb9e87ac6 --- /dev/null +++ b/tests/fix_load_for_packed_rules/README.md @@ -0,0 +1,3 @@ +# Fix load statements for packed rules in struct + +Fixes load statement for newly generated `select.config_setting_group` rule. diff --git a/tests/fix_load_for_packed_rules/WORKSPACE b/tests/fix_load_for_packed_rules/WORKSPACE new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fix_load_for_packed_rules/arguments.txt b/tests/fix_load_for_packed_rules/arguments.txt new file mode 100644 index 000000000..e69de29bb diff --git a/tests/fix_load_for_packed_rules/expectedStdout.txt b/tests/fix_load_for_packed_rules/expectedStdout.txt new file mode 100644 index 000000000..e69de29bb diff --git a/tests/tools.bzl b/tests/tools.bzl new file mode 100644 index 000000000..9869c823a --- /dev/null +++ b/tests/tools.bzl @@ -0,0 +1,23 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +def get_binary(target): + """Get proper binary for test.""" + if "fix_load_for_packed_rules" in target: + return ":gazelle_with_language_load_for_packed_rules" + + if "loads_from_flag" in target: + return ":gazelle_with_language_loads_from_flag" + + return ":gazelle"