Skip to content

Commit

Permalink
check clear/max/min in redefines-builtin-id rule on go1.21+ (#1024)
Browse files Browse the repository at this point in the history
* check clear/max/min in redefines-builtin-id rule on go1.21+

* Update rule/redefines-builtin-id.go

Co-authored-by: Denis Voytyuk <[email protected]>

---------

Co-authored-by: Denis Voytyuk <[email protected]>
  • Loading branch information
skaji and denisvmedia authored Aug 15, 2024
1 parent a638ed6 commit e54773e
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 6 deletions.
6 changes: 6 additions & 0 deletions lint/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var (
falseValue = 2
notSet = 3

go121 = goversion.Must(goversion.NewVersion("1.21"))
go122 = goversion.Must(goversion.NewVersion("1.22"))
)

Expand Down Expand Up @@ -194,6 +195,11 @@ func (p *Package) lint(rules []Rule, config Config, failures chan Failure) {
wg.Wait()
}

// IsAtLeastGo121 returns true if the Go version for this package is 1.21 or higher, false otherwise
func (p *Package) IsAtLeastGo121() bool {
return p.goVersion.GreaterThanOrEqual(go121)
}

// IsAtLeastGo122 returns true if the Go version for this package is 1.22 or higher, false otherwise
func (p *Package) IsAtLeastGo122() bool {
return p.goVersion.GreaterThanOrEqual(go122)
Expand Down
32 changes: 26 additions & 6 deletions rule/redefines-builtin-id.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"go/ast"
"go/token"
"maps"

"github.com/mgechev/revive/lint"
)
Expand Down Expand Up @@ -33,6 +34,12 @@ var builtFunctions = map[string]bool{
"recover": true,
}

var builtFunctionsAfterGo121 = map[string]bool{
"clear": true,
"max": true,
"min": true,
}

var builtInTypes = map[string]bool{
"bool": true,
"byte": true,
Expand Down Expand Up @@ -69,7 +76,17 @@ func (*RedefinesBuiltinIDRule) Apply(file *lint.File, _ lint.Arguments) []lint.F
}

astFile := file.AST
w := &lintRedefinesBuiltinID{onFailure}

builtFuncs := maps.Clone(builtFunctions)
if file.Pkg.IsAtLeastGo121() {
maps.Copy(builtFuncs, builtFunctionsAfterGo121)
}
w := &lintRedefinesBuiltinID{
onFailure: onFailure,
builtInConstAndVars: builtInConstAndVars,
builtFunctions: builtFuncs,
builtInTypes: builtInTypes,
}
ast.Walk(w, astFile)

return failures
Expand All @@ -81,7 +98,10 @@ func (*RedefinesBuiltinIDRule) Name() string {
}

type lintRedefinesBuiltinID struct {
onFailure func(lint.Failure)
onFailure func(lint.Failure)
builtInConstAndVars map[string]bool
builtFunctions map[string]bool
builtInTypes map[string]bool
}

func (w *lintRedefinesBuiltinID) Visit(node ast.Node) ast.Visitor {
Expand Down Expand Up @@ -162,16 +182,16 @@ func (w lintRedefinesBuiltinID) addFailure(node ast.Node, msg string) {
})
}

func (lintRedefinesBuiltinID) isBuiltIn(id string) (r bool, builtInKind string) {
if builtFunctions[id] {
func (w *lintRedefinesBuiltinID) isBuiltIn(id string) (r bool, builtInKind string) {
if w.builtFunctions[id] {
return true, "function"
}

if builtInConstAndVars[id] {
if w.builtInConstAndVars[id] {
return true, "constant or variable"
}

if builtInTypes[id] {
if w.builtInTypes[id] {
return true, "type"
}

Expand Down
9 changes: 9 additions & 0 deletions testdata/redefines-builtin-id.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,12 @@ var i, copy int // MATCH /redefinition of the built-in function copy/

// issue #792
type ()

func foo() {
clear := 0 // MATCH /redefinition of the built-in function clear/
max := 0 // MATCH /redefinition of the built-in function max/
min := 0 // MATCH /redefinition of the built-in function min/
_ = clear
_ = max
_ = min
}

0 comments on commit e54773e

Please sign in to comment.