Adding support for .bzl file operations

build/rule.go

@@ -165,25 +165,32 @@ func (f *File) implicitRuleName() string {
 // begins with a literal, if the call expression does not conform to either of these forms, an
 // empty string will be returned
 func (r *Rule) Kind() string {
+	if r.Call.X == nil {
+		return ""
+	}
+	// The kind may be a simple identifier (e.g. `go_library`) or a dotted expression
+	// (e.g. `native.go_library`). Extract the dotted path conservatively.
 	var names []string
-	expr := r.Call.X
-	for {
-		x, ok := expr.(*DotExpr)
-		if !ok {
-			break
+	var walk func(Expr) bool
+	walk = func(e Expr) bool {
+		switch e := e.(type) {
+		case *Ident:
+			names = append(names, e.Name)
+			return true
+		case *DotExpr:
+			// DotExpr represents `X.Name`.
+			if !walk(e.X) {
+				return false
+			}
+			names = append(names, e.Name)
+			return true
+		default:
+			return false
 		}
-		names = append(names, x.Name)
-		expr = x.X
 	}
-	x, ok := expr.(*Ident)
-	if !ok {
+	if !walk(r.Call.X) {
 		return ""
 	}
-	names = append(names, x.Name)
-	// Reverse the elements since the deepest expression contains the leading literal
-	for l, r := 0, len(names)-1; l < r; l, r = l+1, r-1 {
-		names[l], names[r] = names[r], names[l]
-	}
 	return strings.Join(names, ".")
 }
 

edit/buildozer.go

@@ -86,6 +86,20 @@ type CmdEnvironment struct {
 // The cmdXXX functions implement the various commands.
 
 func cmdAdd(opts *Options, env CmdEnvironment) (*build.File, error) {
+	// Variable targets are represented by a dummy Rule with a nil Call.
+	if env.Rule != nil && env.Rule.Call == nil {
+		varName := env.Rule.ImplicitName
+		varAssign, ok := env.Vars[varName]
+		if !ok {
+			return nil, fmt.Errorf("variable %s is not defined", varName)
+		}
+		for _, val := range env.Args[1:] {
+			strVal := getStringExpr(val, env.Pkg)
+			varAssign.RHS = AddValueToList(varAssign.RHS, env.Pkg, strVal, true)
+		}
+
+		return env.File, nil
+	}
 	attr := env.Args[0]
 	for _, val := range env.Args[1:] {
 		if IsIntList(attr) {
@@ -211,6 +225,23 @@ func cmdPrintComment(opts *Options, env CmdEnvironment) (*build.File, error) {
 }
 
 func cmdDelete(opts *Options, env CmdEnvironment) (*build.File, error) {
+	// Variable targets are represented by a dummy Rule with a nil Call.
+	if env.Rule != nil && env.Rule.Call == nil {
+		varName := env.Rule.ImplicitName
+		varAssign, ok := env.Vars[varName]
+		if !ok {
+			return nil, fmt.Errorf("variable %s is not defined", varName)
+		}
+		var all []build.Expr
+		for _, stmt := range env.File.Stmt {
+			if stmt == varAssign {
+				continue
+			}
+			all = append(all, stmt)
+		}
+		env.File.Stmt = all
+		return env.File, nil
+	}
 	return DeleteRule(env.File, env.Rule), nil
 }
 
@@ -331,6 +362,47 @@ func cmdSubstituteLoad(opts *Options, env CmdEnvironment) (*build.File, error) {
 }
 
 func cmdPrint(opts *Options, env CmdEnvironment) (*build.File, error) {
+	// Variable targets are represented by a dummy Rule with a nil Call.
+	if env.Rule != nil && env.Rule.Call == nil {
+		varName := env.Rule.ImplicitName
+		varAssign, ok := env.Vars[varName]
+		if !ok {
+			return nil, fmt.Errorf("variable %s is not defined", varName)
+		}
+		format := env.Args
+		if len(format) == 0 {
+			format = []string{"value"}
+		}
+		fields := make([]*apipb.Output_Record_Field, len(format))
+		for i, str := range format {
+			switch str {
+			case "name":
+				fields[i] = &apipb.Output_Record_Field{Value: &apipb.Output_Record_Field_Text{Text: varName}}
+			case "kind":
+				fields[i] = &apipb.Output_Record_Field{Value: &apipb.Output_Record_Field_Text{Text: "var"}}
+			case "label":
+				label := labels.Label{Package: env.Pkg, Target: varName}
+				fields[i] = &apipb.Output_Record_Field{Value: &apipb.Output_Record_Field_Text{Text: label.Format()}}
+			case "path":
+				fields[i] = &apipb.Output_Record_Field{Value: &apipb.Output_Record_Field_Text{Text: env.File.Path}}
+			case "startline":
+				start, _ := varAssign.Span()
+				fields[i] = &apipb.Output_Record_Field{Value: &apipb.Output_Record_Field_Number{Number: int32(start.Line)}}
+			case "endline":
+				_, end := varAssign.Span()
+				fields[i] = &apipb.Output_Record_Field{Value: &apipb.Output_Record_Field_Number{Number: int32(end.Line)}}
+			case "rule":
+				fields[i] = &apipb.Output_Record_Field{Value: &apipb.Output_Record_Field_Text{Text: build.FormatString(varAssign)}}
+			case "value":
+				fields[i] = &apipb.Output_Record_Field{Value: &apipb.Output_Record_Field_Text{Text: build.FormatString(varAssign.RHS)}}
+			default:
+				fields[i] = &apipb.Output_Record_Field{Value: &apipb.Output_Record_Field_Error{Error: apipb.Output_Record_Field_MISSING}}
+			}
+		}
+		env.output.Fields = fields
+		return nil, nil
+	}
+
 	format := env.Args
 	if len(format) == 0 {
 		format = []string{"name", "kind"}
@@ -419,6 +491,18 @@ func attrKeysForPattern(rule *build.Rule, pattern string) []string {
 }
 
 func cmdRemove(opts *Options, env CmdEnvironment) (*build.File, error) {
+	// Variable targets are represented by a dummy Rule with a nil Call.
+	if env.Rule != nil && env.Rule.Call == nil {
+		varName := env.Rule.ImplicitName
+		varAssign, ok := env.Vars[varName]
+		if !ok {
+			return nil, fmt.Errorf("variable %s is not defined", varName)
+		}
+		for _, val := range env.Args[1:] {
+			ListDelete(varAssign.RHS, val, env.Pkg)
+		}
+		return env.File, nil
+	}
 	if len(env.Args) == 1 { // Remove the attribute
 		if env.Args[0] == "*" {
 			didDelete := false
@@ -560,6 +644,16 @@ func cmdSubstitute(opts *Options, env CmdEnvironment) (*build.File, error) {
 func cmdSet(opts *Options, env CmdEnvironment) (*build.File, error) {
 	attr := env.Args[0]
 	args := env.Args[1:]
+	// Variable targets are represented by a dummy Rule with a nil Call.
+	if env.Rule != nil && env.Rule.Call == nil {
+		varName := env.Rule.ImplicitName
+		varAssign, ok := env.Vars[varName]
+		if !ok {
+			return nil, fmt.Errorf("variable %s is not defined", varName)
+		}
+		varAssign.RHS = getAttrValueExpr(attr, args, env)
+		return env.File, nil
+	}
 	if attr == "kind" {
 		env.Rule.SetKind(args[0])
 	} else {
@@ -962,7 +1056,7 @@ var readonlyCommands = map[string]bool{
 	"print_comment": true,
 }
 
-func expandTargets(f *build.File, rule string) ([]*build.Rule, error) {
+func expandTargets(f *build.File, rule string, vars map[string]*build.AssignExpr) ([]*build.Rule, error) {
 	if r := FindRuleByName(f, rule); r != nil {
 		return []*build.Rule{r}, nil
 	} else if r := FindExportedFile(f, rule); r != nil {
@@ -982,6 +1076,12 @@ func expandTargets(f *build.File, rule string) ([]*build.Rule, error) {
 		} else {
 			return f.Rules(kind), nil
 		}
+	} else if vars != nil {
+		// When variable editing is enabled, allow targeting global variables directly,
+		// but only if the variable is actually defined in the file.
+		if _, ok := vars[rule]; ok {
+			return []*build.Rule{{ImplicitName: rule}}, nil
+		}
 	}
 	return nil, fmt.Errorf("rule '%s' not found", rule)
 }
@@ -1132,11 +1232,19 @@ type rewriteResult struct {
 func getGlobalVariables(exprs []build.Expr) (vars map[string]*build.AssignExpr) {
 	vars = make(map[string]*build.AssignExpr)
 	for _, expr := range exprs {
-		if as, ok := expr.(*build.AssignExpr); ok {
-			if lhs, ok := as.LHS.(*build.Ident); ok {
-				vars[lhs.Name] = as
+		build.Walk(expr, func(x build.Expr, stk []build.Expr) {
+			//Skip variables defined inside functions
+			for _, frame := range stk {
+				if _, ok := frame.(*build.DefStmt); ok {
+					return
+				}
 			}
-		}
+			if as, ok := x.(*build.AssignExpr); ok {
+				if lhs, ok := as.LHS.(*build.Ident); ok {
+					vars[lhs.Name] = as
+				}
+			}
+		})
 	}
 	return vars
 }
@@ -1238,7 +1346,7 @@ func rewrite(opts *Options, commandsForFile commandsForFile) *rewriteResult {
 			absPkg = f.Pkg
 		}
 
-		targets, err := expandTargets(f, rule)
+		targets, err := expandTargets(f, rule, vars)
 		if err != nil {
 			cerr := commandError(cft.commands, cft.target, err)
 			errs = append(errs, cerr)
@@ -1298,6 +1406,7 @@ func executeCommandsInFile(
 ) (*build.File, error) {
 	changed := false
 	for _, cmd := range cft.commands {
+		cmdName := cmd.tokens[0]
 		cmdInfo := AllCommands[cmd.tokens[0]]
 		// Depending on whether a transformation is rule-specific or not, it should be applied to
 		// every rule that satisfies the filter or just once to the file.
@@ -1306,6 +1415,22 @@ func executeCommandsInFile(
 			cmdTargets = []*build.Rule{nil}
 		}
 		for _, r := range cmdTargets {
+			// Variable targets are represented by a dummy Rule with a nil Call.
+			// Most rule-specific commands assume a non-nil Call, so guard centrally to avoid panics.
+			if r != nil && r.Call == nil {
+				switch cmdName {
+				case "add", "remove", "set", "print", "delete":
+					// Supported variable-target commands.
+				default:
+					err := fmt.Errorf("command %q does not support variable targets", cmdName)
+					cerr := commandError([]command{cmd}, cft.target, err)
+					if opts.KeepGoing {
+						*errs = append(*errs, cerr)
+						continue
+					}
+					return nil, cerr
+				}
+			}
 			record := &apipb.Output_Record{}
 			newf, err := cmdInfo.Fn(opts, CmdEnvironment{f, r, vars, absPkg, cmd.tokens[1:], record})
 			if len(record.Fields) != 0 {
@@ -1691,7 +1816,7 @@ func ExecuteCommandsOnInlineFile(fileContent []byte, commands []string) ([]byte,
 		f.Type = build.TypeBuild
 	}
 	for _, cft := range commandsByTargetName {
-		rules, err := expandTargets(f, cft.target)
+		rules, err := expandTargets(f, cft.target, nil)
 		if err != nil {
 			return nil, err
 		}

edit/buildozer_test.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 	"testing"
 
+	apipb "github.com/bazelbuild/buildtools/api_proto"
 	"github.com/bazelbuild/buildtools/build"
 	"github.com/google/go-cmp/cmp"
 )
@@ -859,6 +860,118 @@ func TestCmdDictOperations(t *testing.T) {
 	}
 }
 
+func TestCmdAddRemove_GlobalVariableInBzl(t *testing.T) {
+	input := `my_patches = ["a.patch"]
+
+def _fn():
+    my_patches = ["function_local.patch"]
+`
+	f, err := build.ParseBzl("defs.bzl", []byte(input))
+	if err != nil {
+		t.Fatal(err)
+	}
+	vars := getGlobalVariables(f.Stmt)
+	if _, ok := vars["my_patches"]; !ok {
+		t.Fatalf("expected my_patches to be detected as a global variable")
+	}
+	// Ensure we don't treat function-local assignments as globals.
+	if _, ok := vars["_fn"]; ok {
+		t.Fatalf("unexpected: function name should not be treated as a variable")
+	}
+
+	// Variable targets are represented by a dummy Rule with nil Call.
+	varTarget := &build.Rule{ImplicitName: "my_patches"}
+
+	// Add b.patch to the variable list.
+	_, err = cmdAdd(NewOpts(), CmdEnvironment{
+		File: f,
+		Rule: varTarget,
+		Vars: vars,
+		Pkg:  "",
+		Args: []string{"patches", "b.patch"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Remove a.patch from the variable list.
+	_, err = cmdRemove(NewOpts(), CmdEnvironment{
+		File: f,
+		Rule: varTarget,
+		Vars: vars,
+		Pkg:  "",
+		Args: []string{"patches", "a.patch"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	got := strings.TrimSpace(string(build.Format(f)))
+	expected := `my_patches = ["b.patch"]
+
+def _fn():
+    my_patches = ["function_local.patch"]`
+	wantF, err := build.ParseBzl("defs.bzl", []byte(expected))
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := strings.TrimSpace(string(build.Format(wantF)))
+	if got != want {
+		t.Errorf("global variable edit in .bzl:\n got: %s\nwant: %s", got, want)
+	}
+}
+
+func TestExecuteCommandsInFile_RejectsUnsupportedVariableTargetCommands(t *testing.T) {
+	f, err := build.ParseBzl("defs.bzl", []byte(`v = ["a"]`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	vars := getGlobalVariables(f.Stmt)
+	varTarget := &build.Rule{ImplicitName: "v"} // Call == nil => variable target
+
+	cft := commandsForTarget{
+		target: "//pkg:v",
+		commands: []command{
+			{tokens: []string{"rename", "a", "b"}},
+		},
+	}
+	records := []*apipb.Output_Record{}
+	errs := []error{}
+	_, execErr := executeCommandsInFile(NewOpts(), f, cft, []*build.Rule{varTarget}, &records, vars, "", &errs)
+	if execErr == nil {
+		t.Fatalf("expected error when running unsupported command on variable target")
+	}
+	if !strings.Contains(execErr.Error(), "does not support variable targets") {
+		t.Fatalf("unexpected error: %v", execErr)
+	}
+}
+
+func TestExecuteCommandsInFile_AllowsSetOnVariableTarget(t *testing.T) {
+	f, err := build.ParseBzl("defs.bzl", []byte(`v = ["a"]`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	vars := getGlobalVariables(f.Stmt)
+	varTarget := &build.Rule{ImplicitName: "v"} // Call == nil => variable target
+
+	cft := commandsForTarget{
+		target: "//pkg:v",
+		commands: []command{
+			{tokens: []string{"set", "deps", "b", "c"}},
+		},
+	}
+	records := []*apipb.Output_Record{}
+	errs := []error{}
+	_, execErr := executeCommandsInFile(NewOpts(), f, cft, []*build.Rule{varTarget}, &records, vars, "", &errs)
+	if execErr != nil {
+		t.Fatalf("unexpected error: %v", execErr)
+	}
+	got := strings.TrimSpace(string(build.Format(f)))
+	if got != `v = ["b", "c"]` {
+		t.Fatalf("got:\n%s\nwant:\n%s", got, `v = ["b", "c"]`)
+	}
+}
+
 func TestCmdSetSelect(t *testing.T) {
 	for i, tc := range []struct {
 		name      string