Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Linting Regex #43

Merged
merged 6 commits into from
Aug 4, 2024
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
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ require (
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/stretchr/objx v0.5.2 // indirect
go.uber.org/multierr v1.10.0 // indirect
golang.org/x/mod v0.19.0 // indirect
golang.org/x/sync v0.7.0 // indirect
golang.org/x/sys v0.22.0 // indirect
)

Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,16 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ=
go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8=
go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E=
golang.org/x/mod v0.19.0 h1:fEdghXQSo20giMthA7cd28ZC+jts4amQ3YMXiP5oMQ8=
golang.org/x/mod v0.19.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI=
Expand Down
1 change: 1 addition & 0 deletions internal/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func (e *Engine) registerDefaultRules() {
&EmitFormatRule{},
&DetectCycleRule{},
&GnoSpecificRule{},
&RepeatedRegexCompilationRule{},
&UselessBreakRule{},
)
}
Expand Down
95 changes: 95 additions & 0 deletions internal/lints/regex_lint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package lints

import (
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestDetectRepeatedRegexCompilation(t *testing.T) {
tests := []struct {
name string
code string
expected int
}{
{
name: "No repeated compilation",
code: `
package main

import "regexp"

func noRepeat() {
r1 := regexp.MustCompile("pattern1")
r2 := regexp.MustCompile("pattern2")
_ = r1
_ = r2
}
`,
expected: 0,
},
{
name: "Repeated compilation",
code: `
package main

import "regexp"

func withRepeat() {
r1 := regexp.MustCompile("pattern")
r2 := regexp.MustCompile("pattern")
_ = r1
_ = r2
}
`,
expected: 1,
},
{
name: "Multiple repeated compilations",
code: `
package main

import "regexp"

func multipleRepeats() {
r1 := regexp.MustCompile("pattern1")
r2 := regexp.MustCompile("pattern1")
r3 := regexp.MustCompile("pattern2")
r4 := regexp.MustCompile("pattern2")
_ = r1
_ = r2
_ = r3
_ = r4
}
`,
expected: 2,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tempDir, err := os.MkdirTemp("", "regex-test")
require.NoError(t, err)
defer os.RemoveAll(tempDir)

tempFile := filepath.Join(tempDir, "test.go")
err = os.WriteFile(tempFile, []byte(tt.code), 0o644)
require.NoError(t, err)

issues, err := DetectRepeatedRegexCompilation(tempFile)
require.NoError(t, err)

assert.Len(t, issues, tt.expected)

if tt.expected > 0 {
for _, issue := range issues {
assert.Equal(t, "repeatedregexcompilation", issue.Rule)
assert.Contains(t, issue.Message, "regexp.Compile called with same pattern more than once")
}
}
})
}
}
120 changes: 120 additions & 0 deletions internal/lints/repeated_regex_compilation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package lints

import (
"go/ast"
"go/token"

tt "github.com/gnoswap-labs/lint/internal/types"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"
)

var RepeatedRegexCompilationAnalyzer = &analysis.Analyzer{
Name: "repeatedregexcompilation",
Doc: "Checks for repeated compilation of the same regex pattern",
Run: runRepeatedRegexCompilation,
}

func DetectRepeatedRegexCompilation(filename string) ([]tt.Issue, error) {
issues, err := runAnalyzer(filename, RepeatedRegexCompilationAnalyzer)
if err != nil {
return nil, err
}
return issues, nil
}

func runAnalyzer(filename string, a *analysis.Analyzer) ([]tt.Issue, error) {
cfg := &packages.Config{
Mode: packages.NeedFiles | packages.NeedSyntax | packages.NeedTypesInfo | packages.NeedTypes,
Tests: false,
}

pkgs, err := packages.Load(cfg, filename)
if err != nil {
return nil, err
}

var diagnostics []analysis.Diagnostic
pass := &analysis.Pass{
Analyzer: a,
Fset: pkgs[0].Fset,
Files: pkgs[0].Syntax,
Pkg: pkgs[0].Types,
TypesInfo: pkgs[0].TypesInfo,
ResultOf: make(map[*analysis.Analyzer]interface{}),
Report: func(d analysis.Diagnostic) {
diagnostics = append(diagnostics, d)
},
}

_, err = a.Run(pass)
if err != nil {
return nil, err
}

var issues []tt.Issue
for _, diag := range diagnostics {
issues = append(issues, tt.Issue{
Rule: a.Name,
Filename: filename,
Start: pass.Fset.Position(diag.Pos),
End: pass.Fset.Position(diag.End),
Message: diag.Message,
})
}

return issues, nil
}

