Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/cloc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v3
uses: actions/checkout@v4
with:
path: pr
- name: Checkout base code
uses: actions/checkout@v3
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.base.sha }}
path: base
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ jobs:
name: golangci-lint
runs-on: ubuntu-latest
steps:
- uses: actions/setup-go@v3
- uses: actions/setup-go@v5
with:
go-version: 1.20.x
- uses: actions/checkout@v2
go-version: stable
- uses: actions/checkout@v4
- name: golangci-lint
uses: golangci/golangci-lint-action@v3.4.0
uses: golangci/golangci-lint-action@v8.0.0
with:
# Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
version: v1.51.1
version: v2.4.0

# Optional: working directory, useful for monorepos
# working-directory: somedir
Expand Down
20 changes: 5 additions & 15 deletions .github/workflows/gorelease.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,19 @@ concurrency:
cancel-in-progress: true

env:
GO_VERSION: 1.20.x
GO_VERSION: stable
jobs:
gorelease:
runs-on: ubuntu-latest
steps:
- name: Install Go stable
if: env.GO_VERSION != 'tip'
uses: actions/setup-go@v4
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: ${{ env.GO_VERSION }}
Comment on lines +12 to 20
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 | Confidence: High

The change replaces specific Go version pinning (1.20.x) with 'stable', which introduces non-deterministic build behavior. This could cause the gorelease tool to analyze the code against different Go versions over time, potentially leading to inconsistent API compatibility reports. For a release tooling workflow, deterministic behavior is critical to ensure consistent analysis results across different runs.

Suggested change
GO_VERSION: stable
jobs:
gorelease:
runs-on: ubuntu-latest
steps:
- name: Install Go stable
if: env.GO_VERSION != 'tip'
uses: actions/setup-go@v4
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: ${{ env.GO_VERSION }}
env:
GO_VERSION: 1.21.x # Pin to a specific supported version

Evidence: constant:GO_VERSION, search:stable

- name: Install Go tip
if: env.GO_VERSION == 'tip'
run: |
curl -sL https://storage.googleapis.com/go-build-snap/go/linux-amd64/$(git ls-remote https://github.com/golang/go.git HEAD | awk '{print $1;}').tar.gz -o gotip.tar.gz
ls -lah gotip.tar.gz
mkdir -p ~/sdk/gotip
tar -C ~/sdk/gotip -xzf gotip.tar.gz
~/sdk/gotip/bin/go version
echo "PATH=$HOME/go/bin:$HOME/sdk/gotip/bin/:$PATH" >> $GITHUB_ENV
- name: Checkout code
uses: actions/checkout@v3
uses: actions/checkout@v4
- name: Gorelease cache
uses: actions/cache@v3
uses: actions/cache@v4
with:
path: |
~/go/bin/gorelease
Expand Down
26 changes: 12 additions & 14 deletions .github/workflows/release-assets.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,18 @@ on:
- created
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GO_VERSION: 1.20.x
GO_VERSION: stable
jobs:
build:
name: Upload Release Assets
runs-on: ubuntu-latest
steps:
- name: Install Go stable
if: env.GO_VERSION != 'tip'
uses: actions/setup-go@v4
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: ${{ env.GO_VERSION }}
- name: Install Go tip
if: env.GO_VERSION == 'tip'
run: |
curl -sL https://storage.googleapis.com/go-build-snap/go/linux-amd64/$(git ls-remote https://github.com/golang/go.git HEAD | awk '{print $1;}').tar.gz -o gotip.tar.gz
ls -lah gotip.tar.gz
mkdir -p ~/sdk/gotip
tar -C ~/sdk/gotip -xzf gotip.tar.gz
~/sdk/gotip/bin/go version
echo "PATH=$HOME/go/bin:$HOME/sdk/gotip/bin/:$PATH" >> $GITHUB_ENV
- name: Checkout code
uses: actions/checkout@v3
uses: actions/checkout@v4
- name: Build artifacts
run: |
make release-assets
Expand All @@ -41,6 +31,14 @@ jobs:
asset_path: ./linux_amd64.tar.gz
asset_name: linux_amd64.tar.gz
asset_content_type: application/tar+gzip
- name: Upload linux_amd64_dbg.tar.gz
if: hashFiles('linux_amd64_dbg.tar.gz') != ''
uses: actions/upload-release-asset@v1
with:
upload_url: ${{ github.event.release.upload_url }}
asset_path: ./linux_amd64_dbg.tar.gz
asset_name: linux_amd64_dbg.tar.gz
asset_content_type: application/tar+gzip
Comment on lines +34 to +41
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: Medium

