Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions pkg/linters/regexpcompileinfunction/regexpcompileinfunction.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func run(pass *analysis.Pass) (any, error) {

for cur := range insp.Root().Preorder((*ast.CallExpr)(nil)) {
call, ok := cur.Node().(*ast.CallExpr)
if !ok || !isRegexpCompileCall(call) {
if !ok || !isRegexpCompileCall(pass, call) {
continue
}
if !hasConstantStringPattern(pass, call) {
Expand Down Expand Up @@ -68,17 +68,30 @@ func run(pass *analysis.Pass) (any, error) {
return nil, nil
}

// isRegexpCompileCall checks if the call is to regexp.MustCompile or regexp.Compile.
func isRegexpCompileCall(call *ast.CallExpr) bool {
// isRegexpCompileCall checks if the call is to regexp.MustCompile or regexp.Compile,
// resolving the package identity via the type checker to handle aliased imports
// and avoid false positives from local identifiers named "regexp".
func isRegexpCompileCall(pass *analysis.Pass, call *ast.CallExpr) bool {
sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
return false
}
if sel.Sel.Name != "MustCompile" && sel.Sel.Name != "Compile" {
return false
}
ident, ok := sel.X.(*ast.Ident)
if !ok {
if !ok || pass.TypesInfo == nil {
return false
}
obj := pass.TypesInfo.ObjectOf(ident)
if obj == nil {
return false
}
pkgName, ok := obj.(*types.PkgName)
if !ok || pkgName.Imported() == nil {
return false
}
return ident.Name == "regexp" && (sel.Sel.Name == "MustCompile" || sel.Sel.Name == "Compile")
return pkgName.Imported().Path() == "regexp"
}

// hasConstantStringPattern checks whether the regexp pattern is a compile-time constant string,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package regexpcompileinfunction

import re "regexp"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undocumented false negative: import . "regexp" is silently skipped. With a dot import, call sites are bare identifiers (MustCompile(...)) rather than selector expressions, so isRegexpCompileCall exits early at the *ast.SelectorExpr guard and produces no diagnostic.

💡 Details

This is a pre-existing limitation, not introduced by this PR, but the PR adds test fixtures specifically targeting import-aliasing edge cases without mentioning this gap. Either add a comment in the function doc noting the limitation, or add a dot_import.go fixture that documents the known miss:

package regexpcompileinfunction

import . "regexp"

// NOT flagged (known limitation: dot imports are not selector expressions).
func DotImportExample(s string) bool {
    r := MustCompile(`^[a-z]+$`) // no // want — dot-import calls are silently skipped
    return r.MatchString(s)
}

Without this, a future refactor that adds dot-import detection would have no regression test to verify the change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added dot_import.go in the latest commit to document this known limitation. The fixture confirms dot-import calls are silently skipped (no // want annotation) and includes a comment explaining why: dot imports produce bare identifier calls, not selector expressions, so isRegexpCompileCall exits early at the *ast.SelectorExpr guard.


// flagged: aliased import of regexp — alias must not cause a false negative.
func ProcessWithAlias(s string) bool {
r := re.MustCompile(`^[a-z]+$`) // want `regexp compilation inside function should be moved to package-level variable`
return r.MatchString(s)
}

func ValidateWithAlias(input string) (bool, error) {
r, err := re.Compile(`\d+`) // want `regexp compilation inside function should be moved to package-level variable`
if err != nil {
return false, err
}
return r.MatchString(input), nil
}

// not flagged: aliased import at package level is fine.
var packageLevelAliasRegexp = re.MustCompile(`^[a-z]+$`)
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package regexpcompileinfunction

import . "regexp"

// NOT flagged: dot imports produce bare identifier calls, not selector expressions,
// so isRegexpCompileCall exits early at the *ast.SelectorExpr guard.
// This is a known limitation of the linter.
func DotImportExample() bool {
r := MustCompile(`^[a-z]+$`) // not flagged: dot-import calls are not selector expressions
return r.MatchString("abc")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package regexpcompileinfunction

// customRegexp is an unrelated type that happens to have Compile/MustCompile methods.
type customRegexp struct{}

func (customRegexp) Compile(_ string) (*customRegexp, error) { return &customRegexp{}, nil }
func (customRegexp) MustCompile(_ string) *customRegexp { return &customRegexp{} }

// not flagged: local variable named "regexp" is not the stdlib regexp package.
func GoodShadowedRegexpIdentifier() bool {
regexp := customRegexp{}
_ = regexp.MustCompile(`^[a-z]+$`)
return true
}