Merge pull request #2113 from OffchainLabs/linter

Implement linter for checking 1>>x expressions

Makefile

@@ -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 $@

linter/koanf/koanf.go

@@ -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",

linter/koanf/koanf_test.go

@@ -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")
 }

linter/pointercheck/pointer.go

@@ -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",

linter/pointercheck/pointer_test.go

@@ -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")

linter/rightshift/rightshift.go

@@ -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)
+}

linter/rightshift/rightshift_test.go

@@ -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
+}

linter/structinit/structinit.go

@@ -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{

linter/structinit/structinit_test.go

@@ -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")
 }

linter/testdata/src/rightshift/rightshift.go

@@ -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
+}