Speculative: The addition of debug binary uploads suggests the build process now generates debug artifacts, but there's no corresponding change in the Makefile to show how these are built. Without seeing the build configuration changes, it's unclear if debug symbols are properly stripped from release binaries or if debug builds follow security best practices (e.g., not including sensitive information).

- name: Upload linux_arm64.tar.gz
if: hashFiles('linux_arm64.tar.gz') != ''
uses: actions/upload-release-asset@v1
Expand Down
40 changes: 15 additions & 25 deletions .github/workflows/test-unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,36 +15,25 @@ concurrency:
env:
GO111MODULE: "on"
RUN_BASE_COVERAGE: "on" # Runs test for PR base in case base test coverage is missing.
COV_GO_VERSION: 1.20.x # Version of Go to collect coverage
COV_GO_VERSION: stable # Version of Go to collect coverage
TARGET_DELTA_COV: 90 # Target coverage of changed lines, in percents
jobs:
test:
strategy:
matrix:
go-version: [ 1.13.x, 1.19.x, 1.20.x ]
go-version: [ 1.13.x, stable, oldstable ]
runs-on: ubuntu-latest
steps:
- name: Install Go stable
if: matrix.go-version != 'tip'
uses: actions/setup-go@v4
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go-version }}

- name: Install Go tip
if: matrix.go-version == 'tip'
run: |
curl -sL https://storage.googleapis.com/go-build-snap/go/linux-amd64/$(git ls-remote https://github.com/golang/go.git HEAD | awk '{print $1;}').tar.gz -o gotip.tar.gz
ls -lah gotip.tar.gz
mkdir -p ~/sdk/gotip
tar -C ~/sdk/gotip -xzf gotip.tar.gz
~/sdk/gotip/bin/go version
echo "PATH=$HOME/go/bin:$HOME/sdk/gotip/bin/:$PATH" >> $GITHUB_ENV

- name: Checkout code
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Go cache
uses: actions/cache@v3
uses: actions/cache@v4
with:
# In order:
# * Module download cache
Expand All @@ -59,7 +48,7 @@ jobs:
- name: Restore base test coverage
id: base-coverage
if: matrix.go-version == env.COV_GO_VERSION && github.event.pull_request.base.sha != ''
uses: actions/cache@v2
uses: actions/cache@v4
with:
path: |
unit-base.txt
Expand Down Expand Up @@ -88,14 +77,15 @@ jobs:
id: annotate
if: matrix.go-version == env.COV_GO_VERSION && github.event.pull_request.base.sha != ''
run: |
curl -sLO https://github.com/vearutop/gocovdiff/releases/download/v1.3.6/linux_amd64.tar.gz && tar xf linux_amd64.tar.gz
curl -sLO https://github.com/vearutop/gocovdiff/releases/download/v1.4.2/linux_amd64.tar.gz && tar xf linux_amd64.tar.gz && rm linux_amd64.tar.gz
gocovdiff_hash=$(git hash-object ./gocovdiff)
[ "$gocovdiff_hash" == "8e507e0d671d4d6dfb3612309b72b163492f28eb" ] || (echo "::error::unexpected hash for gocovdiff, possible tampering: $gocovdiff_hash" && exit 1)
git fetch origin master ${{ github.event.pull_request.base.sha }}
REP=$(./gocovdiff -cov unit.coverprofile -gha-annotations gha-unit.txt -delta-cov-file delta-cov-unit.txt -target-delta-cov ${TARGET_DELTA_COV})
[ "$gocovdiff_hash" == "c37862c73a677e5a9c069470287823ab5bbf0244" ] || (echo "::error::unexpected hash for gocovdiff, possible tampering: $gocovdiff_hash" && exit 1)
# Fetch PR diff from GitHub API.
curl -s -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" -H "Accept: application/vnd.github.v3.diff" https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }} > pull_request.diff
REP=$(./gocovdiff -diff pull_request.diff -mod github.com/$GITHUB_REPOSITORY -cov unit.coverprofile -gha-annotations gha-unit.txt -delta-cov-file delta-cov-unit.txt -target-delta-cov ${TARGET_DELTA_COV})
Comment on lines +80 to +85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 | Confidence: High

