Skip to content

Commit

Permalink
Update golangci-lint action to go through all dirs if no working dire…
Browse files Browse the repository at this point in the history
…ctory set
  • Loading branch information
chudilka1 committed Nov 27, 2024
1 parent 959ab5a commit 0569164
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 25 deletions.
18 changes: 7 additions & 11 deletions .github/actions/golangci-lint/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ runs:
- name: Touching core/web/assets/index.html
shell: bash
run: mkdir -p core/web/assets && touch core/web/assets/index.html
- name: Build binary
working-directory: ${{ inputs.go-directory }}
shell: bash
run: go build ./...
- name: Set golangci-lint working directory
shell: bash
id: set-working-directory
Expand All @@ -59,18 +55,18 @@ runs:
- name: golangci-lint
uses: golangci/golangci-lint-action@38e1018663fa5173f3968ea0777460d3de38f256 # v5.3.0
with:
version: v1.61.0
version: v1.62.2
only-new-issues: true
args: --out-format colored-line-number,checkstyle:golangci-lint-report.xml
args: --out-format colored-line-number,checkstyle:${{ steps.set-working-directory.outputs.golangci-lint-working-directory }}_golangci-lint-report.xml -v
working-directory: ${{ steps.set-working-directory.outputs.golangci-lint-working-directory }}
- name: Print lint report artifact
if: failure()
shell: bash
run: cat ${{ inputs.go-directory }}/golangci-lint-report.xml
shell: bashc
run: cat ./${{ inputs.go-directory }}/${{ steps.set-working-directory.outputs.golangci-lint-working-directory }}_golangci-lint-report.xml
- name: Store lint report artifact
if: always()
uses: actions/upload-artifact@v4.4.3
with:
name: golangci-lint-report
path: ${{ inputs.go-directory }}/golangci-lint-report.xml
retention-days: 7
name: golangci-lint-report-${{ inputs.go-directory }}
# The ./ is needed to preserve the valid pattern for the artifact path
path: ./${{ inputs.go-directory }}/${{ inputs.go-directory }}_golangci-lint-report.xml
48 changes: 35 additions & 13 deletions .github/workflows/ci-core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ jobs:
permissions:
pull-requests: read
outputs:
affected-packages: ${{ steps.affected-modules.outputs.changes }}
deployment-changes: ${{ steps.match-some.outputs.deployment == 'true' }}
should-run-ci-core: ${{ steps.match-some.outputs.core-ci == 'true' || steps.match-every.outputs.non-ignored == 'true' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' }}
should-run-golangci: ${{ steps.match-some.outputs.golang-ci == 'true' || steps.match-every.outputs.non-ignored == 'true' || github.event_name == 'workflow_dispatch' }}
Expand All @@ -47,7 +48,7 @@ jobs:
with:
# "if any changed file matches one or more of the conditions" (https://github.com/dorny/paths-filter/issues/225)
predicate-quantifier: some
# deployment - any changes to files in `deployments/`
# deployment - any changes to files in the `deployments/`
# core-ci - any changes that could affect this workflow definition
# golang-ci - any changes that could affect the linting result
filters: |
Expand Down Expand Up @@ -75,6 +76,7 @@ jobs:
non-ignored:
- '**'
- '!docs/**'
- '!fuzz/**'
- '!integration-tests/**'
- '!tools/secrets/**'
- '!tools/goreleaser-config/**'
Expand All @@ -91,24 +93,44 @@ jobs:
- '!nix-darwin-shell-hook.sh'
- '!LICENSE'
- '!.github/**'
- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
id: affected-modules
with:
# to get affected packages as output (not simply `true` or `false`)
# use the following syntax `package_name: 'path/to/package'`
filters: |
ccip: 'ccip/**'
common: 'common/**'
core: 'core/**'
dashboard-lib: 'dashboard-lib/**'
deployment: 'deployment/**'
plugins: 'plugins/**'
golangci:
# We don't directly merge dependabot PRs, so let's not waste the resources
if: ${{ (github.event_name == 'pull_request' || github.event_name == 'schedule') && github.actor != 'dependabot[bot]' }}
name: lint
# We don't directly merge dependabot PRs, so let's not waste the resources.
# toJson/fromJson is used to account for empty array formatting differences ([], [ ], etc.)
# if: ${{ (github.event_name == 'pull_request' || github.event_name == 'schedule') && github.actor != 'dependabot[bot]' && needs.filter.outputs.should-run-golangci == 'true' && (toJson(fromJson(needs.filter.outputs.affected-packages)) != '[]' && needs.filter.outputs.affected-packages != '')}}
if: always()
needs: [filter, run-frequency]
permissions:
# For golangci-lint-actions to annotate code in the PR.
# To annotate code in the PR.
checks: write
contents: read
# For golangci-lint-action's `only-new-issues` option.
pull-requests: read
runs-on: ubuntu-24.04-8cores-32GB-ARM
needs: [filter, run-frequency]
strategy:
fail-fast: false
matrix:
modules: ${{ fromJson(needs.filter.outputs.affected-packages) }}
steps:
- uses: actions/checkout@v4.2.1
- name: Golang Lint
- name: Checkout
uses: actions/checkout@v4.2.1
- name: Golang Lint (${{ matrix.modules }})
uses: ./.github/actions/golangci-lint
if: ${{ needs.filter.outputs.should-run-golangci == 'true' }}
with:
go-directory: ${{ matrix.modules }}
- name: Notify Slack
if: ${{ failure() && needs.run-frequency.outputs.one-per-day-frequency == 'true' }}
uses: slackapi/slack-github-action@6c661ce58804a1a20f6dc5fbee7f0381b469e001 # v1.25.0
Expand Down Expand Up @@ -375,16 +397,16 @@ jobs:

scan:
name: SonarQube Scan
needs: [core, run-frequency]
if: ${{ always() && needs.run-frequency.outputs.four-per-day-frequency == 'true' && github.actor != 'dependabot[bot]' }}
needs: [core]
if: ${{ always() && github.actor != 'dependabot[bot]' }}
runs-on: ubuntu-latest
steps:
- name: Checkout the repo
uses: actions/checkout@v4.2.1
with:
fetch-depth: 0 # fetches all history for all tags and branches to provide more metadata for sonar reports

- name: Download all workflow run artifacts
- name: Download all workflow artifacts
uses: actions/download-artifact@v4.1.8

- name: Check and Set SonarQube Report Paths
Expand All @@ -410,7 +432,7 @@ jobs:
# Check and assign paths for lint reports
if [ -d "golangci-lint-report" ]; then
sonarqube_lint_report_paths=$(find golangci-lint-report -name golangci-lint-report.xml | paste -sd "," -)
sonarqube_lint_report_paths=$(find -type f -name 'golangci-lint-report.xml' -printf "%p,")
else
sonarqube_lint_report_paths=""
fi
Expand Down
3 changes: 2 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
run:
timeout: 15m0s
allow-parallel-runners: true
allow-serial-runners: true
linters:
enable:
- containedctx
- depguard
- errname
- errorlint
Expand Down
44 changes: 44 additions & 0 deletions ccip/fail_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package ccip

import (
"errors"
"os"
"sync"
"testing"
)

func TestFail(t *testing.T) {
if testing.Short() {
t.Skip()
}
t.Fatal("fake failure")
}

func TestRace(t *testing.T) {
var v int
var wg sync.WaitGroup
wg.Add(100)
for i := 0; i < 100; i++ {
go func() {
defer wg.Done()
v++
v--
}()
}
wg.Wait()
t.Log(v)
}

func TestLint(t *testing.T) {
const v1 = (true && false) && (true && false) // SQ Identical expressions should not be used on both sides of a binary operator

Check failure on line 33 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / lint (ccip)

dupSubExpr: suspicious identical LHS and RHS for `&&` operator (gocritic)

Check failure on line 33 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / Flakeguard Root Project / Run Tests (github.com/smartcontractkit/chainlink/v2/ccip, ubuntu-latest)

redundant and: false && false

Check failure on line 33 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / Flakeguard Root Project / Run Tests (github.com/smartcontractkit/chainlink/v2/ccip, ubuntu-latest)

redundant and: true && true

Check failure on line 33 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / Core Tests (go_core_race_tests)

redundant and: false && false

Check failure on line 33 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / Core Tests (go_core_race_tests)

redundant and: true && true

Check failure on line 33 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / Core Tests (go_core_tests)

redundant and: false && false

Check failure on line 33 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / Core Tests (go_core_tests)

redundant and: true && true
a := 1
if !(a == 2) { // SQ boolean check should not be inverted

Check failure on line 35 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / lint (ccip)

empty-block: this block is empty, you can remove it (revive)
}
const UnusedVar = 1 // lint should complain for unused variable

Check failure on line 37 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / lint (ccip)

const `UnusedVar` is unused (unused)
const ALL_CAPS = 10 // should be AllCaps

Check failure on line 38 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / lint (ccip)

var-naming: don't use ALL_CAPS in Go names; use CamelCase (revive)
err := os.ErrNotExist
if err == os.ErrNotExist { // should use errors.Is

Check failure on line 40 in ccip/fail_test.go

View workflow job for this annotation

GitHub Actions / lint (ccip)

comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
err := errors.New("fake error") // shadowed variable
t.Log(err)
}
}
47 changes: 47 additions & 0 deletions core/fail_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package core

import (
"context"
"sync"
"testing"
)

type MyStruct struct {
Ctx context.Context // triggers `containedctx`` rule
}

func TestFail(t *testing.T) {
if testing.Short() {
t.Skip()
}
t.Fatal("fake failure")
}

func TestRace(t *testing.T) {
var v int
var wg sync.WaitGroup
wg.Add(100)
for i := 0; i < 100; i++ {
go func() {
defer wg.Done()
v++
v--
}()
}
wg.Wait()
t.Log(v)
}

// func TestLint(t *testing.T) {
// const v1 = (true && false) && (true && false) // SQ Identical expressions should not be used on both sides of a binary operator
// a := 1
// if !(a == 2) { // SQ boolean check should not be inverted
// }
// const UnusedVar = 1 // lint should complain for unused variable
// const ALL_CAPS = 10 // should be AllCaps
// err := os.ErrNotExist
// if err == os.ErrNotExist { // should use errors.Is
// err := errors.New("fake error") // shadowed variable
// t.Log(err)
// }
// }
42 changes: 42 additions & 0 deletions dashboard-lib/fail_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package dashboard_lib

Check failure on line 1 in dashboard-lib/fail_test.go

View workflow job for this annotation

GitHub Actions / lint (dashboard-lib)

var-naming: don't use an underscore in package name (revive)

import (
"sync"
"testing"
)

func TestFail(t *testing.T) {
if testing.Short() {
t.Skip()
}
t.Fatal("fake failure")
}

func TestRace(t *testing.T) {
var v int
var wg sync.WaitGroup
wg.Add(100)
for i := 0; i < 100; i++ {
go func() {
defer wg.Done()
v++
v--
}()
}
wg.Wait()
t.Log(v)
}

// func TestLint(t *testing.T) {
// const v1 = (true && false) && (true && false) // SQ Identical expressions should not be used on both sides of a binary operator
// a := 1
// if !(a == 2) { // SQ boolean check should not be inverted
// }
// const UnusedVar = 1 // lint should complain for unused variable
// const ALL_CAPS = 10 // should be AllCaps
// err := os.ErrNotExist
// if err == os.ErrNotExist { // should use errors.Is
// err := errors.New("fake error") // shadowed variable
// t.Log(err)
// }
// }
5 changes: 5 additions & 0 deletions deployment/fail_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package deployment

import (
"context"
"errors"
"os"
"sync"
"testing"
)

type MyStruct struct {
Ctx context.Context // triggers context rules

Check failure on line 12 in deployment/fail_test.go

View workflow job for this annotation

GitHub Actions / lint (deployment)

found a struct that contains a context.Context field (containedctx)
}

func TestFail(t *testing.T) {
if testing.Short() {
t.Skip()
Expand Down

0 comments on commit 0569164

Please sign in to comment.