From 07dddde0b551df27623ab5028088628cf1f09228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Malmstr=C3=B6m?= Date: Tue, 3 Mar 2026 17:25:47 +0100 Subject: [PATCH 1/2] Allowing to disable buildozer on specific targets via #buildozer: leave-alone - Moving inherited comments to parsing instead of live-checks (note, this will miss comments added in the same change) - Moving comment parsing to separate file, reused by both Buildifier and Buildozer - Adding logic for Buildozer to ignore making changes to exprs tagged with #buildozer: leave-alone comments --- build/BUILD.bazel | 2 + build/comments.go | 46 +++++++++ build/comments_test.go | 193 ++++++++++++++++++++++++++++++++++++ build/lex.go | 20 ++++ build/parse_test.go | 74 ++++++++++++++ build/rewrite.go | 87 ++++++---------- build/syntax.go | 66 ++++++------ buildozer/buildozer_test.sh | 78 +++++++++++++++ edit/buildozer.go | 18 +++- edit/edit.go | 12 +++ edit/edit_test.go | 7 +- 11 files changed, 511 insertions(+), 92 deletions(-) create mode 100644 build/comments.go create mode 100644 build/comments_test.go diff --git a/build/BUILD.bazel b/build/BUILD.bazel index e1d3f6f12..6dd364e8a 100644 --- a/build/BUILD.bazel +++ b/build/BUILD.bazel @@ -10,6 +10,7 @@ go_yacc( go_library( name = "build", srcs = [ + "comments.go", "lex.go", "parse.y.baz.go", # keep "print.go", @@ -32,6 +33,7 @@ go_test( name = "build_test", size = "small", srcs = [ + "comments_test.go", "checkfile_test.go", "lex_test.go", "parse_test.go", diff --git a/build/comments.go b/build/comments.go new file mode 100644 index 000000000..501053f64 --- /dev/null +++ b/build/comments.go @@ -0,0 +1,46 @@ +/* +Copyright 2026 Google LLC + +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 + + https://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 build + +import ( + "slices" + "strings" +) + +// checkSelfOrInheritedComment checks comments relate to the expression, and returns true if +// the provided predicate matches any of the comments. +func checkSelfOrInheritedComment(expr Expr, predicate func(Comment) bool) bool { + for _, comment := range slices.Concat( + expr.Comment().Before, + expr.Comment().After, + expr.Comment().Suffix, + expr.Comment().Inherited) { + if predicate(comment) { + return true + } + } + return false +} + +// HasCommentContaining does a case insensitive matching to see if an expression, +// or its parent expressions have a comment containing the provided prefix. +func HasCommentContaining(expr Expr, prefix string) bool { + return checkSelfOrInheritedComment(expr, func(comment Comment) bool { + trimmedComment := strings.Trim(comment.Token, " \t\n#") + return strings.Contains(strings.ToLower(trimmedComment), strings.ToLower(prefix)) + }) +} diff --git a/build/comments_test.go b/build/comments_test.go new file mode 100644 index 000000000..31d54fa7c --- /dev/null +++ b/build/comments_test.go @@ -0,0 +1,193 @@ +/* +Copyright 2026 Google LLC + +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 + + https://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 build + +import ( + "testing" +) + +func callByName(name string) func(f *File) Expr { + return func(f *File) Expr { + var expr Expr + WalkInterruptable(f, func(x Expr, stk []Expr) error { + if call, ok := x.(*CallExpr); ok { + if ident, ok := call.X.(*Ident); ok && ident.Name == name { + expr = call + return &StopTraversalError{} + } + } + return nil + }) + return expr + } +} + +func assignExprByLHSName(name string) func(f *File) Expr { + return func(f *File) Expr { + var expr Expr + WalkInterruptable(f, func(x Expr, stk []Expr) error { + if assign, ok := x.(*AssignExpr); ok { + if ident, ok := assign.LHS.(*Ident); ok && ident.Name == name { + expr = assign + return &StopTraversalError{} + } + } + return nil + }) + return expr + } +} + +func TestHasCommentContaining(t *testing.T) { + var tests = []struct { + name string + buildFile string + selector func(*File) Expr + comment string + want bool + }{ + { + name: "rule_call_with_comment", + buildFile: ` +# has-comment +my_rule( + name = "my_target", +)`, + comment: "has-comment", + selector: callByName("my_rule"), + want: true, + }, + { + name: "rule_call_with_trailing_comment", + buildFile: ` +my_rule( + name = "my_target", +) # has-comment +`, + comment: "has-comment", + selector: callByName("my_rule"), + want: true, + }, + { + name: "rule_call_with_inherited_comment", + buildFile: ` +# has-comment +my_rule( + name = "my_target", + attr = my_func() +) +`, + comment: "has-comment", + selector: callByName("my_func"), + want: true, + }, + { + name: "function_call_with_comment", + buildFile: ` +my_rule( + name = "my_target", + # has-comment + attr = my_func() +) +`, + comment: "has-comment", + selector: callByName("my_func"), + want: true, + }, + { + name: "function_call_with_trailing_comment", + buildFile: ` +my_rule( + name = "my_target", + attr = my_func() # has-comment +) +`, + comment: "has-comment", + selector: callByName("my_func"), + want: true, + }, + { + name: "rule_call_without_comment", + buildFile: ` +my_rule( + name = "my_target", +)`, + comment: "has-comment", + selector: callByName("my_rule"), + want: false, + }, + { + name: "sibling_call_has_comment", + buildFile: ` +my_rule( + name = "my_target", + attr = my_func(), + attr2 = my_other_func() # has-comment +)`, + comment: "has-comment", + selector: callByName("my_func"), + want: false, + }, + { + name: "assign_expr_with_trailing_comment", + buildFile: ` +my_var = 1 # has-comment +`, + comment: "has-comment", + selector: assignExprByLHSName("my_var"), + want: true, + }, + { + name: "assign_expr_with_inherited_comment", + buildFile: ` +# has-comment +my_rule( + name = "my_target", + attr = my_func(func_arg = 1) +) +`, + comment: "has-comment", + selector: assignExprByLHSName("func_arg"), + want: true, + }, + { + name: "assign_expr_without_comment", + buildFile: ` +my_var = 1 +`, + comment: "has-comment", + selector: assignExprByLHSName("my_var"), + want: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + bld, err := Parse("BUILD", []byte(tc.buildFile)) + if err != nil { + t.Error(err) + } + expr := tc.selector(bld) + if expr == nil { + t.Error("selector returned nil") + } + got := HasCommentContaining(expr, tc.comment) + if got != tc.want { + t.Errorf("HasCommentContaining(%q) = %v, want %v", tc.comment, got, tc.want) + } + }) + } +} diff --git a/build/lex.go b/build/lex.go index ace048add..28097bac7 100644 --- a/build/lex.go +++ b/build/lex.go @@ -843,6 +843,7 @@ func (in *input) assignComments() { in.order(in.file) in.assignSuffixComments() in.assignLineComments() + in.assignInheritedComments() } func (in *input) assignSuffixComments() { @@ -903,3 +904,22 @@ func reverseComments(list []Comment) { list[i], list[j] = list[j], list[i] } } + +// assignInheritedComments assigns comments to the Inherited field of each expression. +func (in *input) assignInheritedComments() { + Walk(in.file, func(node Expr, stack []Expr) { + cs := node.Comment() + for _, stackExpr := range stack { + cs.Inherited = append(cs.Inherited, stackExpr.Comment().Before...) + cs.Inherited = append(cs.Inherited, stackExpr.Comment().After...) + + eStart, eEnd := node.Span() + for _, suffixComment := range stackExpr.Comment().Suffix { + if suffixComment.Start.Line <= eStart.Line || suffixComment.Start.Line <= eEnd.Line { + // Suffix comments are inherited only for expressions which overlap with the comment line number. + cs.Inherited = append(cs.Inherited, suffixComment) + } + } + } + }) +} diff --git a/build/parse_test.go b/build/parse_test.go index c50e29e1d..a943c7513 100644 --- a/build/parse_test.go +++ b/build/parse_test.go @@ -250,4 +250,78 @@ var parseTests = []struct { }, }, }, + { + in: `go_binary(name = "x") # comment`, + out: &File{ + Path: "BUILD", + Type: TypeBuild, + Stmt: []Expr{ + &CallExpr{ + X: &Ident{ + NamePos: Position{1, 1, 0}, + Name: "go_binary", + Comments: Comments{ + Inherited: []Comment{ + { + Token: "# comment", + Start: Position{1, 23, 22}, + }, + }, + }, + }, + ListStart: Position{1, 10, 9}, + List: []Expr{ + &AssignExpr{ + LHS: &Ident{ + NamePos: Position{1, 11, 10}, + Name: "name", + Comments: Comments{ + Inherited: []Comment{ + { + Token: "# comment", + Start: Position{1, 23, 22}, + }, + }, + }, + }, + OpPos: Position{1, 16, 15}, + Op: "=", + RHS: &StringExpr{ + Start: Position{1, 18, 17}, + Value: "x", + End: Position{1, 21, 20}, + Token: `"x"`, + Comments: Comments{ + Inherited: []Comment{ + { + Token: "# comment", + Start: Position{1, 23, 22}, + }, + }, + }, + }, + Comments: Comments{ + Inherited: []Comment{ + { + Token: "# comment", + Start: Position{1, 23, 22}, + }, + }, + }, + }, + }, + End: End{Pos: Position{1, 21, 20}}, + ForceMultiLine: false, + Comments: Comments{ + Suffix: []Comment{ + { + Token: "# comment", + Start: Position{1, 23, 22}, + }, + }, + }, + }, + }, + }, + }, } diff --git a/build/rewrite.go b/build/rewrite.go index 75d498459..be2942dcd 100644 --- a/build/rewrite.go +++ b/build/rewrite.go @@ -29,6 +29,17 @@ import ( "github.com/bazelbuild/buildtools/tables" ) +type rewriteComment string + +const ( + rewriteCommentDoNotSort rewriteComment = "do not sort" + rewriteCommentKeepSorted rewriteComment = "keep sorted" + rewriteCommentLeaveAlone rewriteComment = "buildifier: leave-alone" + rewriteCommentDisableSame rewriteComment = "disable=same-origin-load" + rewriteCommentDisableOutOfOrder rewriteComment = "disable=out-of-order-load" + rewriteCommentDisableLoadOnTop rewriteComment = "disable=load-on-top" +) + // DisableRewrites disables certain rewrites (for debugging). var DisableRewrites []string @@ -143,48 +154,16 @@ var rewrites = []struct { // leaveAlone reports whether any of the nodes on the stack are marked // with a comment containing "buildifier: leave-alone". -func leaveAlone(stk []Expr, final Expr) bool { - for _, x := range stk { - if leaveAlone1(x) { - return true - } - } - if final != nil && leaveAlone1(final) { - return true - } - return false -} - -// hasComment reports whether x is marked with a comment that -// after being converted to lower case, contains the specified text. -func hasComment(x Expr, text string) bool { - if x == nil { - return false - } - for _, com := range x.Comment().Before { - if strings.Contains(strings.ToLower(com.Token), text) { - return true - } - } - for _, com := range x.Comment().After { - if strings.Contains(strings.ToLower(com.Token), text) { - return true - } - } - for _, com := range x.Comment().Suffix { - if strings.Contains(strings.ToLower(com.Token), text) { - return true - } - } - return false +func leaveAlone(final Expr) bool { + return HasCommentContaining(final, string(rewriteCommentLeaveAlone)) } // isCommentAnywhere checks whether there's a comment containing the given text // anywhere in the file. -func isCommentAnywhere(f *File, text string) bool { +func isCommentAnywhere(f *File, comment rewriteComment) bool { commentExists := false WalkInterruptable(f, func(node Expr, stack []Expr) (err error) { - if hasComment(node, text) { + if HasCommentContaining(node, string(comment)) { commentExists = true return &StopTraversalError{} } @@ -193,22 +172,16 @@ func isCommentAnywhere(f *File, text string) bool { return commentExists } -// leaveAlone1 reports whether x is marked with a comment containing -// "buildifier: leave-alone", case-insensitive. -func leaveAlone1(x Expr) bool { - return hasComment(x, "buildifier: leave-alone") -} - // doNotSort reports whether x is marked with a comment containing // "do not sort", case-insensitive. func doNotSort(x Expr) bool { - return hasComment(x, "do not sort") + return HasCommentContaining(x, string(rewriteCommentDoNotSort)) } // keepSorted reports whether x is marked with a comment containing // "keep sorted", case-insensitive. func keepSorted(x Expr) bool { - return hasComment(x, "keep sorted") + return HasCommentContaining(x, string(rewriteCommentKeepSorted)) } // labelRE matches label strings, e.g. @r//x/y/z:abc @@ -307,7 +280,7 @@ func fixLabels(f *File, w *Rewriter) { fixLabelsWithinAContainer := func(e *Expr) { if list, ok := (*e).(*ListExpr); ok { for i := range list.List { - if leaveAlone1(list.List[i]) { + if leaveAlone(list.List[i]) { continue } joinLabel(&list.List[i]) @@ -316,7 +289,7 @@ func fixLabels(f *File, w *Rewriter) { } if set, ok := (*e).(*SetExpr); ok { for i := range set.List { - if leaveAlone1(set.List[i]) { + if leaveAlone(set.List[i]) { continue } joinLabel(&set.List[i]) @@ -331,11 +304,11 @@ func fixLabels(f *File, w *Rewriter) { Walk(f, func(v Expr, stk []Expr) { switch v := v.(type) { case *CallExpr: - if leaveAlone(stk, v) { + if leaveAlone(v) { return } for i := range v.List { - if leaveAlone1(v.List[i]) { + if leaveAlone(v.List[i]) { continue } as, ok := v.List[i].(*AssignExpr) @@ -346,7 +319,7 @@ func fixLabels(f *File, w *Rewriter) { if !ok || !w.IsLabelArg[key.Name] || w.LabelDenyList[callName(v)+"."+key.Name] { continue } - if leaveAlone1(as.RHS) { + if leaveAlone(as.RHS) { continue } @@ -371,7 +344,7 @@ func sortCallArgs(f *File, w *Rewriter) { if !ok { return } - if leaveAlone(stk, call) { + if leaveAlone(call) { return } rule := callName(call) @@ -478,7 +451,7 @@ func sortStringLists(f *File, w *Rewriter) { // Rule parameters, not applicable to default file types return } - if leaveAlone(stk, v) { + if leaveAlone(v) { return } if f.Type == TypeBzl { @@ -492,11 +465,11 @@ func sortStringLists(f *File, w *Rewriter) { } rule := callName(v) for _, arg := range v.List { - if leaveAlone1(arg) { + if leaveAlone(arg) { continue } as, ok := arg.(*AssignExpr) - if !ok || leaveAlone1(as) { + if !ok || leaveAlone(as) { continue } key, ok := as.LHS.(*Ident) @@ -955,7 +928,7 @@ func moveLoadOnTop(f *File, _ *Rewriter) { // Moving load statements in Workspace files can break the semantics return } - if isCommentAnywhere(f, "disable=load-on-top") { + if isCommentAnywhere(f, rewriteCommentDisableLoadOnTop) { // For backward compatibility. This rewrite used to be a suppressible warning, // in some cases it's hard to maintain the position of load statements (e.g. // when the file is automatically generated or has automatic transformations @@ -1034,8 +1007,8 @@ func compressSameOriginLoads(f *File, _ *Rewriter) { loads[load.Module.Value] = load continue } - if hasComment(previousLoad, "disable=same-origin-load") || - hasComment(load, "disable=same-origin-load") { + if HasCommentContaining(previousLoad, "disable=same-origin-load") || + HasCommentContaining(load, "disable=same-origin-load") { continue } @@ -1117,7 +1090,7 @@ func compareLoadLabels(load1Label, load2Label string) bool { // sortLoadStatements reorders sorts loads lexicographically by the source file, // but absolute loads have priority over local loads. func sortLoadStatements(f *File, _ *Rewriter) { - if isCommentAnywhere(f, "disable=out-of-order-load") { + if isCommentAnywhere(f, rewriteCommentDisableOutOfOrder) { // For backward compatibility. This rewrite used to be a suppressible warning, // in some cases it's hard to maintain the position of load statements (e.g. // when the file is automatically generated or has automatic transformations diff --git a/build/syntax.go b/build/syntax.go index e756ce7e2..db9150cbb 100644 --- a/build/syntax.go +++ b/build/syntax.go @@ -80,6 +80,8 @@ type Comments struct { // For top-level expressions only, After lists whole-line // comments following the expression. After []Comment + // Comments which are inherited from parent expressions. + Inherited []Comment } // Comment returns the receiver. This isn't useful by itself, but @@ -139,7 +141,7 @@ func (f *File) Span() (start, end Position) { return start, end } -//Copy creates and returns a non-deep copy of File +// Copy creates and returns a non-deep copy of File func (f *File) Copy() Expr { n := *f return &n @@ -157,7 +159,7 @@ func (x *CommentBlock) Span() (start, end Position) { return x.Start, x.Start } -//Copy creates and returns a non-deep copy of CommentBlock +// Copy creates and returns a non-deep copy of CommentBlock func (x *CommentBlock) Copy() Expr { n := *x return &n @@ -175,7 +177,7 @@ func (x *Ident) Span() (start, end Position) { return x.NamePos, x.NamePos.add(x.Name) } -//Copy creates and returns a non-deep copy of Ident +// Copy creates and returns a non-deep copy of Ident func (x *Ident) Copy() Expr { n := *x return &n @@ -205,7 +207,7 @@ func (x *TypedIdent) Span() (start, end Position) { return start, end } -//Copy creates and returns a non-deep copy of TypedIdent +// Copy creates and returns a non-deep copy of TypedIdent func (x *TypedIdent) Copy() Expr { n := *x return &n @@ -223,7 +225,7 @@ func (x *BranchStmt) Span() (start, end Position) { return x.TokenPos, x.TokenPos.add(x.Token) } -//Copy creates and returns a non-deep copy of BranchStmt +// Copy creates and returns a non-deep copy of BranchStmt func (x *BranchStmt) Copy() Expr { n := *x return &n @@ -241,7 +243,7 @@ func (x *LiteralExpr) Span() (start, end Position) { return x.Start, x.Start.add(x.Token) } -//Copy creates and returns a non-deep copy of LiteralExpr +// Copy creates and returns a non-deep copy of LiteralExpr func (x *LiteralExpr) Copy() Expr { n := *x return &n @@ -267,7 +269,7 @@ func (x *StringExpr) Span() (start, end Position) { return x.Start, x.End } -//Copy creates and returns a non-deep copy of StringExpr +// Copy creates and returns a non-deep copy of StringExpr func (x *StringExpr) Copy() Expr { n := *x return &n @@ -285,7 +287,7 @@ func (x *End) Span() (start, end Position) { return x.Pos, x.Pos.add(")") } -//Copy creates and returns a non-deep copy of End +// Copy creates and returns a non-deep copy of End func (x *End) Copy() Expr { n := *x return &n @@ -308,7 +310,7 @@ func (x *CallExpr) Span() (start, end Position) { return start, x.End.Pos.add(")") } -//Copy creates and returns a non-deep copy of CallExpr +// Copy creates and returns a non-deep copy of CallExpr func (x *CallExpr) Copy() Expr { n := *x return &n @@ -329,7 +331,7 @@ func (x *DotExpr) Span() (start, end Position) { return start, x.NamePos.add(x.Name) } -//Copy creates and returns a non-deep copy of DotExpr +// Copy creates and returns a non-deep copy of DotExpr func (x *DotExpr) Copy() Expr { n := *x return &n @@ -351,7 +353,7 @@ func (x *Comprehension) Span() (start, end Position) { return x.Lbrack, x.End.Pos.add("]") } -//Copy creates and returns a non-deep copy of Comprehension +// Copy creates and returns a non-deep copy of Comprehension func (x *Comprehension) Copy() Expr { n := *x return &n @@ -372,7 +374,7 @@ func (x *ForClause) Span() (start, end Position) { return x.For, end } -//Copy creates and returns a non-deep copy of ForClause +// Copy creates and returns a non-deep copy of ForClause func (x *ForClause) Copy() Expr { n := *x return &n @@ -391,7 +393,7 @@ func (x *IfClause) Span() (start, end Position) { return x.If, end } -//Copy creates and returns a non-deep copy of IfClause +// Copy creates and returns a non-deep copy of IfClause func (x *IfClause) Copy() Expr { n := *x return &n @@ -412,7 +414,7 @@ func (x *KeyValueExpr) Span() (start, end Position) { return start, end } -//Copy creates and returns a non-deep copy of KeyValueExpr +// Copy creates and returns a non-deep copy of KeyValueExpr func (x *KeyValueExpr) Copy() Expr { n := *x return &n @@ -432,7 +434,7 @@ func (x *DictExpr) Span() (start, end Position) { return x.Start, x.End.Pos.add("}") } -//Copy creates and returns a non-deep copy of DictExpr +// Copy creates and returns a non-deep copy of DictExpr func (x *DictExpr) Copy() Expr { n := *x return &n @@ -452,7 +454,7 @@ func (x *ListExpr) Span() (start, end Position) { return x.Start, x.End.Pos.add("]") } -//Copy creates and returns a non-deep copy of ListExpr +// Copy creates and returns a non-deep copy of ListExpr func (x *ListExpr) Copy() Expr { n := *x return &n @@ -472,7 +474,7 @@ func (x *SetExpr) Span() (start, end Position) { return x.Start, x.End.Pos.add("}") } -//Copy creates and returns a non-deep copy of SetExpr +// Copy creates and returns a non-deep copy of SetExpr func (x *SetExpr) Copy() Expr { n := *x return &n @@ -499,7 +501,7 @@ func (x *TupleExpr) Span() (start, end Position) { return start, end } -//Copy creates and returns a non-deep copy of TupleExpr +// Copy creates and returns a non-deep copy of TupleExpr func (x *TupleExpr) Copy() Expr { n := *x return &n @@ -522,7 +524,7 @@ func (x *UnaryExpr) Span() (start, end Position) { return x.OpStart, end } -//Copy creates and returns a non-deep copy of UnaryExpr +// Copy creates and returns a non-deep copy of UnaryExpr func (x *UnaryExpr) Copy() Expr { n := *x return &n @@ -545,7 +547,7 @@ func (x *BinaryExpr) Span() (start, end Position) { return start, end } -//Copy creates and returns a non-deep copy of BinaryExpr +// Copy creates and returns a non-deep copy of BinaryExpr func (x *BinaryExpr) Copy() Expr { n := *x return &n @@ -576,7 +578,7 @@ func (x *AssignExpr) Span() (start, end Position) { return start, end } -//Copy creates and returns a non-deep copy of AssignExpr +// Copy creates and returns a non-deep copy of AssignExpr func (x *AssignExpr) Copy() Expr { n := *x return &n @@ -596,7 +598,7 @@ func (x *ParenExpr) Span() (start, end Position) { return x.Start, x.End.Pos.add(")") } -//Copy creates and returns a non-deep copy of ParenExpr +// Copy creates and returns a non-deep copy of ParenExpr func (x *ParenExpr) Copy() Expr { n := *x return &n @@ -621,7 +623,7 @@ func (x *SliceExpr) Span() (start, end Position) { return start, x.End.add("]") } -//Copy creates and returns a non-deep copy of SliceExpr +// Copy creates and returns a non-deep copy of SliceExpr func (x *SliceExpr) Copy() Expr { n := *x return &n @@ -642,7 +644,7 @@ func (x *IndexExpr) Span() (start, end Position) { return start, x.End.add("]") } -//Copy creates and returns a non-deep copy of IndexExpr +// Copy creates and returns a non-deep copy of IndexExpr func (x *IndexExpr) Copy() Expr { n := *x return &n @@ -662,7 +664,7 @@ func (x *Function) Span() (start, end Position) { return x.StartPos, end } -//Copy creates and returns a non-deep copy of Function +// Copy creates and returns a non-deep copy of Function func (x *Function) Copy() Expr { n := *x return &n @@ -679,7 +681,7 @@ func (x *LambdaExpr) Span() (start, end Position) { return x.Function.Span() } -//Copy creates and returns a non-deep copy of LambdaExpr +// Copy creates and returns a non-deep copy of LambdaExpr func (x *LambdaExpr) Copy() Expr { n := *x return &n @@ -704,7 +706,7 @@ func (x *ConditionalExpr) Span() (start, end Position) { return start, end } -//Copy creates and returns a non-deep copy of ConditionalExpr +// Copy creates and returns a non-deep copy of ConditionalExpr func (x *ConditionalExpr) Copy() Expr { n := *x return &n @@ -733,7 +735,7 @@ func (x *LoadStmt) Span() (start, end Position) { return x.Load, x.Rparen.Pos.add(")") } -//Copy creates and returns a non-deep copy of LoadStmt +// Copy creates and returns a non-deep copy of LoadStmt func (x *LoadStmt) Copy() Expr { n := *x return &n @@ -755,7 +757,7 @@ func (x *DefStmt) Span() (start, end Position) { return x.Function.Span() } -//Copy creates and returns a non-deep copy of DefStmt +// Copy creates and returns a non-deep copy of DefStmt func (x *DefStmt) Copy() Expr { n := *x return &n @@ -782,7 +784,7 @@ func (x *ReturnStmt) Span() (start, end Position) { return x.Return, end } -//Copy creates and returns a non-deep copy of ReturnStmt +// Copy creates and returns a non-deep copy of ReturnStmt func (x *ReturnStmt) Copy() Expr { n := *x return &n @@ -804,7 +806,7 @@ func (x *ForStmt) Span() (start, end Position) { return x.For, end } -//Copy creates and returns a non-deep copy of ForStmt +// Copy creates and returns a non-deep copy of ForStmt func (x *ForStmt) Copy() Expr { n := *x return &n @@ -831,7 +833,7 @@ func (x *IfStmt) Span() (start, end Position) { return x.If, end } -//Copy creates and returns a non-deep copy of IfStmt +// Copy creates and returns a non-deep copy of IfStmt func (x *IfStmt) Copy() Expr { n := *x return &n diff --git a/buildozer/buildozer_test.sh b/buildozer/buildozer_test.sh index 955c582ed..340dba4dd 100755 --- a/buildozer/buildozer_test.sh +++ b/buildozer/buildozer_test.sh @@ -2423,4 +2423,82 @@ EOF diff -u MODULE.bazel.expected.stderr stderr || fail "Error output didn't match" } +function test_buildozer_leave_alone_delete_is_not_deleted() { +in='# buildozer: leave-alone +my_rule( + name = "x", + srcs = ["x.cc"], +) + +my_rule( + name = "y", + srcs = ["y.cc"], +)' + + run "$in" 'delete' '//pkg:%my_rule' + + assert_equals '# buildozer: leave-alone +my_rule( + name = "x", + srcs = ["x.cc"], +)' +} + +function test_buildozer_leave_alone_empty_package_is_not_deleted() { +in=' +# buildozer: leave-alone +package() + +package() + +my_rule( + name = "x", + srcs = ["x.cc"], +) +' + + run "$in" 'delete' '//pkg:%my_rule' + + assert_equals '# buildozer: leave-alone +package()' +} + +function test_buildozer_leave_alone_set_is_not_updated() { +in=' +# buildozer: leave-alone +my_rule( + name = "x", + srcs = ["x.cc"], +) + +my_rule( + name = "y", + srcs = ["y.cc"], +)' + + run "$in" 'set foo "bar"' '//pkg:%my_rule' + + assert_equals '# buildozer: leave-alone +my_rule( + name = "x", + srcs = ["x.cc"], +) + +my_rule( + name = "y", + srcs = ["y.cc"], + foo = "bar", +)' +} + +function test_buildozer_leave_alone_set_targeting_leave_alone_returns_error() { +in='# buildozer: leave-alone +my_rule( + name = "x", + srcs = ["x.cc"], +)' + + ERROR=2 run "$in" 'set foo "bar"' '//pkg:x' +} + run_suite "buildozer tests" diff --git a/edit/buildozer.go b/edit/buildozer.go index d246bfa65..170fea547 100644 --- a/edit/buildozer.go +++ b/edit/buildozer.go @@ -43,6 +43,12 @@ import ( "github.com/golang/protobuf/proto" ) +type BuildozerComment string + +const ( + BuildozerCommentLeaveAlone BuildozerComment = "buildozer: leave-alone" +) + // Options represents choices about how buildozer should behave. type Options struct { Stdout bool // write changed BUILD file to stdout @@ -987,10 +993,18 @@ func expandTargets(f *build.File, rule string) ([]*build.Rule, error) { } func filterRules(opts *Options, rules []*build.Rule) (result []*build.Rule) { + var rs []*build.Rule + for _, rule := range rules { + // Skip rules that are marked as leave-alone + if build.HasCommentContaining(rule.Call, string(BuildozerCommentLeaveAlone)) { + continue + } + rs = append(rs, rule) + } if len(opts.FilterRuleTypes) == 0 { - return rules + return rs } - for _, rule := range rules { + for _, rule := range rs { for _, filterType := range opts.FilterRuleTypes { if rule.Kind() == filterType { result = append(result, rule) diff --git a/edit/edit.go b/edit/edit.go index 5bc91641f..38de3127a 100644 --- a/edit/edit.go +++ b/edit/edit.go @@ -182,6 +182,10 @@ func PackageDeclaration(f *build.File) *build.Rule { func RemoveEmptyPackage(f *build.File) *build.File { var all []build.Expr for _, stmt := range f.Stmt { + if build.HasCommentContaining(stmt, string(BuildozerCommentLeaveAlone)) { + all = append(all, stmt) + continue + } if isEmptyPackage(stmt) { continue } @@ -216,6 +220,10 @@ func RemoveEmptyUseRepoCalls(f *build.File) *build.File { } var all []build.Expr for _, stmt := range f.Stmt { + if build.HasCommentContaining(stmt, string(BuildozerCommentLeaveAlone)) { + all = append(all, stmt) + continue + } if isEmptyUseRepoCall(stmt) { continue } @@ -303,6 +311,10 @@ func IndexOfRuleByName(f *build.File, name string) (int, *build.Rule) { } for i, stmt := range f.Stmt { + if build.HasCommentContaining(stmt, string(BuildozerCommentLeaveAlone)) { + // Do not find rules that are marked to be left alone. + continue + } // first check if call is a CallExpr, else check if it's an AssignExpr // with a CallExpr on the RHS call, ok := stmt.(*build.CallExpr) diff --git a/edit/edit_test.go b/edit/edit_test.go index 762f42bf8..311eda5b3 100644 --- a/edit/edit_test.go +++ b/edit/edit_test.go @@ -840,6 +840,11 @@ my_rule( a = my_macro( name="r2", ) + +# buildozer: leave-alone +my_rule( + name="r4", +) ` bld, err := build.Parse("BUILD", []byte(input)) @@ -852,7 +857,7 @@ a = my_macro( name string shouldFind bool }{ - {"r1", true}, {"r2", true}, {"r3", false}, + {"r1", true}, {"r2", true}, {"r3", false}, {"r4", false}, } for _, tc := range testCases { From 76f6250ebaf0169418b280576176c4a0b843f5ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20Malmstr=C3=B6m?= Date: Thu, 5 Mar 2026 13:45:49 +0100 Subject: [PATCH 2/2] Fixing BUILD file order --- build/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/BUILD.bazel b/build/BUILD.bazel index 6dd364e8a..192d5623c 100644 --- a/build/BUILD.bazel +++ b/build/BUILD.bazel @@ -33,8 +33,8 @@ go_test( name = "build_test", size = "small", srcs = [ - "comments_test.go", "checkfile_test.go", + "comments_test.go", "lex_test.go", "parse_test.go", "print_test.go",