Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: ashanbrown/forbidigo
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v2.0.1
Choose a base ref
...
head repository: ashanbrown/forbidigo
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v2.1.0
Choose a head ref
  • 1 commit
  • 5 files changed
  • 2 contributors

Commits on Jan 25, 2025

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

    Treat builtins as having an empty string package.
    
    ---------
    
    Co-authored-by: Iegor Sergieienkov <iegor@nova-labs.com>
    ashanbrown and isergieienkov authored Jan 25, 2025

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    Copy the full SHA
    77ce5aa View commit details
Showing with 33 additions and 10 deletions.
  1. +5 −3 README.md
  2. +12 −7 forbidigo/forbidigo.go
  3. +11 −0 forbidigo/forbidigo_test.go
  4. +1 −0 pkg/analyzer/analyzer_test.go
  5. +4 −0 pkg/analyzer/testdata/src/expandtext/expandtext.go
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 12 additions & 7 deletions forbidigo/forbidigo.go
Original file line number Diff line number Diff line change
@@ -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:
11 changes: 11 additions & 0 deletions forbidigo/forbidigo_test.go
Original file line number Diff line number Diff line change
@@ -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, `
1 change: 1 addition & 0 deletions pkg/analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
@@ -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 {
4 changes: 4 additions & 0 deletions pkg/analyzer/testdata/src/expandtext/expandtext.go
Original file line number Diff line number Diff line change
@@ -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 {