The test coverage analysis now fetches PR diff via GitHub API instead of using git fetch, which changes the fundamental approach to determining code changes. This introduces a dependency on GitHub API rate limits and authentication that wasn't present before. The new method may fail if the GitHub token lacks permissions or if API limits are exceeded, potentially breaking the coverage analysis workflow. The removal of git fetch also means the analysis no longer has access to the actual base commit files, which could lead to inaccurate coverage calculations.

echo "${REP}"
cat gha-unit.txt
DIFF=$(test -e unit-base.txt && ./gocovdiff -func-cov unit.txt -func-base-cov unit-base.txt || echo "Missing base coverage file")
DIFF=$(test -e unit-base.txt && ./gocovdiff -mod github.com/$GITHUB_REPOSITORY -func-cov unit.txt -func-base-cov unit-base.txt || echo "Missing base coverage file")
TOTAL=$(cat delta-cov-unit.txt)
echo "rep<<EOF" >> $GITHUB_OUTPUT && echo "$REP" >> $GITHUB_OUTPUT && echo "EOF" >> $GITHUB_OUTPUT
echo "diff<<EOF" >> $GITHUB_OUTPUT && echo "$DIFF" >> $GITHUB_OUTPUT && echo "EOF" >> $GITHUB_OUTPUT
Expand Down Expand Up @@ -130,7 +120,7 @@ jobs:

- name: Upload code coverage
if: matrix.go-version == env.COV_GO_VERSION
uses: codecov/codecov-action@v1
uses: codecov/codecov-action@v5
with:
file: ./unit.coverprofile
files: ./unit.coverprofile
flags: unittests
136 changes: 74 additions & 62 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,73 +1,85 @@
# See https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml
# See https://golangci-lint.run/docs/linters/configuration/
version: "2"
run:
tests: true

linters-settings:
errcheck:
check-type-assertions: true
check-blank: true
gocyclo:
min-complexity: 20
dupl:
threshold: 100
misspell:
locale: US
unused:
check-exported: false
unparam:
check-exported: true
cyclop:
max-complexity: 15

linters:
enable-all: true
default: all
disable:
- nilnil
- tparallel
- goerr113
- err113
Comment on lines 5 to +10
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: Medium

The linter configuration switches from enabling specific linters to enabling all and then disabling many. This approach may catch more issues but could introduce noise from linters not suited to this codebase. The extensive disable list (28+ linters) suggests the 'enable all' approach might not be optimal for this project's coding style and could make the configuration harder to maintain.

- errorlint
- lll
- maligned
- gochecknoglobals
- gomnd
- wrapcheck
- paralleltest
- noinlineerr
- wsl_v5
- funcorder
- copyloopvar
- depguard
- dupword
- errname
- exhaustruct
- forbidigo
- exhaustivestruct
- interfacer # deprecated
- forcetypeassert
- scopelint # deprecated
- ifshort # too many false positives
- golint # deprecated
- varnamelen
- tagliatelle
- errname
- gochecknoglobals
- intrange
- ireturn
- exhaustruct
- lll
- mnd
- nonamedreturns
- nosnakecase
- structcheck
- varcheck
- deadcode
- paralleltest
- recvcheck
- tagalign
- tagliatelle
- testableexamples
- dupword

issues:
exclude-use-default: false
exclude-rules:
- linters:
- gomnd
- goconst
- goerr113
- noctx
- funlen
- dupl
- structcheck
- unused
- unparam
- nosnakecase
path: "_test.go"
- linters:
- errcheck # Error checking omitted for brevity.
- gosec
path: "example_"

- testifylint
- varnamelen
- wrapcheck
settings:
dupl:
threshold: 100
errcheck:
check-type-assertions: true
check-blank: true
gocyclo:
min-complexity: 20
cyclop:
max-complexity: 15
misspell:
locale: US
unparam:
check-exported: true
exclusions:
generated: lax
rules:
- linters:
- gosec
- dupl
- funlen
- goconst
- mnd
- noctx
- unparam
- unused
path: _test.go
- linters:
- errcheck
- gosec
path: example_
- linters:
- revive
text: 'unused-parameter: parameter'
paths:
- third_party$
- builtin$
- examples$
formatters:
enable:
- gci
- gofmt
- gofumpt
- goimports
exclusions:
generated: lax
paths:
- third_party$
- builtin$
- examples$
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#GOLANGCI_LINT_VERSION := "v1.51.1" # Optional configuration to pinpoint golangci-lint version.
#GOLANGCI_LINT_VERSION := "v2.5.0" # Optional configuration to pinpoint golangci-lint version.

