Skip to content

Commit

Permalink
Implement linter for checking 1>>x expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
anodar committed Jan 29, 2024
1 parent 3cb5f77 commit a4cbf3b
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 15 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ contracts/test/prover/proofs/%.json: $(arbitrator_cases)/%.wasm $(arbitrator_pro
.make/lint: $(DEP_PREDICATE) build-node-deps $(ORDER_ONLY_PREDICATE) .make
go run ./linter/koanf ./...
go run ./linter/pointercheck ./...
go run ./linter/rightshift ./...
golangci-lint run --fix
yarn --cwd contracts solhint
@touch $@
Expand Down
4 changes: 0 additions & 4 deletions linter/koanf/koanf.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ var (
errIncorrectFlag = errors.New("mismatching flag initialization")
)

func New(conf any) ([]*analysis.Analyzer, error) {
return []*analysis.Analyzer{Analyzer}, nil
}

var Analyzer = &analysis.Analyzer{
Name: "koanfcheck",
Doc: "check for koanf misconfigurations",
Expand Down
2 changes: 1 addition & 1 deletion linter/koanf/koanf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func testData(t *testing.T) string {
t.Helper()
wd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get wd: %s", err)
t.Fatalf("Failed to get working directory: %v", err)
}
return filepath.Join(filepath.Dir(wd), "testdata")
}
Expand Down
4 changes: 0 additions & 4 deletions linter/pointercheck/pointer.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ import (
"golang.org/x/tools/go/analysis/singlechecker"
)

func New(conf any) ([]*analysis.Analyzer, error) {
return []*analysis.Analyzer{Analyzer}, nil
}

var Analyzer = &analysis.Analyzer{
Name: "pointercheck",
Doc: "check for pointer comparison",
Expand Down
2 changes: 1 addition & 1 deletion linter/pointercheck/pointer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
func TestAll(t *testing.T) {
wd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get wd: %s", err)
t.Fatalf("Failed to get working directory: %v", err)
}
testdata := filepath.Join(filepath.Dir(wd), "testdata")
res := analysistest.Run(t, testdata, analyzerForTests, "pointercheck")
Expand Down
76 changes: 76 additions & 0 deletions linter/rightshift/rightshift.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package main

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

"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/singlechecker"
)

var Analyzer = &analysis.Analyzer{
Name: "rightshift",
Doc: "check for 1 >> x operation",
Run: func(p *analysis.Pass) (interface{}, error) { return run(false, p) },
ResultType: reflect.TypeOf(Result{}),
}

var analyzerForTests = &analysis.Analyzer{
Name: "testrightshift",
Doc: "check for pointer comparison (for tests)",
Run: func(p *analysis.Pass) (interface{}, error) { return run(true, p) },
ResultType: reflect.TypeOf(Result{}),
}

// rightShiftError indicates the position of pointer comparison.
type rightShiftError struct {
Pos token.Position
Message string
}

// Result is returned from the checkStruct function, and holds all rightshift
// operations.
type Result struct {
Errors []rightShiftError
}

func run(dryRun bool, pass *analysis.Pass) (interface{}, error) {
var ret Result
for _, f := range pass.Files {
ast.Inspect(f, func(node ast.Node) bool {
be, ok := node.(*ast.BinaryExpr)
if !ok {
return true
}
// Check if the expression is '1 >> x'.
if be.Op == token.SHR && isOne(be.X) {
err := rightShiftError{
Pos: pass.Fset.Position(be.Pos()),
Message: "found rightshift ('1 >> x') expression, did you mean '1 << x' ?",
}
ret.Errors = append(ret.Errors, err)
if !dryRun {
pass.Report(analysis.Diagnostic{
Pos: pass.Fset.File(f.Pos()).Pos(err.Pos.Offset),
Message: err.Message,
Category: "pointercheck",
})
}
}
return true
},
)
}
return ret, nil
}

// isOne checks if the expression is a constant 1.
func isOne(expr ast.Expr) bool {
bl, ok := expr.(*ast.BasicLit)
return ok && bl.Kind == token.INT && bl.Value == "1"
}

func main() {
singlechecker.Main(Analyzer)
}
36 changes: 36 additions & 0 deletions linter/rightshift/rightshift_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package main

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

"github.com/google/go-cmp/cmp"
"golang.org/x/tools/go/analysis/analysistest"
)

func TestAll(t *testing.T) {
wd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get working directory: %v", err)
}
testdata := filepath.Join(filepath.Dir(wd), "testdata")
res := analysistest.Run(t, testdata, analyzerForTests, "rightshift")
want := []int{6, 11, 12}
got := erroLines(res)
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("analysistest.Ru() unexpected diff in error lines:\n%s\n", diff)
}
}

func erroLines(errs []*analysistest.Result) []int {
var ret []int
for _, e := range errs {
if r, ok := e.Result.(Result); ok {
for _, err := range r.Errors {
ret = append(ret, err.Pos.Line)
}
}
}
return ret
}
4 changes: 0 additions & 4 deletions linter/structinit/structinit.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ import (
// Note: comment should be directly line above the struct definition.
const linterTip = "// lint:require-exhaustive-initialization"

func New(conf any) ([]*analysis.Analyzer, error) {
return []*analysis.Analyzer{Analyzer}, nil
}

// Analyzer implements struct analyzer for structs that are annotated with
// `linterTip`, it checks that every instantiation initializes all the fields.
var Analyzer = &analysis.Analyzer{
Expand Down
2 changes: 1 addition & 1 deletion linter/structinit/structinit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ func testData(t *testing.T) string {
t.Helper()
wd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get wd: %s", err)
t.Fatalf("Failed to get working directory: %v", err)
}
return filepath.Join(filepath.Dir(wd), "testdata")
}
Expand Down
14 changes: 14 additions & 0 deletions linter/testdata/src/rightshift/rightshift.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package rightshift

import "fmt"

func doThing(v int) int {
return 1 >> v // Error: Ln: 6
}

func calc() {
val := 10
fmt.Printf("%v", 1>>val) // Error: Ln 11
_ = doThing(1 >> val) // Error: Ln 12
fmt.Printf("%v", 1<<val) // valid
}

0 comments on commit a4cbf3b

Please sign in to comment.