Skip to content

Commit

Permalink
Adding "Compact Constants" Option
Browse files Browse the repository at this point in the history
* Adding `CompactConstantDefinitions` boolean flag to `tables/tables.go`.
  * Can be set via tables json file (`--add_tables` / `--tables` command line or `tables` / `addTables` json config option).
* Updating `compactStmt()` in `build/print.go`.
  * If `CompactConstantDefinitions` is true and both statements are assignments (for const definitions), then the extra line should be removed.
  * Lower precedence than various other checks (like comment checks) to still allow extra lines when necessary.
* Updating `setFlags()` in `build/print_test.go`.
  * Checking golden filenames for ".compactconst." to set `CompactConstantDefinitions` to true.
  * Adding `CompactConstantDefinitions` reset back to false in returned (deferred) func.
* Copying various golden files (which contain constant definitions) to be ".compactconst." files.
  * Removing extra lines between const definitions as applicable.

Fixes bazelbuild#108.
  • Loading branch information
CauhxMilloy committed Feb 13, 2024
1 parent 03bf520 commit 895be84
Show file tree
Hide file tree
Showing 13 changed files with 289 additions and 0 deletions.
9 changes: 9 additions & 0 deletions build/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions build/print_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
6 changes: 6 additions & 0 deletions build/testdata/004.compactconst.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
JAVA_FILES = [
"Foo.java",
"Bar.java",
"Baz.java",
"Quux.java",
]
7 changes: 7 additions & 0 deletions build/testdata/005.compactconst.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
JAVA_FILES = [
# Comment regarding Foo.java
"Foo.java",
"Bar.java",
"Baz.java", # Comment regarding Baz.java
"Quux.java",
]
30 changes: 30 additions & 0 deletions build/testdata/006.compactconst.build.golden
Original file line number Diff line number Diff line change
@@ -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"),
)
21 changes: 21 additions & 0 deletions build/testdata/006.compactconst.bzl.golden
Original file line number Diff line number Diff line change
@@ -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,
)
37 changes: 37 additions & 0 deletions build/testdata/017.compactconst.golden
Original file line number Diff line number Diff line change
@@ -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")
12 changes: 12 additions & 0 deletions build/testdata/043.compactconst.golden
Original file line number Diff line number Diff line change
@@ -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[:]
31 changes: 31 additions & 0 deletions build/testdata/066.compactconst.build.golden
Original file line number Diff line number Diff line change
@@ -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
31 changes: 31 additions & 0 deletions build/testdata/066.compactconst.bzl.golden
Original file line number Diff line number Diff line change
@@ -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
53 changes: 53 additions & 0 deletions build/testdata/067.compactconst.build.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
load(
# A
"foo.bzl",
# B
"bar",
)
load(":a", "b")

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)
46 changes: 46 additions & 0 deletions build/testdata/067.compactconst.bzl.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
load(
# A
"foo.bzl",
# B
"bar",
)
load(":a", "b")

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)
2 changes: 2 additions & 0 deletions tables/tables.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ var NamePriority = map[string]int{
"alwayslink": 7,
}

var CompactConstantDefinitions = false

var StripLabelLeadingSlashes = false

var ShortenAbsoluteLabelsToRelative = false
Expand Down

0 comments on commit 895be84

Please sign in to comment.