func runRepeatedRegexCompilation(pass *analysis.Pass) (interface{}, error) {
for _, file := range pass.Files {
ast.Inspect(file, func(n ast.Node) bool {
funcDecl, ok := n.(*ast.FuncDecl)
if !ok {
return true
}

regexPatterns := make(map[string]token.Pos)
ast.Inspect(funcDecl.Body, func(node ast.Node) bool {
callExpr, ok := node.(*ast.CallExpr)
if !ok {
return true
}

if isRegexpCompile(callExpr) {
if pattern, ok := getRegexPattern(callExpr); ok {
if firstPos, exists := regexPatterns[pattern]; exists {
pass.Reportf(callExpr.Pos(), "regexp.Compile called with same pattern more than once. First occurrence at line %d", pass.Fset.Position(firstPos).Line)
} else {
regexPatterns[pattern] = callExpr.Pos()
}
}
}

return true
})

return true
})
}

return nil, nil
}

func isRegexpCompile(callExpr *ast.CallExpr) bool {
if selExpr, ok := callExpr.Fun.(*ast.SelectorExpr); ok {
if ident, ok := selExpr.X.(*ast.Ident); ok {
return ident.Name == "regexp" && (selExpr.Sel.Name == "Compile" || selExpr.Sel.Name == "MustCompile")
}
}
return false
}

func getRegexPattern(callExpr *ast.CallExpr) (string, bool) {
if len(callExpr.Args) > 0 {
if lit, ok := callExpr.Args[0].(*ast.BasicLit); ok && lit.Kind == token.STRING {
return lit.Value, true
}
}
return "", false
}
13 changes: 13 additions & 0 deletions internal/rule_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,19 @@ func (r *UselessBreakRule) Name() string {
return "useless-break"
}

// -----------------------------------------------------------------------------
// Regex related rules

type RepeatedRegexCompilationRule struct{}

func (r *RepeatedRegexCompilationRule) Check(filename string, node *ast.File, fset *token.FileSet) ([]tt.Issue, error) {
return lints.DetectRepeatedRegexCompilation(filename)
}

func (r *RepeatedRegexCompilationRule) Name() string {
return "repeated-regex-compilation"
}

// -----------------------------------------------------------------------------

type CyclomaticComplexityRule struct {
Expand Down
15 changes: 15 additions & 0 deletions testdata/regex/regex0.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package main

import (
"regexp"

"gno.land/p/demo/ufmt"
)

func repeatedRegexCompilation(inputs []string) {
for _, input := range inputs {
regex := regexp.MustCompile(`\d+`)
matches := regex.FindAllString(input, -1)
ufmt.Println(matches)
}
}
16 changes: 16 additions & 0 deletions testdata/regex/regex1.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package main

import (
"regexp"

"gno.land/p/demo/ufmt"
)


func avoidRepeatedRegexCompilation(inputs []string) {
regex := regexp.MustCompile(`\d+`)
for _, input := range inputs {
matches := regex.FindAllString(input, -1)
ufmt.Println(matches)
}
}
9 changes: 9 additions & 0 deletions testdata/regex/regex2.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package main

import (
"regexp"
)

func complexRegexExample() {
regex := regexp.MustCompile(`^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$`)
}
10 changes: 10 additions & 0 deletions testdata/regex/regex3.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package main

import "regexp"

func unnecessaryCaptureGroupExample() {
regex := regexp.MustCompile(`(https?://\S+)`)
}

// minimized:
// (?:https?://\S+)`
11 changes: 11 additions & 0 deletions testdata/regex/regex4.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package main

import "regexp"

func greedyQuantifierExample(largeInput string) {
regex := regexp.MustCompile(`<.*>`)
matches := regex.FindAllString(largeInput, -1)
}

// minimized:
// <.*?>
17 changes: 17 additions & 0 deletions testdata/regex/regex5.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package main

import (
"regexp"
)

func preferStringMethodExample(input string) bool {
return regexp.MustCompile(`^[a-zA-Z]+$`).MatchString(input)
}

// Suggestion:
// for _, char := range input {
// if !unicode.IsLetter(char) {
// return false
// }
// }
// return true
10 changes: 10 additions & 0 deletions testdata/regex/regex6.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package main

import "regexp"

func withRepeat() {
r1 := regexp.MustCompile("pattern")
r2 := regexp.MustCompile("pattern")
_ = r1
_ = r2
}
Loading