Repository: bazelbuild/buildtools
Pull request #1061 (2022-04-12 22:01:12)

Fix symbols considered unused when referenced as types
This fix is trying to address the issue of symbols considered unused when imported and used as types only

Example:

```
load(
  ":f.bzl",
  "s1",
  "s2",
)
def test(x: s1) -> List[s2]:
  pass
```

The PR removes the need for using the `# @unused` directive.


Patch:
diff --git a/build/utils.go b/build/utils.go
index ba3f7a6..1e7c2b9 100644
--- a/build/utils.go
+++ b/build/utils.go
@@ -46,3 +46,36 @@ func GetParamIdent(param Expr) (ident *Ident, op string) {
 	}
 	return nil, ""
 }
+
+// GetTypes returns the list of types defined by the a given expression.
+// Examples:
+//
+// List[tuple[bool, int]] should return [List, Tuple, bool, int]
+// str should return str
+func GetTypes(t Expr) []string {
+	switch t := t.(type) {
+	case *TypedIdent:
+		return GetTypes(t.Type)
+	case *Ident:
+		return []string{t.Name}
+	case *DefStmt:
+		ret := GetTypes(t.Type)
+		params := make([]string, 0)
+		for _, p := range t.Params {
+			params = append(params, GetTypes(p)...)
+		}
+		return append(ret, params...)
+	case *IndexExpr:
+		left := GetTypes(t.X)
+		right := GetTypes(t.Y)
+		return append(left, right...)
+	case *DotExpr:
+		// Special handling for skylark-rust interpreter, types are referred to by a `.type` suffix
+		if t.Name == "type" {
+			return GetTypes(t.X)
+		}
+		return []string{}
+	default:
+		return []string{}
+	}
+}
diff --git a/edit/edit.go b/edit/edit.go
index 3848e78..a461011 100644
--- a/edit/edit.go
+++ b/edit/edit.go
@@ -968,6 +968,31 @@ func UsedSymbols(stmt build.Expr) map[string]bool {
 	return symbols
 }
 
+// UsedTypes returns the set of types used in the BUILD file (variables, function names).
+func UsedTypes(stmt build.Expr) map[string]bool {
+	symbols := make(map[string]bool)
+	build.Walk(stmt, func(expr build.Expr, stack []build.Expr) {
+		// Don't traverse inside load statements
+		if len(stack) > 0 {
+			if _, ok := stack[len(stack)-1].(*build.LoadStmt); ok {
+				return
+			}
+		}
+		// Types can only be found in method declarations and
+		switch expr := expr.(type) {
+		case *build.TypedIdent:
+			for _, t := range build.GetTypes(expr) {
+				symbols[t] = true
+			}
+		case *build.DefStmt:
+			for _, t := range build.GetTypes(expr) {
+				symbols[t] = true
+			}
+		}
+	})
+	return symbols
+}
+
 // NewLoad creates a new LoadStmt node
 func NewLoad(location string, from, to []string) *build.LoadStmt {
 	load := &build.LoadStmt{
diff --git a/warn/warn_control_flow.go b/warn/warn_control_flow.go
index a594c03..ed9da0a 100644
--- a/warn/warn_control_flow.go
+++ b/warn/warn_control_flow.go
@@ -578,6 +578,7 @@ func unusedLoadWarning(f *build.File) []*LinterFinding {
 	})
 
 	symbols := edit.UsedSymbols(f)
+	types := edit.UsedTypes(f)
 	for stmtIndex := 0; stmtIndex < len(f.Stmt); stmtIndex++ {
 		originalLoad, ok := f.Stmt[stmtIndex].(*build.LoadStmt)
 		if !ok {
@@ -620,6 +621,10 @@ func unusedLoadWarning(f *build.File) []*LinterFinding {
 				continue
 			}
 			_, ok := symbols[to.Name]
+			if !ok {
+				// Fallback to verify if the symbol is used as a type.
+				_, ok = types[to.Name]
+			}
 			if !ok && !edit.ContainsComments(originalLoad, "@unused") && !edit.ContainsComments(to, "@unused") && !edit.ContainsComments(from, "@unused") {
 				// The loaded symbol is not used and is not protected by a special "@unused" comment
 				load.To = append(load.To[:i], load.To[i+1:]...)


Test patch:
diff --git a/warn/warn_control_flow_test.go b/warn/warn_control_flow_test.go
index e72ea84..a30a1b6 100644
--- a/warn/warn_control_flow_test.go
+++ b/warn/warn_control_flow_test.go
@@ -805,6 +805,58 @@ x = "unused"`, `
 x = "unused"`,
 		[]string{":1: Loaded symbol \"x\" is unused."},
 		scopeEverywhere)
+
+	checkFindings(t, "load", `
+load(
+  ":f.bzl",
+   "s1",
+)
+
+def test(x: s1):
+  pass
+`,
+		[]string{},
+		scopeEverywhere)
+	checkFindings(t, "load", `
+load(
+  ":f.bzl",
+  "s1",
+  "s2",
+)
+
+def test(x: s1) -> List[s2]:
+  pass
+`,
+		[]string{},
+		scopeEverywhere)
+	checkFindingsAndFix(t, "load", `
+load(
+  ":f.bzl",
+  "s1",
+  "s2",
+)
+
+load(
+  ":s.bzl",
+  "s3",
+)
+
+def test(x: s1) -> List[s2]:
+  pass
+`, `
+load(
+  ":f.bzl",
+  "s1",
+  "s2",
+)
+
+def test(x: s1) -> List[s2]:
+  pass
+`,
+		[]string{
+			":9: Loaded symbol \"s3\" is unused.",
+		},
+		scopeEverywhere)
 }
 
 func TestUninitializedVariable(t *testing.T) {