From 6d16c057d6d493648b99781e9c1a4e56b4b7ca71 Mon Sep 17 00:00:00 2001 From: Steve Coffman Date: Thu, 16 May 2024 17:06:38 -0400 Subject: [PATCH] Redo github actions (#295) * Redo github actions Signed-off-by: Steve Coffman * Update golangci-lint to match gqlgen Signed-off-by: Steve Coffman * Linter fixes Signed-off-by: Steve Coffman --------- Signed-off-by: Steve Coffman --- .github/workflows/lint.yml | 51 +++++-------- .github/workflows/manual-linter.yml | 25 ------- .github/workflows/pull-request.yml | 56 -------------- .../{manual-go-test.yml => test.yml} | 29 +++----- .golangci.yaml | 73 +++++++++++-------- ast/argmap_test.go | 8 +- ast/decode.go | 2 +- ast/document_test.go | 2 +- ast/path.go | 6 +- ast/path_test.go | 4 +- formatter/formatter_test.go | 1 - gqlerror/error_test.go | 2 +- lexer/lexer.go | 16 ++-- lexer/lexer_test.go | 1 - parser/parser_test.go | 20 ++--- parser/query_test.go | 2 +- parser/schema_test.go | 6 +- parser/testrunner/runner.go | 11 ++- validator/imported_test.go | 4 +- validator/rules/fields_on_correct_type.go | 16 +++- validator/rules/known_directives.go | 2 +- validator/rules/no_unused_fragments.go | 1 - .../rules/overlapping_fields_can_be_merged.go | 3 - validator/rules/possible_fragment_spreads.go | 1 - validator/rules/values_of_correct_type.go | 2 +- validator/schema.go | 1 - validator/schema_test.go | 20 ++--- validator/validator_test.go | 16 ++-- validator/vars.go | 3 +- validator/vars_test.go | 45 ++++++------ validator/walk.go | 8 ++ validator/walk_test.go | 8 +- 32 files changed, 183 insertions(+), 262 deletions(-) delete mode 100644 .github/workflows/manual-linter.yml delete mode 100644 .github/workflows/pull-request.yml rename .github/workflows/{manual-go-test.yml => test.yml} (64%) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index d5265b68..6e3d894a 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,46 +1,33 @@ name: golangci-lint on: + workflow_dispatch: push: branches: - - main + - master pull_request: - branches: - - main + types: [ opened, synchronize ] +# When a new revision is pushed to a PR, cancel all in-progress CI runs for that +# PR. See https://docs.github.com/en/actions/using-jobs/using-concurrency +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + env: GOFLAGS: "-trimpath" - GO_VERSION: 1.19 jobs: - resolve-modules: - name: Resolve Modules - # runs-on: [self-hosted, linux, x64] - runs-on: ubuntu-latest - outputs: - matrix: ${{ steps.set-matrix.outputs.matrix }} - steps: - - name: Checkout Sources - uses: actions/checkout@v2 - - id: set-matrix - run: ./tools/resolve-modules.sh - golangci: - name: Linter - needs: resolve-modules - runs-on: ubuntu-latest + golangci-lint: strategy: - matrix: ${{ fromJson(needs.resolve-modules.outputs.matrix) }} + matrix: + go: ["1.21", "1.22"] + runs-on: ubuntu-latest steps: - - name: Install Go - uses: actions/setup-go@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 with: - # stable: 'false' # Keep this line to be able to use rc and beta version of Go (ex: 1.18.0-rc1). - go-version: ${{ env.GO_VERSION }} - - uses: actions/checkout@v2 - - name: lint - uses: golangci/golangci-lint-action@v3.2.0 + go-version: ${{ matrix.go }} + - name: golangci-lint + uses: golangci/golangci-lint-action@v6.0.1 with: version: latest - # skip cache because of flaky behaviors - skip-build-cache: true - skip-pkg-cache: true - working-directory: ${{ matrix.workdir }} - args: --timeout 5m --issues-exit-code=0 + args: '--timeout 5m' # only-new-issues: true #show only new issues if it's a pull request. options working-directory and only-new-issues are incompatible \ No newline at end of file diff --git a/.github/workflows/manual-linter.yml b/.github/workflows/manual-linter.yml deleted file mode 100644 index ddb2d879..00000000 --- a/.github/workflows/manual-linter.yml +++ /dev/null @@ -1,25 +0,0 @@ -name: Manual Linter Run - -on: - workflow_dispatch: - -# When a new revision is pushed to a PR, cancel all in-progress CI runs for that -# PR. See https://docs.github.com/en/actions/using-jobs/using-concurrency -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -jobs: - go-lint: - name: Go Lint - runs-on: ubuntu-latest - steps: - - name: Check out code - uses: actions/checkout@v3 - - - name: Golang Style and Lint Check - uses: golangci/golangci-lint-action@v3 - timeout-minutes: 30 - with: - # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version - version: latest diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml deleted file mode 100644 index aecbed97..00000000 --- a/.github/workflows/pull-request.yml +++ /dev/null @@ -1,56 +0,0 @@ -name: PR Check - -on: [pull_request] - -# When a new revision is pushed to a PR, cancel all in-progress CI runs for that -# PR. See https://docs.github.com/en/actions/using-jobs/using-concurrency -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -jobs: - go-lint: - name: Go Lint - runs-on: ubuntu-latest - steps: - - name: Check out code - uses: actions/checkout@v3 - - - name: Golang Style and Lint Check - uses: golangci/golangci-lint-action@v3 - timeout-minutes: 30 - with: - # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version - version: latest - - go-test: - name: Go Test (${{ matrix.os }}) - runs-on: ${{ matrix.run }} - strategy: - fail-fast: false - matrix: - include: - - os: linux - run: ubuntu-latest - # MacOS is disabled due to the high cost multiplier on GH Actions. - #- os: darwin - # run: macos-latest - # Windows not allowed currently because of line-ending conversion issues. - #- os: windows - # run: windows-latest - steps: - - name: Check out code - uses: actions/checkout@v3 - - - id: go_version - name: Read go version - run: echo "::set-output name=go_version::$(cat .go-version)" - - - name: Install Go (${{ steps.go_version.outputs.go_version }}) - uses: actions/setup-go@v3 - with: - go-version: ${{ steps.go_version.outputs.go_version }} - - - name: Unit Test Golang - run: go test ./... - timeout-minutes: 30 diff --git a/.github/workflows/manual-go-test.yml b/.github/workflows/test.yml similarity index 64% rename from .github/workflows/manual-go-test.yml rename to .github/workflows/test.yml index fa7cd3bb..aa2231d4 100644 --- a/.github/workflows/manual-go-test.yml +++ b/.github/workflows/test.yml @@ -1,8 +1,11 @@ -name: Manual Go Test Run - +name: Test on: workflow_dispatch: - + push: + branches: + - master + pull_request: + types: [ opened, synchronize ] # When a new revision is pushed to a PR, cancel all in-progress CI runs for that # PR. See https://docs.github.com/en/actions/using-jobs/using-concurrency concurrency: @@ -16,28 +19,20 @@ jobs: strategy: fail-fast: false matrix: - include: - - os: linux - run: ubuntu-latest + os: [ubuntu-latest] + go: ["1.21", "1.22"] # MacOS is disabled due to the high cost multiplier on GH Actions. #- os: darwin # run: macos-latest # Windows not allowed currently because of line-ending conversion issues. #- os: windows # run: windows-latest + continue-on-error: true steps: - - name: Check out code - uses: actions/checkout@v3 - - - id: go_version - name: Read go version - run: echo "::set-output name=go_version::$(cat .go-version)" - - - name: Install Go (${{ steps.go_version.outputs.go_version }}) - uses: actions/setup-go@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 with: - go-version: ${{ steps.go_version.outputs.go_version }} - + go-version: ${{ matrix.go }} - name: Unit Test Golang run: go test ./... timeout-minutes: 30 diff --git a/.golangci.yaml b/.golangci.yaml index accb83fa..97a514b9 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,45 +1,54 @@ run: - timeout: 10m + tests: true + +linters-settings: + errcheck: + exclude-functions: + - (io.Writer).Write + - io.Copy + - io.WriteString + revive: + enable-all-rules: false + rules: + - name: empty-lines + testifylint: + disable-all: true + enable: + - bool-compare + - compares + - error-is-as + - error-nil + - expected-actual + - nil-compare + linters: disable-all: true enable: + - bodyclose + - dupl + - errcheck + - gocritic + - gofmt + - goimports + - gosimple + - govet - ineffassign + - misspell + - nakedret + - prealloc + - revive + - staticcheck + - testifylint - typecheck - - varcheck + - unconvert - unused - - structcheck - - deadcode - - gosimple - - goimports - - errcheck - - staticcheck - - stylecheck - - gosec - - asciicheck - - bodyclose - - exportloopref - - rowserrcheck - - makezero - - durationcheck - - prealloc - - predeclared -linters-settings: - staticcheck: - checks: ["S1002","S1004","S1007","S1009","S1010","S1012","S1019","S1020","S1021","S1024","S1030","SA2*","SA3*","SA4009","SA5*","SA6000","SA6001","SA6005", "-SA2002"] - stylecheck: - checks: ["-ST1003"] - gosec: - severity: "low" - confidence: "low" - excludes: - - G101 - - G112 issues: + exclude-dirs: + - bin exclude-rules: + # Exclude some linters from running on tests files. - path: _test\.go linters: + - dupl - errcheck - - gosec - - rowserrcheck - - makezero diff --git a/ast/argmap_test.go b/ast/argmap_test.go index aa0292f0..9ac50a6b 100644 --- a/ast/argmap_test.go +++ b/ast/argmap_test.go @@ -32,8 +32,8 @@ func TestArg2Map(t *testing.T) { {Name: "A", Value: &Value{Kind: NullValue}}, {Name: "B", Value: &Value{Kind: NullValue}}, }, nil) - require.Equal(t, nil, args["A"]) - require.Equal(t, nil, args["B"]) + require.Nil(t, args["A"]) + require.Nil(t, args["B"]) require.Contains(t, args, "A") require.Contains(t, args, "B") }) @@ -55,8 +55,8 @@ func TestArg2Map(t *testing.T) { "VarA": nil, "VarB": nil, }) - require.Equal(t, nil, args["A"]) - require.Equal(t, nil, args["B"]) + require.Nil(t, args["A"]) + require.Nil(t, args["B"]) require.Contains(t, args, "A") require.Contains(t, args, "B") }) diff --git a/ast/decode.go b/ast/decode.go index d0092055..c9966b24 100644 --- a/ast/decode.go +++ b/ast/decode.go @@ -11,7 +11,7 @@ func UnmarshalSelectionSet(b []byte) (SelectionSet, error) { return nil, err } - var result = make([]Selection, 0) + result := make([]Selection, 0) for _, item := range tmp { var field Field if err := json.Unmarshal(item, &field); err == nil { diff --git a/ast/document_test.go b/ast/document_test.go index b6461594..1f6ea721 100644 --- a/ast/document_test.go +++ b/ast/document_test.go @@ -18,7 +18,7 @@ func TestQueryDocMethods(t *testing.T) { } `}) - require.Nil(t, err) + require.NoError(t, err) t.Run("GetOperation", func(t *testing.T) { require.EqualValues(t, "Bob", doc.Operations.ForName("Bob").Name) require.Nil(t, doc.Operations.ForName("Alice")) diff --git a/ast/path.go b/ast/path.go index 4f5c6748..f40aa953 100644 --- a/ast/path.go +++ b/ast/path.go @@ -14,8 +14,10 @@ type PathElement interface { isPathElement() } -var _ PathElement = PathIndex(0) -var _ PathElement = PathName("") +var ( + _ PathElement = PathIndex(0) + _ PathElement = PathName("") +) func (path Path) String() string { if path == nil { diff --git a/ast/path_test.go b/ast/path_test.go index e077563e..73fc210d 100644 --- a/ast/path_test.go +++ b/ast/path_test.go @@ -57,7 +57,7 @@ func TestPath_MarshalJSON(t *testing.T) { for _, spec := range specs { t.Run(spec.Value.String(), func(t *testing.T) { b, err := json.Marshal(spec.Value) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, spec.Expected, string(b)) }) @@ -88,7 +88,7 @@ func TestPath_UnmarshalJSON(t *testing.T) { t.Run(spec.Value, func(t *testing.T) { var path Path err := json.Unmarshal([]byte(spec.Value), &path) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, spec.Expected, path) }) diff --git a/formatter/formatter_test.go b/formatter/formatter_test.go index b4573b98..5181c6f9 100644 --- a/formatter/formatter_test.go +++ b/formatter/formatter_test.go @@ -215,7 +215,6 @@ func executeGoldenTesting(t *testing.T, cfg *goldenConfig) { t.Fatal(err) } return - } else if err != nil { t.Fatal(err) } diff --git a/gqlerror/error_test.go b/gqlerror/error_test.go index 69db4c45..ee975721 100644 --- a/gqlerror/error_test.go +++ b/gqlerror/error_test.go @@ -35,7 +35,7 @@ func TestErrorFormatting(t *testing.T) { err := ErrorLocf("", 66, 2, "kabloom") require.Equal(t, `input:66: kabloom`, err.Error()) - require.Equal(t, nil, err.Extensions["file"]) + require.Nil(t, err.Extensions["file"]) }) t.Run("with filename", func(t *testing.T) { diff --git a/lexer/lexer.go b/lexer/lexer.go index 3fd6b4c0..21dd3cf4 100644 --- a/lexer/lexer.go +++ b/lexer/lexer.go @@ -254,7 +254,6 @@ func (s *Lexer) readNumber() (Token, error) { return s.makeToken(Float) } return s.makeToken(Int) - } // acceptByte if it matches any of given bytes, returning true if it found anything @@ -317,8 +316,8 @@ func (s *Lexer) readString() (Token, error) { } switch r { default: - var char = rune(r) - var w = 1 + char := rune(r) + w := 1 // skip unicode overhead if we are in the ascii range if r >= 127 { @@ -440,11 +439,12 @@ func (s *Lexer) readBlockString() (Token, error) { return s.makeError(`Invalid character within String: "\u%04d".`, r) } - if r == '\\' && s.end+4 <= inputLen && s.Input[s.end:s.end+4] == `\"""` { + switch { + case r == '\\' && s.end+4 <= inputLen && s.Input[s.end:s.end+4] == `\"""`: buf.WriteString(`"""`) s.end += 4 s.endRunes += 4 - } else if r == '\r' { + case r == '\r': if s.end+1 < inputLen && s.Input[s.end+1] == '\n' { s.end++ s.endRunes++ @@ -455,9 +455,9 @@ func (s *Lexer) readBlockString() (Token, error) { s.endRunes++ s.line++ s.lineStartRunes = s.endRunes - } else { - var char = rune(r) - var w = 1 + default: + char := rune(r) + w := 1 // skip unicode overhead if we are in the ascii range if r >= 127 { diff --git a/lexer/lexer_test.go b/lexer/lexer_test.go index 8ae5b928..00c75915 100644 --- a/lexer/lexer_test.go +++ b/lexer/lexer_test.go @@ -16,7 +16,6 @@ func TestLexer(t *testing.T) { ret := testrunner.Spec{} for { tok, err := l.ReadToken() - if err != nil { ret.Error = err.(*gqlerror.Error) break diff --git a/parser/parser_test.go b/parser/parser_test.go index 16eb0308..6b677a96 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -16,20 +16,20 @@ func TestParserUtils(t *testing.T) { tok, _ := p.expectKeyword("asdf") require.Equal(t, "asdf", tok.Value) require.Equal(t, "asdf", p.prev.Value) - require.Nil(t, p.err) + require.NoError(t, p.err) require.Equal(t, "1.0", p.peek().Value) require.Equal(t, "1.0", p.peek().Value) tok, _ = p.expect(lexer.Float) require.Equal(t, "1.0", tok.Value) require.Equal(t, "1.0", p.prev.Value) - require.Nil(t, p.err) + require.NoError(t, p.err) require.True(t, p.skip(lexer.Name)) - require.Nil(t, p.err) + require.NoError(t, p.err) require.Equal(t, lexer.EOF, p.peek().Kind) - require.Nil(t, p.err) + require.NoError(t, p.err) }) t.Run("test many", func(t *testing.T) { @@ -40,11 +40,11 @@ func TestParserUtils(t *testing.T) { p.many(lexer.BracketL, lexer.BracketR, func() { arr = append(arr, p.next().Value) }) - require.Nil(t, p.err) + require.NoError(t, p.err) require.Equal(t, []string{"a", "b", "c", "d"}, arr) require.Equal(t, lexer.EOF, p.peek().Kind) - require.Nil(t, p.err) + require.NoError(t, p.err) }) t.Run("return if open is not found", func(t *testing.T) { @@ -53,7 +53,7 @@ func TestParserUtils(t *testing.T) { p.many(lexer.BracketL, lexer.BracketR, func() { t.Error("cb should not be called") }) - require.Nil(t, p.err) + require.NoError(t, p.err) require.Equal(t, "turtles", p.next().Value) }) @@ -80,11 +80,11 @@ func TestParserUtils(t *testing.T) { p.some(lexer.BracketL, lexer.BracketR, func() { arr = append(arr, p.next().Value) }) - require.Nil(t, p.err) + require.NoError(t, p.err) require.Equal(t, []string{"a", "b", "c", "d"}, arr) require.Equal(t, lexer.EOF, p.peek().Kind) - require.Nil(t, p.err) + require.NoError(t, p.err) }) t.Run("can't read empty array", func(t *testing.T) { @@ -105,7 +105,7 @@ func TestParserUtils(t *testing.T) { p.some(lexer.BracketL, lexer.BracketR, func() { t.Error("cb should not be called") }) - require.Nil(t, p.err) + require.NoError(t, p.err) require.Equal(t, "turtles", p.next().Value) }) diff --git a/parser/query_test.go b/parser/query_test.go index f1772617..9170f4ef 100644 --- a/parser/query_test.go +++ b/parser/query_test.go @@ -39,7 +39,7 @@ query SomeOperation { } `, }) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, 3, query.Operations.ForName("SomeOperation").Position.Line) assert.Equal(t, 5, query.Operations.ForName("SomeOperation").SelectionSet[0].GetPosition().Line) }) diff --git a/parser/schema_test.go b/parser/schema_test.go index e9df4139..16b35058 100644 --- a/parser/schema_test.go +++ b/parser/schema_test.go @@ -33,7 +33,7 @@ func TestTypePosition(t *testing.T) { } `, }) - assert.Nil(t, parseErr) + assert.NoError(t, parseErr) assert.Equal(t, 2, schema.Definitions.ForName("query").Fields.ForName("me").Type.Position.Line) }) t.Run("type line number with bang", func(t *testing.T) { @@ -43,7 +43,7 @@ func TestTypePosition(t *testing.T) { } `, }) - assert.Nil(t, parseErr) + assert.NoError(t, parseErr) assert.Equal(t, 2, schema.Definitions.ForName("query").Fields.ForName("me").Type.Position.Line) }) t.Run("type line number with comments", func(t *testing.T) { @@ -54,7 +54,7 @@ func TestTypePosition(t *testing.T) { } `, }) - assert.Nil(t, parseErr) + assert.NoError(t, parseErr) assert.Equal(t, 3, schema.Definitions.ForName("query").Fields.ForName("me").Type.Position.Line) }) } diff --git a/parser/testrunner/runner.go b/parser/testrunner/runner.go index 528d2567..24ec5994 100644 --- a/parser/testrunner/runner.go +++ b/parser/testrunner/runner.go @@ -52,14 +52,14 @@ func Test(t *testing.T, filename string, f func(t *testing.T, input string) Spec for _, spec := range specs { t.Run(spec.Name, func(t *testing.T) { result := f(t, spec.Input) - - if spec.Error == nil { + switch { + case spec.Error == nil: if result.Error != nil { t.Errorf("unexpected error %s", result.Error.Message) } - } else if result.Error == nil { + case result.Error == nil: t.Errorf("expected error but got none") - } else { + default: if result.Error.Message != spec.Error.Message { t.Errorf("wrong error returned\nexpected: %s\ngot: %s", spec.Error.Message, result.Error.Message) } @@ -85,7 +85,7 @@ func Test(t *testing.T, filename string, f func(t *testing.T, input string) Spec for i, tok := range result.Tokens { expected := spec.Tokens[i] - if !strings.EqualFold(strings.Replace(expected.Kind, "_", "", -1), tok.Kind) { + if !strings.EqualFold(strings.ReplaceAll(expected.Kind, "_", ""), tok.Kind) { t.Errorf("token[%d].kind should be %s, was %s", i, expected.Kind, tok.Kind) } if expected.Value != "undefined" && expected.Value != tok.Value { @@ -134,5 +134,4 @@ func Test(t *testing.T, filename string, f func(t *testing.T, input string) Spec } }) } - } diff --git a/validator/imported_test.go b/validator/imported_test.go index 5cafdaea..41a3eedf 100644 --- a/validator/imported_test.go +++ b/validator/imported_test.go @@ -43,7 +43,7 @@ func TestValidation(t *testing.T) { d.pattern = regexp.MustCompile("^" + d.Rule + "$") } - var schemas = make([]*ast.Schema, 0, len(rawSchemas)) + schemas := make([]*ast.Schema, 0, len(rawSchemas)) for i, schema := range rawSchemas { schema, err := gqlparser.LoadSchema(&ast.Source{Input: schema, Name: fmt.Sprintf("schemas.yml[%d]", i)}) if err != nil { @@ -110,7 +110,7 @@ func runSpec(t *testing.T, schemas []*ast.Schema, deviations []*Deviation, filen spec.Errors[i].Rule = spec.Rule // remove inconsistent use of ; - spec.Errors[i].Message = strings.Replace(spec.Errors[i].Message, "; Did you mean", ". Did you mean", -1) + spec.Errors[i].Message = strings.ReplaceAll(spec.Errors[i].Message, "; Did you mean", ". Did you mean") } sort.Slice(spec.Errors, compareErrors(spec.Errors)) sort.Slice(finalErrors, compareErrors(finalErrors)) diff --git a/validator/rules/fields_on_correct_type.go b/validator/rules/fields_on_correct_type.go index 77c3fdac..24d4f3db 100644 --- a/validator/rules/fields_on_correct_type.go +++ b/validator/rules/fields_on_correct_type.go @@ -44,7 +44,7 @@ func getSuggestedTypeNames(walker *Walker, parent *ast.Definition, name string) } possibleTypes := walker.Schema.GetPossibleTypes(parent) - var suggestedObjectTypes = make([]string, 0, len(possibleTypes)) + suggestedObjectTypes := make([]string, 0, len(possibleTypes)) var suggestedInterfaceTypes []string interfaceUsageCount := map[string]int{} @@ -67,7 +67,7 @@ func getSuggestedTypeNames(walker *Walker, parent *ast.Definition, name string) } } - suggestedTypes := append(suggestedInterfaceTypes, suggestedObjectTypes...) + suggestedTypes := concatSlice(suggestedInterfaceTypes, suggestedObjectTypes) sort.SliceStable(suggestedTypes, func(i, j int) bool { typeA, typeB := suggestedTypes[i], suggestedTypes[j] @@ -81,6 +81,16 @@ func getSuggestedTypeNames(walker *Walker, parent *ast.Definition, name string) return suggestedTypes } +// By employing a full slice expression (slice[low:high:max]), +// where max is set to the slice’s length, +// we ensure that appending elements results +// in a slice backed by a distinct array. +// This method prevents the shared array issue +func concatSlice(first []string, second []string) []string { + n := len(first) + return append(first[:n:n], second...) +} + // For the field name provided, determine if there are any similar field names // that may be the result of a typo. func getSuggestedFieldNames(parent *ast.Definition, name string) []string { @@ -88,7 +98,7 @@ func getSuggestedFieldNames(parent *ast.Definition, name string) []string { return nil } - var possibleFieldNames = make([]string, 0, len(parent.Fields)) + possibleFieldNames := make([]string, 0, len(parent.Fields)) for _, field := range parent.Fields { possibleFieldNames = append(possibleFieldNames, field.Name) } diff --git a/validator/rules/known_directives.go b/validator/rules/known_directives.go index 9b5f28db..f7bae811 100644 --- a/validator/rules/known_directives.go +++ b/validator/rules/known_directives.go @@ -14,7 +14,7 @@ func init() { Line int Column int } - var seen = map[mayNotBeUsedDirective]bool{} + seen := map[mayNotBeUsedDirective]bool{} observers.OnDirective(func(walker *Walker, directive *ast.Directive) { if directive.Definition == nil { addError( diff --git a/validator/rules/no_unused_fragments.go b/validator/rules/no_unused_fragments.go index 218ffa77..59d9c15c 100644 --- a/validator/rules/no_unused_fragments.go +++ b/validator/rules/no_unused_fragments.go @@ -9,7 +9,6 @@ import ( func init() { AddRule("NoUnusedFragments", func(observers *Events, addError AddErrFunc) { - inFragmentDefinition := false fragmentNameUsed := make(map[string]bool) diff --git a/validator/rules/overlapping_fields_can_be_merged.go b/validator/rules/overlapping_fields_can_be_merged.go index 2ffc3dd1..eaa2035e 100644 --- a/validator/rules/overlapping_fields_can_be_merged.go +++ b/validator/rules/overlapping_fields_can_be_merged.go @@ -12,7 +12,6 @@ import ( ) func init() { - AddRule("OverlappingFieldsCanBeMerged", func(observers *Events, addError AddErrFunc) { /** * Algorithm: @@ -304,10 +303,8 @@ func (m *overlappingFieldsCanBeMergedManager) collectConflictsBetweenFieldsAndFr } func (m *overlappingFieldsCanBeMergedManager) collectConflictsBetweenFragments(conflicts *conflictMessageContainer, areMutuallyExclusive bool, fragmentSpreadA *ast.FragmentSpread, fragmentSpreadB *ast.FragmentSpread) { - var check func(fragmentSpreadA *ast.FragmentSpread, fragmentSpreadB *ast.FragmentSpread) check = func(fragmentSpreadA *ast.FragmentSpread, fragmentSpreadB *ast.FragmentSpread) { - if fragmentSpreadA.Name == fragmentSpreadB.Name { return } diff --git a/validator/rules/possible_fragment_spreads.go b/validator/rules/possible_fragment_spreads.go index bed70289..244e5f20 100644 --- a/validator/rules/possible_fragment_spreads.go +++ b/validator/rules/possible_fragment_spreads.go @@ -9,7 +9,6 @@ import ( func init() { AddRule("PossibleFragmentSpreads", func(observers *Events, addError AddErrFunc) { - validate := func(walker *Walker, parentDef *ast.Definition, fragmentName string, emitError func()) { if parentDef == nil { return diff --git a/validator/rules/values_of_correct_type.go b/validator/rules/values_of_correct_type.go index 7ba6928b..914e428e 100644 --- a/validator/rules/values_of_correct_type.go +++ b/validator/rules/values_of_correct_type.go @@ -159,7 +159,7 @@ func unexpectedTypeMessageOnly(v *ast.Value) ErrorOption { return Message(`Float cannot represent non numeric value: %s`, v.String()) case "ID", "ID!": return Message(`ID cannot represent a non-string and non-integer value: %s`, v.String()) - //case "Enum": + // case "Enum": // return Message(`Enum "%s" cannot represent non-enum value: %s`, v.ExpectedType.String(), v.String()) default: if v.Definition.Kind == ast.Enum { diff --git a/validator/schema.go b/validator/schema.go index 5eacdf90..d8590284 100644 --- a/validator/schema.go +++ b/validator/schema.go @@ -13,7 +13,6 @@ import ( func LoadSchema(inputs ...*Source) (*Schema, error) { sd, err := parser.ParseSchemas(inputs...) - if err != nil { return nil, gqlerror.WrapIfUnwrapped(err) } diff --git a/validator/schema_test.go b/validator/schema_test.go index e7ed5cde..7b994c41 100644 --- a/validator/schema_test.go +++ b/validator/schema_test.go @@ -14,7 +14,7 @@ import ( func TestLoadSchema(t *testing.T) { t.Run("prelude", func(t *testing.T) { s, err := LoadSchema(Prelude) - require.Nil(t, err) + require.NoError(t, err) boolDef := s.Types["Boolean"] require.Equal(t, "Boolean", boolDef.Name) @@ -28,9 +28,9 @@ func TestLoadSchema(t *testing.T) { }) t.Run("swapi", func(t *testing.T) { file, err := os.ReadFile("testdata/swapi.graphql") - require.Nil(t, err) + require.NoError(t, err) s, err := LoadSchema(Prelude, &ast.Source{Input: string(file), Name: "TestLoadSchema"}) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, "Query", s.Query.Name) require.Equal(t, "hero", s.Query.Fields[0].Name) @@ -53,9 +53,9 @@ func TestLoadSchema(t *testing.T) { t.Run("default root operation type names", func(t *testing.T) { file, err := os.ReadFile("testdata/default_root_operation_type_names.graphql") - require.Nil(t, err) + require.NoError(t, err) s, err := LoadSchema(Prelude, &ast.Source{Input: string(file), Name: "TestLoadSchema"}) - require.Nil(t, err) + require.NoError(t, err) require.Nil(t, s.Mutation) require.Nil(t, s.Subscription) @@ -66,9 +66,9 @@ func TestLoadSchema(t *testing.T) { t.Run("type extensions", func(t *testing.T) { file, err := os.ReadFile("testdata/extensions.graphql") - require.Nil(t, err) + require.NoError(t, err) s, err := LoadSchema(Prelude, &ast.Source{Input: string(file), Name: "TestLoadSchema"}) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, "Subscription", s.Subscription.Name) require.Equal(t, "dogEvents", s.Subscription.Fields[0].Name) @@ -89,9 +89,9 @@ func TestLoadSchema(t *testing.T) { t.Run("interfaces", func(t *testing.T) { file, err := os.ReadFile("testdata/interfaces.graphql") - require.Nil(t, err) + require.NoError(t, err) s, err := LoadSchema(Prelude, &ast.Source{Input: string(file), Name: "interfaces"}) - require.Nil(t, err) + require.NoError(t, err) implements := s.GetImplements(s.Types["Canine"]) require.Len(t, implements, 1) @@ -126,7 +126,7 @@ func TestSchemaDescription(t *testing.T) { entity: String } `, BuiltIn: false}) - require.Nil(t, err) + require.NoError(t, err) want := "A simple GraphQL schema which is well described." require.Equal(t, want, s.Description) } diff --git a/validator/validator_test.go b/validator/validator_test.go index c018eb8d..50b6ebc2 100644 --- a/validator/validator_test.go +++ b/validator/validator_test.go @@ -36,7 +36,7 @@ extend type Query { } } }`}) - require.Nil(t, err) + require.NoError(t, err) require.Nil(t, validator.Validate(s, q)) } @@ -68,8 +68,9 @@ query SomeOperation { id } } - `}) - require.Nil(t, err) + `, + }) + require.NoError(t, err) r1 := validator.Validate(s, q1) require.Len(t, r1, 1) const errorString = `SomeOperation:4: Field "myAction" argument "myEnum" of type "Locale!" is required, but it was not provided.` @@ -84,8 +85,9 @@ query SomeOperation ($locale: Locale! = DE) { id } } - `}) - require.Nil(t, err) + `, + }) + require.NoError(t, err) require.Nil(t, validator.Validate(s, q2)) // Repeating same query and expecting to still return same validation error @@ -110,7 +112,7 @@ func TestDeprecatingTypes(t *testing.T) { } _, err := validator.LoadSchema(append([]*ast.Source{validator.Prelude}, schema)...) - require.Nil(t, err) + require.NoError(t, err) } func TestNoUnusedVariables(t *testing.T) { @@ -132,7 +134,7 @@ func TestNoUnusedVariables(t *testing.T) { bar @include(if: $flag) } `}) - require.Nil(t, err) + require.NoError(t, err) require.Nil(t, validator.Validate(s, q)) }) } diff --git a/validator/vars.go b/validator/vars.go index df530234..c386b6b9 100644 --- a/validator/vars.go +++ b/validator/vars.go @@ -67,7 +67,6 @@ func VariableValues(schema *ast.Schema, op *ast.OperationDefinition, variables m return nil, gqlerror.ErrorPathf(validator.path, "cannot use value %f as %s", f, v.Type.NamedType) } rv = reflect.ValueOf(f) - } } if rv.Kind() == reflect.Ptr || rv.Kind() == reflect.Interface { @@ -222,7 +221,7 @@ func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) (reflec if fieldDef.Type.NonNull && field.IsNil() { return val, gqlerror.ErrorPathf(v.path, "cannot be null") } - //allow null object field and skip it + // allow null object field and skip it if !fieldDef.Type.NonNull && field.IsNil() { continue } diff --git a/validator/vars_test.go b/validator/vars_test.go index de54ef6c..b26a9db7 100644 --- a/validator/vars_test.go +++ b/validator/vars_test.go @@ -35,14 +35,14 @@ func TestValidateVars(t *testing.T) { t.Run("with default", func(t *testing.T) { q := gqlparser.MustLoadQuery(schema, `query($id: Int! = 1) { intArg(i: $id) }`) vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), nil) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, 1, vars["id"]) }) t.Run("with union", func(t *testing.T) { q := gqlparser.MustLoadQuery(schema, `query($id: Int! = 1) { intArg(i: $id) }`) vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), nil) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, 1, vars["id"]) }) }) @@ -59,7 +59,7 @@ func TestValidateVars(t *testing.T) { t.Run("defaults", func(t *testing.T) { q := gqlparser.MustLoadQuery(schema, `query foo($var: InputType! = {name: "foo"}) { structArg(i: $var) }`) vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), nil) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, map[string]interface{}{"name": "foo"}, vars["var"]) }) @@ -70,7 +70,7 @@ func TestValidateVars(t *testing.T) { "name": "foobar", }, }) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, map[string]interface{}{"name": "foobar"}, vars["var"]) }) @@ -82,7 +82,7 @@ func TestValidateVars(t *testing.T) { "nullName": nil, }, }) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, map[string]interface{}{"name": "foobar", "nullName": nil}, vars["var"]) }) @@ -112,7 +112,7 @@ func TestValidateVars(t *testing.T) { "nullEmbedded": nil, }, }) - require.Nil(t, gerr) + require.NoError(t, gerr) }) t.Run("unknown field", func(t *testing.T) { @@ -134,9 +134,8 @@ func TestValidateVars(t *testing.T) { "__typename": "InputType", }, }) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, map[string]interface{}{"__typename": "InputType", "name": "foobar"}, vars["var"]) - }) t.Run("enum input object", func(t *testing.T) { @@ -147,7 +146,7 @@ func TestValidateVars(t *testing.T) { "enum": "A", }, }) - require.Nil(t, gerr) + require.NoError(t, gerr) }) t.Run("unknown enum value input object", func(t *testing.T) { @@ -168,7 +167,7 @@ func TestValidateVars(t *testing.T) { vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": map[string]interface{}{"name": "hello"}, }) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, []map[string]interface{}{{"name": "hello"}}, vars["var"]) }) @@ -177,7 +176,7 @@ func TestValidateVars(t *testing.T) { vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": 5, }) - require.Nil(t, gerr) + require.NoError(t, gerr) expected := []int{5} require.EqualValues(t, expected, vars["var"]) }) @@ -187,7 +186,7 @@ func TestValidateVars(t *testing.T) { vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": []map[string]interface{}{{"and": 5}}, }) - require.Nil(t, gerr) + require.NoError(t, gerr) expected := []map[string]interface{}{{"and": []int{5}}} require.EqualValues(t, expected, vars["var"]) }) @@ -195,7 +194,7 @@ func TestValidateVars(t *testing.T) { t.Run("defaults", func(t *testing.T) { q := gqlparser.MustLoadQuery(schema, `query foo($var: [InputType!] = [{name: "foo"}]) { arrayArg(i: $var) }`) vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), nil) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, []interface{}{map[string]interface{}{ "name": "foo", }}, vars["var"]) @@ -208,7 +207,7 @@ func TestValidateVars(t *testing.T) { "name": "foo", }}, }) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, []interface{}{map[string]interface{}{ "name": "foo", }}, vars["var"]) @@ -249,7 +248,7 @@ func TestValidateVars(t *testing.T) { vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": "asdf", }) - require.Nil(t, gerr) + require.NoError(t, gerr) require.EqualValues(t, "asdf", vars["var"]) }) @@ -278,7 +277,7 @@ func TestValidateVars(t *testing.T) { t.Run("Undefined -> Int", func(t *testing.T) { q := gqlparser.MustLoadQuery(schema, `query foo($var: Int) { optionalIntArg(i: $var) }`) _, gerr := validator.VariableValues(schema, q.Operations.ForName(""), nil) - require.Nil(t, gerr) + require.NoError(t, gerr) }) t.Run("Json Number -> Int", func(t *testing.T) { @@ -286,7 +285,7 @@ func TestValidateVars(t *testing.T) { vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": 10, }) - require.Nil(t, gerr) + require.NoError(t, gerr) require.Equal(t, 10, vars["var"]) }) @@ -295,7 +294,7 @@ func TestValidateVars(t *testing.T) { vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": 10.2, }) - require.Nil(t, gerr) + require.NoError(t, gerr) require.Equal(t, 10.2, vars["var"]) }) @@ -304,8 +303,8 @@ func TestValidateVars(t *testing.T) { vars, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": nil, }) - require.Nil(t, gerr) - require.Equal(t, nil, vars["var"]) + require.NoError(t, gerr) + require.Nil(t, vars["var"]) }) t.Run("Bool -> Int", func(t *testing.T) { @@ -326,7 +325,7 @@ func TestValidateVars(t *testing.T) { _, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": []*int{&a, &b, nil}, }) - require.Nil(t, gerr) + require.NoError(t, gerr) }) }) @@ -339,7 +338,7 @@ func TestValidateVars(t *testing.T) { _, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": []*string{&a, &b, nil}, }) - require.Nil(t, gerr) + require.NoError(t, gerr) }) }) @@ -352,7 +351,7 @@ func TestValidateVars(t *testing.T) { _, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ "var": []*bool{&a, &b, nil}, }) - require.Nil(t, gerr) + require.NoError(t, gerr) }) }) } diff --git a/validator/walk.go b/validator/walk.go index 1e34f037..d3140746 100644 --- a/validator/walk.go +++ b/validator/walk.go @@ -22,27 +22,35 @@ type Events struct { func (o *Events) OnOperation(f func(walker *Walker, operation *ast.OperationDefinition)) { o.operationVisitor = append(o.operationVisitor, f) } + func (o *Events) OnField(f func(walker *Walker, field *ast.Field)) { o.field = append(o.field, f) } + func (o *Events) OnFragment(f func(walker *Walker, fragment *ast.FragmentDefinition)) { o.fragment = append(o.fragment, f) } + func (o *Events) OnInlineFragment(f func(walker *Walker, inlineFragment *ast.InlineFragment)) { o.inlineFragment = append(o.inlineFragment, f) } + func (o *Events) OnFragmentSpread(f func(walker *Walker, fragmentSpread *ast.FragmentSpread)) { o.fragmentSpread = append(o.fragmentSpread, f) } + func (o *Events) OnDirective(f func(walker *Walker, directive *ast.Directive)) { o.directive = append(o.directive, f) } + func (o *Events) OnDirectiveList(f func(walker *Walker, directives []*ast.Directive)) { o.directiveList = append(o.directiveList, f) } + func (o *Events) OnValue(f func(walker *Walker, value *ast.Value)) { o.value = append(o.value, f) } + func (o *Events) OnVariable(f func(walker *Walker, variable *ast.VariableDefinition)) { o.variable = append(o.variable, f) } diff --git a/validator/walk_test.go b/validator/walk_test.go index 3343872c..d92b8858 100644 --- a/validator/walk_test.go +++ b/validator/walk_test.go @@ -10,9 +10,9 @@ import ( func TestWalker(t *testing.T) { schema, err := LoadSchema(Prelude, &ast.Source{Input: "type Query { name: String }\n schema { query: Query }"}) - require.Nil(t, err) + require.NoError(t, err) query, err := parser.ParseQuery(&ast.Source{Input: "{ as: name }"}) - require.Nil(t, err) + require.NoError(t, err) called := false observers := &Events{} @@ -32,9 +32,9 @@ func TestWalker(t *testing.T) { func TestWalkInlineFragment(t *testing.T) { schema, err := LoadSchema(Prelude, &ast.Source{Input: "type Query { name: String }\n schema { query: Query }"}) - require.Nil(t, err) + require.NoError(t, err) query, err := parser.ParseQuery(&ast.Source{Input: "{ ... { name } }"}) - require.Nil(t, err) + require.NoError(t, err) called := false observers := &Events{}