Support forbidding access to methods on builtin identifiers (#59)

Treat builtins as having an empty string package.

---------

Co-authored-by: Iegor Sergieienkov <[email protected]>

README.md

@@ -29,7 +29,9 @@ name is so generic that import aliases have to be used. To solve this,
 forbidigo also supports a more advanced mode where it uses type information to
 identify what an expression references. This needs to be enabled through the
 `analyze_types` command line parameter. Beware this may have a performance
-impact because additional information is required for the analysis.
+impact because additional information is required for the analysis.  Note that 
+[builtin types](https://pkg.go.dev/builtin) types (`error`, `byte`, etc) are 
+considered to have the package name "" (emptry string).
 
 Replacing the literal source code works for items in a package as in the
 `fmt2.Print` example above and also for struct fields and methods. For those,
@@ -101,8 +103,8 @@ The full pattern struct has the following fields:
 
 * `msg`: an additional comment that gets added to the error message when a
   pattern matches.
-* `p`: the regular expression that matches the source code or expanded
-  expression, depending on the global flag.
+* `p`: the regular expression that matches the source code or, when `analyze_flags` is set, the expanded
+  expression including the package name.
 * `pkg`: a regular expression for the full package import path. The package
   path includes the package version if the package has a version >= 2. This is
   only supported when `analyze_types` is enabled.

forbidigo/forbidigo.go

@@ -317,13 +317,15 @@ func (v *visitor) expandMatchText(node ast.Node, srcText string) (matchTexts []s
 		// type. We don't care about the value.
 		selectorText := v.textFor(node)
 		if typeAndValue, ok := v.runConfig.TypesInfo.Types[selector]; ok {
-			m, p, ok := typeNameWithPackage(typeAndValue.Type)
-			if !ok {
-				v.runConfig.DebugLog("%s: selector %q with supported type %T", location, selectorText, typeAndValue.Type)
+			if typeName, pkgPath, ok := typeNameWithPackage(typeAndValue.Type); ok {
+				v.runConfig.DebugLog("%s: selector %q with supported type %q: %q -> %q, package %q", location, selectorText, typeAndValue.Type.String(), srcText, matchTexts, pkgPath)
+				matchTexts = []string{typeName + "." + field}
+				pkgText = pkgPath
+			} else {
+				// handle cases such as anonymous structs
+				v.runConfig.DebugLog("%s: selector %q with unknown type %T", location, selectorText, typeAndValue.Type)
+				matchTexts = []string{}
 			}
-			matchTexts = []string{m + "." + field}
-			pkgText = p
-			v.runConfig.DebugLog("%s: selector %q with supported type %q: %q -> %q, package %q", location, selectorText, typeAndValue.Type.String(), srcText, matchTexts, pkgText)
 		}
 		// Some expressions need special treatment.
 		switch selector := selector.(type) {
@@ -340,7 +342,9 @@ func (v *visitor) expandMatchText(node ast.Node, srcText string) (matchTexts []s
 						pkgText = packageName
 						v.runConfig.DebugLog("%s: selector %q is variable of type %q: %q -> %q, package %q", location, selectorText, object.Type().String(), srcText, matchTexts, pkgText)
 					} else {
+						// handle cases such as anonymous structs
 						v.runConfig.DebugLog("%s: selector %q is variable with unsupported type %T", location, selectorText, object.Type())
+						matchTexts = []string{}
 					}
 				default:
 					// Something else?
@@ -373,8 +377,9 @@ func typeNameWithPackage(t types.Type) (typeName, packagePath string, ok bool) {
 	case *types.Named:
 		obj := t.Obj()
 		pkg := obj.Pkg()
+		// we either lack a package or the package is the "universe" (i.e. builtin)
 		if pkg == nil {
-			return "", "", false
+			return obj.Name(), "", true
 		}
 		return pkg.Name() + "." + obj.Name(), pkg.Path(), true
 	default:

forbidigo/forbidigo_test.go

@@ -23,6 +23,17 @@ func foo() {
 }`, "use of `fmt.Printf` forbidden by pattern `fmt\\.Printf` at testing.go:5:2")
 	})
 
+	t.Run("it finds forbidden identifiers in builtin package", func(t *testing.T) {
+		linter, _ := NewLinter([]string{`^error\.Error$`}, OptionAnalyzeTypes(true))
+		expectIssues(t, linter, true, `
+package bar
+
+func foo() {
+	var err error
+	err.Error()
+}`, "use of `err.Error` forbidden by pattern `^error\\.Error$` at testing.go:6:2")
+	})
+
 	t.Run("it finds forbidden, renamed identifiers", func(t *testing.T) {
 		linter, _ := NewLinter([]string{`fmt\.Printf`}, OptionAnalyzeTypes(true))
 		expectIssues(t, linter, true, `

pkg/analyzer/analyzer_test.go

@@ -36,6 +36,7 @@ func TestExpandAnalyzer(t *testing.T) {
 		`{p: myCustomInterface\.AlsoForbidden, pkg: ^expandtext$}`,
 		`{p: renamed\.Forbidden, pkg: ^example.com/some/renamedpkg$}`,
 		`{p: renamed\.Struct.Forbidden, pkg: ^example.com/some/renamedpkg$}`,
+		`{p: ^error\.Error$}`,
 	)
 	a := newAnalyzer(t.Logf)
 	for _, pattern := range patterns {

pkg/analyzer/testdata/src/expandtext/expandtext.go

@@ -69,6 +69,10 @@ func Foo() {
 	// Package name != import path.
 	renamed.ForbiddenFunc()            // want "renamed.Forbidden.* by pattern .*renamed..Forbidden"
 	renamed.Struct{}.ForbiddenMethod() // want "renamed.Struct...ForbiddenMethod.* by pattern .*renamed.*Struct.*Forbidden"
+
+	// Builtin type.
+	err := error(nil)
+	err.Error() // want "err\\.Error.*forbidden by pattern.*\\^error.*Error\\$"
 }
 
 func Bar() string {