diff --git a/build/print.go b/build/print.go index 73dfaae09..62a242135 100644 --- a/build/print.go +++ b/build/print.go @@ -277,6 +277,10 @@ func (p *printer) compactStmt(s1, s2 Expr) bool { // Standalone comment blocks shouldn't be attached to other statements return false } else if (p.formattingMode() == TypeBuild) && p.level == 0 { + if tables.CompactConstantDefinitions && isAssignmentExpr(s1) && isAssignmentExpr(s2) { + // Two constant definitions do not need an extra line (if compact option enabled). + return true + } // Top-level statements in a BUILD or WORKSPACE file return false } else if isFunctionDefinition(s1) || isFunctionDefinition(s2) { @@ -446,6 +450,11 @@ func isCommentBlock(x Expr) bool { return ok } +func isAssignmentExpr(x Expr) bool { + _, ok := x.(*AssignExpr) + return ok +} + // isFunctionDefinition checks if the statement is a def code block func isFunctionDefinition(x Expr) bool { _, ok := x.(*DefStmt) diff --git a/build/print_test.go b/build/print_test.go index d76ec794d..2e6284799 100644 --- a/build/print_test.go +++ b/build/print_test.go @@ -45,9 +45,13 @@ func setFlags(file string) func() { if strings.Contains(file, string(os.PathSeparator)+"050.") { tables.ShortenAbsoluteLabelsToRelative = true } + if strings.Contains(file, ".compactconst.") { + tables.CompactConstantDefinitions = true + } return func() { tables.StripLabelLeadingSlashes = false tables.ShortenAbsoluteLabelsToRelative = false + tables.CompactConstantDefinitions = false } } diff --git a/build/testdata/004.compactconst.golden b/build/testdata/004.compactconst.golden new file mode 100644 index 000000000..46cd9f9c0 --- /dev/null +++ b/build/testdata/004.compactconst.golden @@ -0,0 +1,6 @@ +JAVA_FILES = [ + "Foo.java", + "Bar.java", + "Baz.java", + "Quux.java", +] diff --git a/build/testdata/005.compactconst.golden b/build/testdata/005.compactconst.golden new file mode 100644 index 000000000..9cb3bb799 --- /dev/null +++ b/build/testdata/005.compactconst.golden @@ -0,0 +1,7 @@ +JAVA_FILES = [ + # Comment regarding Foo.java + "Foo.java", + "Bar.java", + "Baz.java", # Comment regarding Baz.java + "Quux.java", +] diff --git a/build/testdata/006.compactconst.build.golden b/build/testdata/006.compactconst.build.golden new file mode 100644 index 000000000..b4cb637c9 --- /dev/null +++ b/build/testdata/006.compactconst.build.golden @@ -0,0 +1,30 @@ +set = { + 1, + 2, + 3, +} +multiline_set = { + 4, + 5, + [6], +} +plus = lambda x, y: x + y +two = (lambda x: x(x))(lambda z: lambda y: z)(1)(2)(3) +make_one = lambda: 1 +l = lambda x, y, *args, **kwargs: f( + y, + key = x, + *args, + **kwargs +) + +TriggerActionAfterProbe( + action = lambda State, response: None, + predicate = lambda State, response: all([ + response["request"]["method"] == "GET", + response["request"]["url"].find("/posts/") != -1, + response["request"]["url"].endswith("/comments"), + response["status_code"] in range(200, 299), + ]), + probe = ("http", "response"), +) diff --git a/build/testdata/006.compactconst.bzl.golden b/build/testdata/006.compactconst.bzl.golden new file mode 100644 index 000000000..fd0fbbbe2 --- /dev/null +++ b/build/testdata/006.compactconst.bzl.golden @@ -0,0 +1,21 @@ +set = {1, 2, 3} +multiline_set = { + 4, + 5, + [6], +} +plus = lambda x, y: x + y +two = (lambda x: x(x))(lambda z: lambda y: z)(1)(2)(3) +make_one = lambda: 1 +l = lambda x, y, *args, **kwargs: f(y, key = x, *args, **kwargs) + +TriggerActionAfterProbe( + probe = ("http", "response"), + predicate = lambda State, response: all([ + response["request"]["method"] == "GET", + response["request"]["url"].find("/posts/") != -1, + response["request"]["url"].endswith("/comments"), + response["status_code"] in range(200, 299), + ]), + action = lambda State, response: None, +) diff --git a/build/testdata/017.compactconst.golden b/build/testdata/017.compactconst.golden new file mode 100644 index 000000000..65cca16da --- /dev/null +++ b/build/testdata/017.compactconst.golden @@ -0,0 +1,37 @@ +# c1 +greeting = "hello " + \ + "world" # c2 +# c3 + +# c4 +greeting = "hello " + \ + "world" # c5 +# c6 + +# c7 +greeting = "hello " + \ + "world" # c8 +# c9 + +# c10 +greeting = ("hello " + # c11 + "world") # c12 +# c13 + +# c14 +greeting = ("hello " + # c15 + "world") # c16 +# c17 + +# c18 +greeting = ("hello" + # c19 + # c20 + "world") # c21 +# c22 + +greeting = "hello " + \ + "world" # c23 +greeting = ("hello " + # c24 + "world") +greeting = ("hello " + # c25 + "world") diff --git a/build/testdata/043.compactconst.golden b/build/testdata/043.compactconst.golden new file mode 100644 index 000000000..f88f67403 --- /dev/null +++ b/build/testdata/043.compactconst.golden @@ -0,0 +1,12 @@ +bar = "bar" + +# This gets an empty expr_opt in the parser, causing a nil to appear. +b = bar[:-2] + +# Test that slices and partial slices are parsed properly +f = foo[-1:-2:-3] +f = foo[1::] +f = foo[:1:] +f = foo[::1] +f = foo[::] +f = foo[:] diff --git a/build/testdata/066.compactconst.build.golden b/build/testdata/066.compactconst.build.golden new file mode 100644 index 000000000..06e4b3715 --- /dev/null +++ b/build/testdata/066.compactconst.build.golden @@ -0,0 +1,31 @@ +st1 = "abc" +st2 = """ +multiline""" +st3 = ( + "multi" +) +n = 123 +id = a +fct1 = foo(1) +fct2 = foo( + arg = 1, +) +fct3 = ( + foo( + arg = 1, + ) +) + +macro( + name = "foo", + arg = ["a"], +) + +nested = 1 +comments = [ + "a", + ("b"), # end of line comment + + # before comment + ("c"), +] # comment diff --git a/build/testdata/066.compactconst.bzl.golden b/build/testdata/066.compactconst.bzl.golden new file mode 100644 index 000000000..ec587594e --- /dev/null +++ b/build/testdata/066.compactconst.bzl.golden @@ -0,0 +1,31 @@ +st1 = ("abc") +st2 = (""" +multiline""") +st3 = ( + "multi" +) +n = (123) +id = (a) +fct1 = (foo(1)) +fct2 = (foo( + arg = 1, +)) +fct3 = ( + foo( + arg = 1, + ) +) + +(macro( + name = ("foo"), + arg = ([("a")]), +)) + +nested = (((((1))))) +comments = ([ + "a", + ("b"), # end of line comment + + # before comment + ("c"), +]) # comment diff --git a/build/testdata/067.compactconst.build.golden b/build/testdata/067.compactconst.build.golden new file mode 100644 index 000000000..292ecd384 --- /dev/null +++ b/build/testdata/067.compactconst.build.golden @@ -0,0 +1,53 @@ +load(":a", "b") +load( + # A + ":foo.bzl", + # B + "bar", +) + +cc_binary( + # A + name = "bin", + # B + srcs = ["bin.cc"], + # C +) + +cc_binary( + name = "wibble", + srcs = ["wibble.cc"], +) + +my_list = [ + 1, + # A + 2, + # B +] +my_1tuple = ( + # A + 1, + # B +) +my_2tuple = ( + # A + 1, + # B + 2, + # C +) +my_dict = { + "a": 1, + # A + "b": 2, + # B +} + +func(a) + +func(b) + +func(c, d) + +func(e, f) diff --git a/build/testdata/067.compactconst.bzl.golden b/build/testdata/067.compactconst.bzl.golden new file mode 100644 index 000000000..4514180f5 --- /dev/null +++ b/build/testdata/067.compactconst.bzl.golden @@ -0,0 +1,46 @@ +load(":a", "b") +load( + # A + ":foo.bzl", + # B + "bar", +) + +cc_binary( + # A + name = "bin", + # B + srcs = ["bin.cc"], + # C +) +cc_binary(name = "wibble", srcs = ["wibble.cc"]) + +my_list = [ + 1, + # A + 2, + # B +] +my_1tuple = ( + # A + 1, + # B +) +my_2tuple = ( + # A + 1, + # B + 2, + # C +) +my_dict = { + "a": 1, + # A + "b": 2, + # B +} + +func(a) +func(b) +func(c, d) +func(e, f) diff --git a/tables/jsonparser.go b/tables/jsonparser.go index 43f3f3fcc..00d1abda3 100644 --- a/tables/jsonparser.go +++ b/tables/jsonparser.go @@ -29,6 +29,7 @@ type Definitions struct { SortableDenylist map[string]bool SortableAllowlist map[string]bool NamePriority map[string]int + CompactConstantDefinitions bool StripLabelLeadingSlashes bool ShortenAbsoluteLabelsToRelative bool } @@ -55,9 +56,9 @@ func ParseAndUpdateJSONDefinitions(file string, merge bool) error { } if merge { - MergeTables(definitions.IsLabelArg, definitions.LabelDenylist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableDenylist, definitions.SortableAllowlist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative) + MergeTables(definitions.IsLabelArg, definitions.LabelDenylist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableDenylist, definitions.SortableAllowlist, definitions.NamePriority, definitions.CompactConstantDefinitions, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative) } else { - OverrideTables(definitions.IsLabelArg, definitions.LabelDenylist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableDenylist, definitions.SortableAllowlist, definitions.NamePriority, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative) + OverrideTables(definitions.IsLabelArg, definitions.LabelDenylist, definitions.IsListArg, definitions.IsSortableListArg, definitions.SortableDenylist, definitions.SortableAllowlist, definitions.NamePriority, definitions.CompactConstantDefinitions, definitions.StripLabelLeadingSlashes, definitions.ShortenAbsoluteLabelsToRelative) } return nil } diff --git a/tables/jsonparser_test.go b/tables/jsonparser_test.go index 800934de6..7cb1871b1 100644 --- a/tables/jsonparser_test.go +++ b/tables/jsonparser_test.go @@ -30,13 +30,14 @@ func TestParseJSONDefinitions(t *testing.T) { } expected := Definitions{ - IsLabelArg: map[string]bool{"srcs": true}, - LabelDenylist: map[string]bool{}, - IsSortableListArg: map[string]bool{"srcs": true, "visibility": true}, - SortableDenylist: map[string]bool{"genrule.srcs": true}, - SortableAllowlist: map[string]bool{}, - NamePriority: map[string]int{"name": -1}, - StripLabelLeadingSlashes: true, + IsLabelArg: map[string]bool{"srcs": true}, + LabelDenylist: map[string]bool{}, + IsSortableListArg: map[string]bool{"srcs": true, "visibility": true}, + SortableDenylist: map[string]bool{"genrule.srcs": true}, + SortableAllowlist: map[string]bool{}, + NamePriority: map[string]int{"name": -1}, + CompactConstantDefinitions: true, + StripLabelLeadingSlashes: true, } if !reflect.DeepEqual(expected, definitions) { t.Errorf("ParseJSONDefinitions(simple_tables.json) = %v; want %v", definitions, expected) diff --git a/tables/tables.go b/tables/tables.go index 720487edb..c041f05aa 100644 --- a/tables/tables.go +++ b/tables/tables.go @@ -209,6 +209,8 @@ var NamePriority = map[string]int{ "alwayslink": 7, } +var CompactConstantDefinitions = false + var StripLabelLeadingSlashes = false var ShortenAbsoluteLabelsToRelative = false @@ -299,7 +301,7 @@ var IsModuleOverride = map[string]bool{ } // OverrideTables allows a user of the build package to override the special-case rules. The user-provided tables replace the built-in tables. -func OverrideTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sortAllowlist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool) { +func OverrideTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sortAllowlist map[string]bool, namePriority map[string]int, compactConstantDefinitions, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool) { IsLabelArg = labelArg LabelDenylist = denylist IsListArg = listArg @@ -307,12 +309,13 @@ func OverrideTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, SortableDenylist = sortDenylist SortableAllowlist = sortAllowlist NamePriority = namePriority + CompactConstantDefinitions = compactConstantDefinitions StripLabelLeadingSlashes = stripLabelLeadingSlashes ShortenAbsoluteLabelsToRelative = shortenAbsoluteLabelsToRelative } // MergeTables allows a user of the build package to override the special-case rules. The user-provided tables are merged into the built-in tables. -func MergeTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sortAllowlist map[string]bool, namePriority map[string]int, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool) { +func MergeTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sortAllowlist map[string]bool, namePriority map[string]int, compactConstantDefinitions, stripLabelLeadingSlashes, shortenAbsoluteLabelsToRelative bool) { for k, v := range labelArg { IsLabelArg[k] = v } @@ -334,6 +337,7 @@ func MergeTables(labelArg, denylist, listArg, sortableListArg, sortDenylist, sor for k, v := range namePriority { NamePriority[k] = v } + CompactConstantDefinitions = compactConstantDefinitions || CompactConstantDefinitions StripLabelLeadingSlashes = stripLabelLeadingSlashes || StripLabelLeadingSlashes ShortenAbsoluteLabelsToRelative = shortenAbsoluteLabelsToRelative || ShortenAbsoluteLabelsToRelative } diff --git a/tables/testdata/simple_tables.json b/tables/testdata/simple_tables.json index 9243e2d52..d07864484 100644 --- a/tables/testdata/simple_tables.json +++ b/tables/testdata/simple_tables.json @@ -19,5 +19,6 @@ "NamePriority": { "name": -1 }, + "CompactConstantDefinitions": true, "StripLabelLeadingSlashes": true }