Repository: bazelbuild/buildtools
Pull request #743 (2019-11-15 14:53:01)

Implement list-append warning and fix
Warn about and fix single-element list concatenations.


Patch:
diff --git a/WARNINGS.md b/WARNINGS.md
index d38e931..e38264a 100644
--- a/WARNINGS.md
+++ b/WARNINGS.md
@@ -26,6 +26,7 @@ Warning categories supported by buildifier's linter:
   * [http-archive](#http-archive)
   * [integer-division](#integer-division)
   * [keyword-positional-params](#keyword-positional-params)
+  * [list-append](#list-append)
   * [load](#load)
   * [load-on-top](#load-on-top)
   * [module-docstring](#module-docstring)
@@ -438,7 +439,16 @@ Some parameters for builtin functions in Starlark are keyword for legacy reasons
 their names are not meaningful (e.g. `x`). Making them positional-only will improve
 the readability.
 
- --------------------------------------------------------------------------------
+--------------------------------------------------------------------------------
+
+## <a name="list-append"></a>Prefer using ".append()" to adding a single element list
+
+  * Category name: `list-append`
+  * Automatic fix: yes
+
+Transforming `x += [expr]` to `x.append(expr)` avoids a list allocation.
+
+--------------------------------------------------------------------------------
 
 ## <a name="load"></a>Loaded symbol is unused
 
diff --git a/warn/warn.go b/warn/warn.go
index 2e7c75b..8ddeebd 100644
--- a/warn/warn.go
+++ b/warn/warn.go
@@ -124,6 +124,7 @@ var FileWarningMap = map[string]func(f *build.File) []*LinterFinding{
 	"http-archive":              nativeHTTPArchiveWarning,
 	"integer-division":          integerDivisionWarning,
 	"keyword-positional-params": keywordPositionalParametersWarning,
+	"list-append":               listAppendWarning,
 	"load":                      unusedLoadWarning,
 	"load-on-top":               loadOnTopWarning,
 	"module-docstring":          moduleDocstringWarning,
diff --git a/warn/warn_operation.go b/warn/warn_operation.go
index 30c7c26..6885247 100644
--- a/warn/warn_operation.go
+++ b/warn/warn_operation.go
@@ -210,3 +210,33 @@ func stringEscapeWarning(f *build.File) []*LinterFinding {
 	})
 	return findings
 }
+
+func listAppendWarning(f *build.File) []*LinterFinding {
+	var findings []*LinterFinding
+
+	build.WalkPointers(f, func(expr *build.Expr, stack []build.Expr) {
+		as, ok := (*expr).(*build.AssignExpr)
+		if !ok || as.Op != "+=" {
+			return
+		}
+
+		list, ok := as.RHS.(*build.ListExpr)
+		if !ok || len(list.List) != 1 {
+			return
+		}
+
+		_, end := as.Span()
+		findings = append(findings, makeLinterFinding(as, `Prefer using ".append()" to adding a single element list.`,
+			LinterReplacement{expr, &build.CallExpr{
+				Comments: as.Comments,
+				X: &build.DotExpr{
+					X:    as.LHS,
+					Name: "append",
+				},
+				List: list.List,
+				End:  build.End{Pos: end},
+			}}))
+
+	})
+	return findings
+}


Test patch:
diff --git a/warn/warn_operation_test.go b/warn/warn_operation_test.go
index c70292e..2d5d3b5 100644
--- a/warn/warn_operation_test.go
+++ b/warn/warn_operation_test.go
@@ -151,3 +151,34 @@ r'''
 		[]string{},
 		scopeEverywhere)
 }
+
+func TestListAppend(t *testing.T) {
+	checkFindingsAndFix(t, "list-append", `
+x = []
+x += y
+x += [1]
+x += [2, 3]
+x += [4 for y in z]
+x += 5
+x += [foo(
+    bar,
+    baz,
+)]
+`, `
+x = []
+x += y
+x.append(1)
+x += [2, 3]
+x += [4 for y in z]
+x += 5
+x.append(foo(
+    bar,
+    baz,
+))
+`,
+		[]string{
+			`:3: Prefer using ".append()" to adding a single element list`,
+			`:7: Prefer using ".append()" to adding a single element list`,
+		},
+		scopeEverywhere)
+}