# The head of Makefile determines location of dev-go to include standard targets.
GO ?= go
Expand Down
2 changes: 1 addition & 1 deletion cmd/jsoncompact/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

input = flag.Arg(0)
if input == "" {
_, _ = fmt.Fprintln(flag.CommandLine.Output(), "Missing input path argument, use `-` for stdin.")
_, _ = fmt.Fprintln(flag.CommandLine.Output(), "Missing input path argument, use `-` for stdin.") //nolint

Check notice on line 43 in cmd/jsoncompact/main.go

View workflow job for this annotation

GitHub Actions / test (stable)

3 statement(s) on lines 42:47 are not covered by tests.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: High

The blanket nolint directive suppresses all linter warnings without specifying which linter or warning is being ignored. This makes the code harder to maintain and could hide legitimate issues. Specific linters should be named in the nolint directive to ensure only intended warnings are suppressed.

Suggested change
_, _ = fmt.Fprintln(flag.CommandLine.Output(), "Missing input path argument, use `-` for stdin.") //nolint
_, _ = fmt.Fprintln(flag.CommandLine.Output(), "Missing input path argument, use `-` for stdin.") //nolint:errcheck

Evidence: method:fmt.Fprintln, search:nolint

flag.Usage()

return
Expand Down
2 changes: 1 addition & 1 deletion equal.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func (c Comparer) filterExpected(expected []byte) ([]byte, error) {
return nil, fmt.Errorf("failed to marshal var %s: %v", k, err) // Not wrapping to support go1.12.
}

expected = bytes.Replace(expected, []byte(`"`+k+`"`), j, -1) //nolint:gocritic // To support go1.11.
expected = bytes.Replace(expected, []byte(`"`+k+`"`), j, -1) //nolint:gocritic,staticcheck // To support go1.11.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 | Confidence: High

The added staticcheck nolint directive suppresses a legitimate lint warning about using deprecated functionality (bytes.Replace with -1 count). Since the project now uses Go 1.17+ (per go.mod), this should be updated to use bytes.ReplaceAll() for better clarity and forward compatibility, as the current approach relies on deprecated behavior.

Suggested change
expected = bytes.Replace(expected, []byte(`"`+k+`"`), j, -1) //nolint:gocritic,staticcheck // To support go1.11.
expected = bytes.ReplaceAll(expected, []byte(`"`+k+`"`), j)

Evidence: method:bytes.Replace, search:gocritic,staticcheck

}
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/swaggest/assertjson
go 1.17

require (
github.com/bool64/dev v0.2.29
github.com/bool64/dev v0.2.43
github.com/bool64/shared v0.1.5
github.com/iancoleman/orderedmap v0.3.0
github.com/stretchr/testify v1.4.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
github.com/bool64/dev v0.2.17/go.mod h1:iJbh1y/HkunEPhgebWRNcs8wfGq7sjvJ6W5iabL8ACg=
github.com/bool64/dev v0.2.29 h1:x+syGyh+0eWtOzQ1ItvLzOGIWyNWnyjXpHIcpF2HvL4=
github.com/bool64/dev v0.2.29/go.mod h1:iJbh1y/HkunEPhgebWRNcs8wfGq7sjvJ6W5iabL8ACg=
github.com/bool64/dev v0.2.43 h1:yQ7qiZVef6WtCl2vDYU0Y+qSq+0aBrQzY8KXkklk9cQ=
github.com/bool64/dev v0.2.43/go.mod h1:iJbh1y/HkunEPhgebWRNcs8wfGq7sjvJ6W5iabL8ACg=
github.com/bool64/shared v0.1.5 h1:fp3eUhBsrSjNCQPcSdQqZxxh9bBwrYiZ+zOKFkM0/2E=
github.com/bool64/shared v0.1.5/go.mod h1:081yz68YC9jeFB3+Bbmno2RFWvGKv1lPKkMP6MHJlPs=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down
Loading