From a853d9887ccc14ce0b58b6a49ef8c60d4c11c538 Mon Sep 17 00:00:00 2001 From: Keegan Campbell Date: Tue, 30 Jan 2024 11:29:11 -0800 Subject: [PATCH] maint: Introduce linting * Add linting tools and configuration * Fix linter error in example * Add blank line to end of script/lint.sh --- .github/workflows/lint.yml | 32 +++++ .golangci.toml | 214 +++++++++++++++++++++++++++++ pkg/authentication/example_test.go | 12 +- script/lint.sh | 9 ++ 4 files changed, 261 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100644 .golangci.toml create mode 100755 script/lint.sh diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..2f7cffe0 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,32 @@ +name: golangci-lint +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + checks: write + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-go@v4 + with: + go-version: '1.21' + cache: false + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + version: v1.55 + + # Optional: golangci-lint command line arguments. + # + # Note: By default, the `.golangci.yml` file should be at the root of the repository. + # The location of the configuration file can be changed by using `--config=` + # args: --timeout=30m --config=/my/path/.golangci.yml --issues-exit-code=0 + diff --git a/.golangci.toml b/.golangci.toml new file mode 100644 index 00000000..19ed0e2d --- /dev/null +++ b/.golangci.toml @@ -0,0 +1,214 @@ + +[linters] + + # set all to disabled so we can manually enable only the ones we want + disable-all = true + + # This is a list is just the linters that are critical to run and pass on + # our code. These linters find bugs. If they find a problem with your code, + # you should fix it immediately. + # + # Take care when adding to this list, as any code that fails these linters + # anywhere in any repo that uses this linter will need to be fixed before + # that repo can updating the version of go-linter it's using. + enable = [ + "bodyclose", # checks whether HTTP response body is closed successfully + # Forgetting to close an HTTP body can be a memory leak + "durationcheck", # check for two durations multiplied together + # this is probably a rare bug, but should have basically zero false positives. + "exportloopref", # catch bugs resulting from referencing variables on range scope + # variables initialized in for loops change with each loop, which can cause bugs. + "gocritic", # Provides many diagnostics that check for bugs, performance and style issues. + # This is highly configurable, see the gocritic config section below. + "gosec", # Inspects source code for security problems + # high quality linter that finds real bugs + "govet", # reports suspicious constructs like printf calls that don't have the right # of arguments + # high quality, low false positives + "ineffassign", # Detects when assignments to existing variables are not used + # this finds bugs all the time, where you assign to a value but then never use + # the assigned value due to shadowing etc. + "nilerr", # Finds the code that returns nil even if it checks that the error is not nil. + # finds fairly common bug + "rowserrcheck", # checks whether Err of rows is checked successfully + # finds bugs in SQL code + "sqlclosecheck", # Checks that sql.Rows and sql.Stmt are closed. + # easy and finds bugs + ] + + # We manually enable only the linters we want, above, so we don't need a + # manual disable list as well. + disable = [] + +[run] + # options for analysis running + # Increase timeout from default 1m, first pre-cache run can take a bit in CI/CD + timeout = "5m" + + # default concurrency is the available CPU number + # concurrency = 4 + + # exit code when at least one issue was found, default is 1 + issues-exit-code = 1 + + # include test files or not, default is true + tests = true + + # list of build tags, all linters use it. Default is empty list. + build-tags = [] + + # which dirs to skip: issues from them won't be reported; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but default dirs are skipped independently + # from this option's value (see skip-dirs-use-default). + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + skip-dirs = [] + + # default is true. Enables skipping of directories: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs-use-default = true + + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + + # skipping pkg/github/markdown/raw_request_builder.go's error of + # "G104: Errors unhandled. + # "requestInfo.SetContentFromScalar(ctx, m.BaseRequestBuilder.RequestAdapter, "text/plain", body)" + # because it happens due to generation and is not quickly fixed. + # 'exclude' cannot be used here because the error text applies to many possible errors + # and we don't want to ignore other instances of this error + skip-files = [ "pkg/github/markdown/raw_request_builder.go" ] + + # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules": + # If invoked with -mod=readonly, the go command is disallowed from the implicit + # automatic updating of go.mod described above. Instead, it fails when any changes + # to go.mod are needed. This setting is most useful to check that go.mod does + # not need updates, such as in a continuous integration and testing system. + # If invoked with -mod=vendor, the go command assumes that the vendor + # directory holds the correct copies of dependencies and ignores + # the dependency descriptions in go.mod. + modules-download-mode = "" + + # Allow multiple parallel golangci-lint instances running. + # If false (default) - golangci-lint acquires file lock on start. + allow-parallel-runners = false + +[output] + # colored-line-number|line-number|json|tab|checkstyle|code-climate|junit-xml|github-actions + # default is "colored-line-number" + format = "colored-line-number" + + # print lines of code with issue, default is true + print-issued-lines = true + + # print linter name in the end of issue text, default is true + print-linter-name = true + + # make issues output unique by line, default is true + uniq-by-line = true + + # add a prefix to the output file references; default is no prefix + path-prefix = "" + + # sorts results by: filepath, line and column + sort-results = true + +# options to enable differentiating between error and warning severities +[severity] + # GitHub Actions annotations support error and warning only: + # https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#setting-an-error-message + default-severity = "error" + + # If set to true severity-rules regular expressions become case sensitive. + # The default value is false. + case-sensitive = false + + # Default value is empty list. + # When a list of severity rules are provided, severity information will be added to lint + # issues. Severity rules have the same filtering capability as exclude rules except you + # are allowed to specify one matcher per severity rule. + # Only affects out formats that support setting severity information. + # [[severity.rules]] + # linters = [ + # "revive", + # ] + # severity = "warning" + +[issues] + # List of regexps of issue texts to exclude, empty list by default. + # Please document every exception here so we know what we're suppressing and why. + exclude = [] + + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter = 0 + + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues = 0 + + # The default value is false. If set to true exclude and exclude-rules + # regular expressions become case sensitive. + # exclude-case-sensitive = false + + # This flag suppresses lint issues from several linters, overriding any other configuration you have set. + # It defaults to true. + # NEVER remove this configuration. If you want to suppress something, do so explicitly elsewhere. + exclude-use-default = false + + # The list of ids of default excludes to include or disable. By default it's empty. + # We shouldn't ever need this, since we turn off default excludes. + include = [] + + # Show only new issues: if there are unstaged changes or untracked files, + # only those changes are analyzed, else only changes in HEAD~ are analyzed. + # It's a super-useful option for integration of golangci-lint into existing + # large codebase. It's not practical to fix all existing issues at the moment + # of integration: much better don't allow issues in new code. + # Default is false. + new = false + + # Show only new issues created in git patch with set file path. + # new-from-patch = "path/to/patch/file" + + # Show only new issues created after git revision `REV` + # new-from-rev = "REV" + + # Fix found issues (if it's supported by the linter). Default is false. + fix = false + + # reduce noise in some linters that don't necessarily need to be run in tests + [[issues.exclude-rules]] + path = "_test\\.go" + linters = ["errcheck", "gosec", "noctx", "bodyclose"] + +# +# Specific Linter Settings +# + +[linters-settings.gocritic] + # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks. + # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". + enabled-tags = [ + # "diagnostic", // enabled by default + ] + disabled-tags = ["style"] + disabled-checks = ["appendAssign"] + +[linters-settings.govet] + enable = [ "httpresponse" ] + +[linters-settings.gosec] + excludes = [ + "G110", # potential decompression bomb + "G204", # subprocess launched with variable. + "G301", # expect directory permissions to be 0750 or less. + "G302", # Expect file permissions to be 0600 or less. + "G304", # file inclusion via variable. + "G307", # deferring methods with errors. + "G404", # use of math/rand + ] +[linters-settings.gosec.config.G104] + os = ["Setenv"] diff --git a/pkg/authentication/example_test.go b/pkg/authentication/example_test.go index d9cf2f16..33ec8ea0 100644 --- a/pkg/authentication/example_test.go +++ b/pkg/authentication/example_test.go @@ -12,7 +12,9 @@ import ( "github.com/octokit/go-sdk/pkg/github/octocat" ) -func ExampleUnauthenticatedRequest() { +// ExampleApiClient_Octocat shows how to initialize an unauthenticated client +// and make a simple API request. +func ExampleApiClient_Octocat() { tokenProvider := auth.NewTokenProvider( // to create an authenticated provider, uncomment the below line and pass in your token // auth.WithAuthorizationToken("ghp_your_token"), @@ -25,16 +27,15 @@ func ExampleUnauthenticatedRequest() { client := github.NewApiClient(adapter) - // unauthenticated request s := "Salutations" - // create headers that accept json back; our spec says octet-stream - // but that's not actually what the API returns in this case + // create headers that accept json back; GitHub's OpenAPI definition says + // octet-stream but that's not actually what the API returns in this case headers := abstractions.NewRequestHeaders() _ = headers.TryAdd("Accept", "application/vnd.github.v3+json") octocatRequestConfig := &abstractions.RequestConfiguration[octocat.OctocatRequestBuilderGetQueryParameters]{ - QueryParameters: &octocat.OctocatRequestBuilderGetQueryParameters { + QueryParameters: &octocat.OctocatRequestBuilderGetQueryParameters{ S: &s, }, Headers: headers, @@ -67,5 +68,4 @@ func ExampleUnauthenticatedRequest() { // ~~~~~~==~==~~~==~==~~~~~~ // ~~~~~~==~==~==~==~~~~~~ // :~==~==~==~==~~ - } diff --git a/script/lint.sh b/script/lint.sh new file mode 100755 index 00000000..1bc77823 --- /dev/null +++ b/script/lint.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash + +# GitHub Actions runs linting checks from .github/workflows/lint.yml. +# This script may be used to detect issues before pushing to a PR check. + +# note: check the version of golangci-lint used in .github/workflows/lint.yml to ensure accuracy +go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55 + +golangci-lint run ./...