From 411987a87c2750c3e4b6b9b2bfdd1685a48de59b Mon Sep 17 00:00:00 2001 From: Iskander Sharipov Date: Tue, 30 Jan 2024 13:18:38 +0400 Subject: [PATCH] enable gofmt checks in golangci-lint, run gofmt over all files I noticed that we don't have gofmt checks on our CI. Before golangci-lint was used ~everywhere, people were comparing the `gofmt -s` output manually in their CI scripts. Since we use golangci-lint, we can just add `gofmt` linter and get on with it. --- .golangci.yml | 15 ++- Makefile | 9 +- cmd/cli/main.go | 2 +- pkg/hintrunner/hint_bechmark_test.go | 2 +- pkg/hintrunner/hint_test.go | 119 ++++++++++++------------ pkg/hintrunner/hintcode.go | 5 +- pkg/hintrunner/zerohint.go | 18 ++-- pkg/hintrunner/zerohint_test.go | 22 ++--- pkg/runners/zero/zero_benchmark_test.go | 20 ++-- pkg/vm/vm_test.go | 2 +- 10 files changed, 115 insertions(+), 99 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index e29d39d3c..fbfb7e4a5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,4 +1,17 @@ +linters: + disable-all: true # We'll use an explicit allow-list below + enable: + # First, list all the default ones: + - errcheck + - gosimple + - govet + - ineffassign + - staticcheck + - typecheck + - unused + # Then add something extra (this list may grow over time): + - gofmt run: skip-files: - pkg/assembler/grammar.go - - pkg/hintrunner/hintparser.go \ No newline at end of file + - pkg/hintrunner/hintparser.go diff --git a/Makefile b/Makefile index 623f1f139..3aaeb31fa 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,6 @@ -.PHONY: build clean test help format staticcheck pre-commit +.PHONY: lint build clean test help format staticcheck pre-commit +GOPATH_DIR=`go env GOPATH` BINARY_DIR := bin BINARY_NAME := cairo-vm @@ -54,3 +55,9 @@ testall: bench: @echo "Running benchmarks..." @go run scripts/benchmark.go --pkg=${PKG_NAME} --test=${TEST} + +# Use the same version of the golangci-lint as in our CI linting config. +lint: + go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3 + golangci-lint run ./... + @echo "lint: all good!" diff --git a/cmd/cli/main.go b/cmd/cli/main.go index 63b3a5c69..cc7b98e59 100644 --- a/cmd/cli/main.go +++ b/cmd/cli/main.go @@ -74,7 +74,7 @@ func main() { if err != nil { return fmt.Errorf("cannot load program: %w", err) } - + hints, err := hintrunner.GetZeroHints(cairoZeroJson) if err != nil { return fmt.Errorf("cannot create hints: %w", err) diff --git a/pkg/hintrunner/hint_bechmark_test.go b/pkg/hintrunner/hint_bechmark_test.go index b4994f09c..cb5c1fde9 100644 --- a/pkg/hintrunner/hint_bechmark_test.go +++ b/pkg/hintrunner/hint_bechmark_test.go @@ -205,7 +205,7 @@ func BenchmarkLinearSplit(b *testing.B) { var x ApCellRef = 0 var y ApCellRef = 1 - + b.ResetTimer() for i := 0; i < b.N; i++ { value := Immediate(randomFeltElement(rand)) diff --git a/pkg/hintrunner/hint_test.go b/pkg/hintrunner/hint_test.go index 0dc4c967a..58cde8bf2 100644 --- a/pkg/hintrunner/hint_test.go +++ b/pkg/hintrunner/hint_test.go @@ -269,59 +269,58 @@ func TestWideMul128(t *testing.T) { } func TestDivMod(t *testing.T) { - vm := defaultVirtualMachine() - vm.Context.Ap = 0 - vm.Context.Fp = 0 + vm := defaultVirtualMachine() + vm.Context.Ap = 0 + vm.Context.Fp = 0 - var quo ApCellRef = 1 - var rem ApCellRef = 2 + var quo ApCellRef = 1 + var rem ApCellRef = 2 - lhsValue := Immediate(f.NewElement(89)) - rhsValue := Immediate(f.NewElement(7)) + lhsValue := Immediate(f.NewElement(89)) + rhsValue := Immediate(f.NewElement(7)) - hint := DivMod{ - lhs: lhsValue, - rhs: rhsValue, - quotient: quo, - remainder: rem, - } + hint := DivMod{ + lhs: lhsValue, + rhs: rhsValue, + quotient: quo, + remainder: rem, + } - err := hint.Execute(vm, nil) - require.Nil(t, err) + err := hint.Execute(vm, nil) + require.Nil(t, err) - expectedQuotient := mem.MemoryValueFromInt(12) - expectedRemainder := mem.MemoryValueFromInt(5) + expectedQuotient := mem.MemoryValueFromInt(12) + expectedRemainder := mem.MemoryValueFromInt(5) - actualQuotient := readFrom(vm, VM.ExecutionSegment, 1) - actualRemainder := readFrom(vm, VM.ExecutionSegment, 2) + actualQuotient := readFrom(vm, VM.ExecutionSegment, 1) + actualRemainder := readFrom(vm, VM.ExecutionSegment, 2) - require.Equal(t, expectedQuotient, actualQuotient) - require.Equal(t, expectedRemainder, actualRemainder) + require.Equal(t, expectedQuotient, actualQuotient) + require.Equal(t, expectedRemainder, actualRemainder) } -func TestDivModDivisionByZeroError (t *testing.T) { +func TestDivModDivisionByZeroError(t *testing.T) { vm := defaultVirtualMachine() - vm.Context.Ap = 0 - vm.Context.Fp = 0 + vm.Context.Ap = 0 + vm.Context.Fp = 0 - var quo ApCellRef = 1 - var rem ApCellRef = 2 + var quo ApCellRef = 1 + var rem ApCellRef = 2 - lhsValue := Immediate(f.NewElement(43)) - rhsValue := Immediate(f.NewElement(0)) + lhsValue := Immediate(f.NewElement(43)) + rhsValue := Immediate(f.NewElement(0)) - hint := DivMod{ - lhs: lhsValue, - rhs: rhsValue, - quotient: quo, - remainder: rem, - } + hint := DivMod{ + lhs: lhsValue, + rhs: rhsValue, + quotient: quo, + remainder: rem, + } - err := hint.Execute(vm, nil) - require.ErrorContains(t, err, "cannot be divided by zero, rhs: 0") + err := hint.Execute(vm, nil) + require.ErrorContains(t, err, "cannot be divided by zero, rhs: 0") } - func TestUint256DivMod(t *testing.T) { t.Run("test uint256DivMod", func(t *testing.T) { vm := defaultVirtualMachine() @@ -335,10 +334,10 @@ func TestUint256DivMod(t *testing.T) { dividend0Felt := f.NewElement(89) dividend1Felt := f.NewElement(72) - + divisor0Felt := f.NewElement(3) divisor1Felt := f.NewElement(7) - + hint := Uint256DivMod{ dividend0: Immediate(dividend0Felt), dividend1: Immediate(dividend1Felt), @@ -349,44 +348,44 @@ func TestUint256DivMod(t *testing.T) { remainder0: remainder0, remainder1: remainder1, } - + err := hint.Execute(vm, nil) require.Nil(t, err) - + quotient0Val := &f.Element{} _, err = quotient0Val.SetString("10") require.Nil(t, err) - + require.Equal( t, mem.MemoryValueFromFieldElement(quotient0Val), readFrom(vm, VM.ExecutionSegment, 1), ) - + quotient1Val := &f.Element{} quotient1Val.SetZero() require.Nil(t, err) - + require.Equal( t, mem.MemoryValueFromFieldElement(quotient1Val), readFrom(vm, VM.ExecutionSegment, 2), ) - + remainder0Val := &f.Element{} _, err = remainder0Val.SetString("59") require.Nil(t, err) - + require.Equal( t, mem.MemoryValueFromFieldElement(remainder0Val), readFrom(vm, VM.ExecutionSegment, 3), ) - + remainder1Val := &f.Element{} _, err = remainder1Val.SetString("2") require.Nil(t, err) - + require.Equal( t, mem.MemoryValueFromFieldElement(remainder1Val), @@ -411,7 +410,7 @@ func TestUint256DivMod(t *testing.T) { divisor0Felt := f.NewElement(1<<8 + 1) divisor1Felt := f.NewElement(1<<8 + 1) - + hint := Uint256DivMod{ dividend0: Immediate(dividend0Felt), dividend1: Immediate(dividend1Felt), @@ -422,51 +421,51 @@ func TestUint256DivMod(t *testing.T) { remainder0: remainder0, remainder1: remainder1, } - + err = hint.Execute(vm, nil) require.Nil(t, err) - + quotient0Val := &f.Element{} quotient0Val.SetOne() require.Nil(t, err) - + require.Equal( t, mem.MemoryValueFromFieldElement(quotient0Val), readFrom(vm, VM.ExecutionSegment, 1), ) - + quotient1Val := &f.Element{} quotient1Val.SetZero() require.Nil(t, err) - + require.Equal( t, mem.MemoryValueFromFieldElement(quotient1Val), readFrom(vm, VM.ExecutionSegment, 2), ) - + remainder0Val := &f.Element{} _, err = remainder0Val.SetString("170141183460469231731687303715884105471") require.Nil(t, err) - + require.Equal( t, mem.MemoryValueFromFieldElement(remainder0Val), readFrom(vm, VM.ExecutionSegment, 3), ) - + remainder1Val := &f.Element{} remainder1Val.SetZero() require.Nil(t, err) - + require.Equal( t, mem.MemoryValueFromFieldElement(remainder1Val), readFrom(vm, VM.ExecutionSegment, 4), ) }) -} +} func TestUint256DivModDivisionByZero(t *testing.T) { vm := defaultVirtualMachine() @@ -478,7 +477,6 @@ func TestUint256DivModDivisionByZero(t *testing.T) { var dstRemainder0 ApCellRef = 3 var dstRemainder1 ApCellRef = 4 - dividend0Felt := f.NewElement(1<<8 + 1) dividend1Felt := f.NewElement(1<<8 + 1) @@ -1136,4 +1134,3 @@ func TestFieldSqrt(t *testing.T) { }) } } - diff --git a/pkg/hintrunner/hintcode.go b/pkg/hintrunner/hintcode.go index b480b3ba9..839b511ee 100644 --- a/pkg/hintrunner/hintcode.go +++ b/pkg/hintrunner/hintcode.go @@ -1,6 +1,5 @@ package hintrunner - -const( +const ( AllocSegmentCode string = "memory[ap] = segments.add()" -) \ No newline at end of file +) diff --git a/pkg/hintrunner/zerohint.go b/pkg/hintrunner/zerohint.go index cedcf175d..cf8998825 100644 --- a/pkg/hintrunner/zerohint.go +++ b/pkg/hintrunner/zerohint.go @@ -4,9 +4,9 @@ import ( "fmt" "strconv" - "github.com/alecthomas/participle/v2" sn "github.com/NethermindEth/cairo-vm-go/pkg/parsers/starknet" zero "github.com/NethermindEth/cairo-vm-go/pkg/parsers/zero" + "github.com/alecthomas/participle/v2" ) var parser *participle.Parser[IdentifierExp] = participle.MustBuild[IdentifierExp](participle.UseLookahead(10)) @@ -36,7 +36,7 @@ func GetZeroHints(cairoZeroJson *zero.ZeroProgram) (map[uint64]Hinter, error) { return hints, nil } -func GetHintFromCode(program *zero.ZeroProgram, rawHint zero.Hint, hintPC uint64) (Hinter, error){ +func GetHintFromCode(program *zero.ZeroProgram, rawHint zero.Hint, hintPC uint64) (Hinter, error) { cellRefParams, resOpParams, err := GetParameters(program, rawHint, hintPC) if err != nil { return nil, err @@ -51,10 +51,10 @@ func GetHintFromCode(program *zero.ZeroProgram, rawHint zero.Hint, hintPC uint64 } func CreateAllocSegmentHinter(cellRefParams []CellRefer, resOpParams []ResOperander) (Hinter, error) { - if len(cellRefParams) + len(resOpParams) != 0 { + if len(cellRefParams)+len(resOpParams) != 0 { return nil, fmt.Errorf("Expected no arguments for %s hint", sn.AllocSegmentName) } - return &AllocSegment { dst: ApCellRef(0) }, nil + return &AllocSegment{dst: ApCellRef(0)}, nil } func GetParameters(zeroProgram *zero.ZeroProgram, hint zero.Hint, hintPC uint64) ([]CellRefer, []ResOperander, error) { @@ -83,12 +83,12 @@ func GetParameters(zeroProgram *zero.ZeroProgram, hint zero.Hint, hintPC uint64) var reference zero.Reference ok = false for i := len(references) - 1; i >= 0; i-- { - if references[i].Pc <= hintPC{ + if references[i].Pc <= hintPC { reference = references[i] ok = true break - } - } + } + } if !ok { return nil, nil, fmt.Errorf("identifier %s should have a reference with pc smaller or equal than %d", referenceName, hintPC) } @@ -97,7 +97,7 @@ func GetParameters(zeroProgram *zero.ZeroProgram, hint zero.Hint, hintPC uint64) if err != nil { return nil, nil, err } - switch result := param.(type){ + switch result := param.(type) { case CellRefer: cellRefParams = append(cellRefParams, result) case ResOperander: @@ -117,4 +117,4 @@ func ParseIdentifier(value string) (any, error) { } return identifierExp.Evaluate() -} \ No newline at end of file +} diff --git a/pkg/hintrunner/zerohint_test.go b/pkg/hintrunner/zerohint_test.go index f169d7e5a..7a8039421 100644 --- a/pkg/hintrunner/zerohint_test.go +++ b/pkg/hintrunner/zerohint_test.go @@ -20,27 +20,27 @@ func TestHintParser(t *testing.T) { ExpectedResOperander: nil, }, { - Parameter: "[cast(ap + (-1) + 2, starkware.cairo.common.cairo_builtins.BitwiseBuiltin**)]", - ExpectedCellRefer: nil, + Parameter: "[cast(ap + (-1) + 2, starkware.cairo.common.cairo_builtins.BitwiseBuiltin**)]", + ExpectedCellRefer: nil, ExpectedResOperander: Deref{ deref: ApCellRef(1), }, }, { - Parameter: "[cast([ap + 2], felt)]", - ExpectedCellRefer: nil, + Parameter: "[cast([ap + 2], felt)]", + ExpectedCellRefer: nil, ExpectedResOperander: DoubleDeref{ - deref: ApCellRef(2), + deref: ApCellRef(2), offset: 0, }, }, { - Parameter: "cast([ap + 2] + [ap], felt)", - ExpectedCellRefer: nil, - ExpectedResOperander: BinaryOp { + Parameter: "cast([ap + 2] + [ap], felt)", + ExpectedCellRefer: nil, + ExpectedResOperander: BinaryOp{ operator: Add, - lhs: ApCellRef(2), - rhs: Deref { + lhs: ApCellRef(2), + rhs: Deref{ deref: ApCellRef(0), }, }, @@ -59,4 +59,4 @@ func TestHintParser(t *testing.T) { require.Equal(t, test.ExpectedResOperander, output, "Expected ResOperander type") } } -} \ No newline at end of file +} diff --git a/pkg/runners/zero/zero_benchmark_test.go b/pkg/runners/zero/zero_benchmark_test.go index f853a41ed..adeab4b9e 100644 --- a/pkg/runners/zero/zero_benchmark_test.go +++ b/pkg/runners/zero/zero_benchmark_test.go @@ -5,7 +5,7 @@ import ( "testing" hintrunner "github.com/NethermindEth/cairo-vm-go/pkg/hintrunner" - zero "github.com/NethermindEth/cairo-vm-go/pkg/parsers/zero" + zero "github.com/NethermindEth/cairo-vm-go/pkg/parsers/zero" ) func BenchmarkRunnerWithFibonacci(b *testing.B) { @@ -216,22 +216,22 @@ func BenchmarkRunnerWithFibonacci(b *testing.B) { } `) - b.ResetTimer() + b.ResetTimer() for i := 0; i < b.N; i++ { - cairoZeroJson, err := zero.ZeroProgramFromJSON(compiledJson) - if err != nil { - panic(err) - } + cairoZeroJson, err := zero.ZeroProgramFromJSON(compiledJson) + if err != nil { + panic(err) + } program, err := LoadCairoZeroProgram(cairoZeroJson) if err != nil { panic(err) } - hints, err := hintrunner.GetZeroHints(cairoZeroJson) - if err != nil { - panic(err) - } + hints, err := hintrunner.GetZeroHints(cairoZeroJson) + if err != nil { + panic(err) + } runner, err := NewRunner(program, hints, true, math.MaxUint64) if err != nil { diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index 9a1d796be..332d8c921 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -1004,7 +1004,7 @@ func TestRunStepInstructions(t *testing.T) { assert.Equal(t, vm.Context.Pc.Offset, uint64(15)) }) - + t.Run("test abs jump with address", func(t *testing.T) { vm := defaultVirtualMachineWithCode("jmp abs [ap];") setInitialReg(vm, 1, 1, 0)