Skip to content

Commit

Permalink
maint: Add linting for Go to project (#52)
Browse files Browse the repository at this point in the history
* Add linting to project

* Separate linting steps for each post-processor
  • Loading branch information
kfcampbell committed Mar 5, 2024
1 parent 53bc1c2 commit fb91690
Show file tree
Hide file tree
Showing 6 changed files with 273 additions and 7 deletions.
43 changes: 43 additions & 0 deletions .github/workflows/lint.yml
@@ -0,0 +1,43 @@
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.22'
cache: false
- name: lint go post-processor
uses: golangci/golangci-lint-action@v3
with:
version: v1.56
args: post-processors/go/main.go
- name: lint csharp post-processor
uses: golangci/golangci-lint-action@v3
with:
version: v1.56
args: post-processors/csharp/main.go
- name: lint schema tooling
uses: golangci/golangci-lint-action@v3
with:
version: v1.56
args: schemas/main.go

# 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

213 changes: 213 additions & 0 deletions .golangci.toml
@@ -0,0 +1,213 @@
[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"]
5 changes: 2 additions & 3 deletions post-processors/csharp/main.go
Expand Up @@ -42,15 +42,14 @@ func run() error {
fileContents = fixStringToBool(fileContents)
}

if file.Name() == "WebhookConfigInsecureSsl.cs" {
if file.Name() == "WebhookConfigInsecureSsl.cs" {
fileContents = fixUsing(fileContents)
}

fileContents = fixThumbsUpThumbsDownProperties(fileContents)
fileContents = fixPagesPaths(fileContents)

// TODO(kfcampbell): verify file permission is what we want
err = os.WriteFile(path, []byte(fileContents), 0666)
err = os.WriteFile(path, []byte(fileContents), 0600)
if err != nil {
return err
}
Expand Down
5 changes: 2 additions & 3 deletions post-processors/go/main.go
Expand Up @@ -43,8 +43,7 @@ func run() error {

fileContents = fixKiotaNonDeterminism(fileContents, file.Name())

// TODO(kfcampbell): verify file permission is what we want
err = os.WriteFile(path, []byte(fileContents), 0666)
err = os.WriteFile(path, []byte(fileContents), 0600)
if err != nil {
return err
}
Expand Down Expand Up @@ -124,7 +123,7 @@ func run() error {
stdout.Reset()
stderr.Reset()

output, err = cmd.Output()
_, err = cmd.Output()
if err != nil {
fmt.Printf("could not run go mod tidy: %v\nfull error log:\n%s", err, stderr.String())
}
Expand Down
5 changes: 4 additions & 1 deletion scripts/install-tools.sh
@@ -1,4 +1,7 @@
#!/bin/sh
#!/usr/bin/env bash

# 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.56

# Install Kiota
KIOTA_TOOL_NAME="microsoft.openapi.kiota"
Expand Down
9 changes: 9 additions & 0 deletions scripts/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.
set -ex
./scripts/install-tools.sh
golangci-lint run post-processors/go/main.go
golangci-lint run post-processors/csharp/main.go
golangci-lint run schemas/main.go

0 comments on commit fb91690

Please sign in to comment.