Skip to content

Commit

Permalink
feat: Linting Regex (#43)
Browse files Browse the repository at this point in the history
* add regex examples

* detect repeated regex compilation

* fix: Improve slice bounds check lint for range loop contexts (#45)

* fix: Improve slice bounds check lint for range loop contexts

* update docs
  • Loading branch information
notJoon authored Aug 4, 2024
1 parent a943952 commit f25b3cb
Show file tree
Hide file tree
Showing 13 changed files with 325 additions and 0 deletions.
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
}

0 comments on commit f25b3cb

Please sign in to comment.