Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
1c51cb3
chore: migrate to golangci-lint v2
romshark May 15, 2025
9902bf1
fix: use v2 module
romshark May 15, 2025
9ee0372
fix: move deprecated flags to yaml config
romshark May 15, 2025
ccebfbf
refactor: remove embedded fields in selectors
romshark May 15, 2025
22c1a65
refactor: uncapitalize error messages
romshark May 15, 2025
2c009b0
refactor: simplify code using De Morgan's law
romshark May 15, 2025
fbefa22
refactor: remove underscore receivers
romshark May 15, 2025
be61460
refactor: use strings.ReplaceAll (QF1004)
romshark May 15, 2025
9f05f89
refactor: replace if-else with switch (QF1003)
romshark May 15, 2025
2f3f874
Merge branch 'master' into chore-golangci-lint-v2
romshark May 15, 2025
4d9c1c6
refactor: remove unused
romshark May 15, 2025
b1be505
refactor: punctuation in errors (ST1005)
romshark May 15, 2025
2bf1881
refactor: remove redundant imports (ST1019)
romshark May 15, 2025
66dc678
refactor: simplify code (QF1006)
romshark May 15, 2025
cdc0731
refactor: simplify code (ST1023)
romshark May 15, 2025
03ac089
refactor: simplify code (QF1009)
romshark May 15, 2025
4f7c56f
refactor: return error last (ST1008)
romshark May 15, 2025
e58573a
refactor: untyped constants (SA9004)
romshark May 15, 2025
df48429
refactor: replace deprecated method (SA1019)
romshark May 15, 2025
69e52dc
doc: ignore deprecated method in test (SA1019)
romshark May 15, 2025
4362e6b
refactor: replace deprecated method
romshark May 15, 2025
ddd8d4b
Merge branch 'master' into chore-golangci-lint-v2
romshark May 16, 2025
2d1f66d
fix: exlude SA1019 for deprecated grpc APIs
romshark May 23, 2025
33f780c
fix: ignore SA1019 for bazel.Runfile
romshark May 26, 2025
caac21e
fix: ignore SA1019 for periodic.Start
romshark May 26, 2025
a85d803
chore: Fix lines too long
romshark May 26, 2025
8ff3f00
fix: ignore SA1019 for elliptic.Marshal
romshark May 26, 2025
0d83fe7
fix: expect typed int in test
romshark May 26, 2025
9ec177d
Merge branch 'master' into chore-golangci-lint-v2
romshark May 26, 2025
779e2a4
chore: run make all
romshark May 26, 2025
71e0f19
chore: reconfigure staticcheck
romshark May 28, 2025
19f7502
refactor: Simplify boolean expression
romshark May 28, 2025
990ce59
Revert "refactor: remove embedded fields in selectors"
romshark May 28, 2025
735de53
fix: disable QF1008 in staticcheck
romshark May 28, 2025
06a1bcd
Merge branch 'master' into chore-golangci-lint-v2
romshark May 28, 2025
c2bba7d
docs: remove accidental tabulator in yaml
romshark May 28, 2025
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
111 changes: 111 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
version: "2"
run:
skip-dirs:
- ^doc/
timeout: 3m
linters:
default: none
enable:
- contextcheck
- copyloopvar
- errcheck
- forbidigo
- goheader
- govet
- ineffassign
- lll
- misspell
- rowserrcheck
- sqlclosecheck
- staticcheck
- unconvert
- unused
settings:
Copy link
Contributor

Choose a reason for hiding this comment

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

May be QF1001 can be disabled from staticcheck. so we do not have to always bend to a machine's opinion regarding the legibility of boolean expressions?

Viewed in some else's config file:

    staticcheck:
      # SAxxxx checks in https://staticcheck.dev/docs/configuration/options/#checks
      # Example (to disable some checks): [ "all", "-SA1000", "-SA1001"]
      # Default: ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022"]
      checks:
        - all
        # Poorly chosen identifier.
        # https://staticcheck.dev/docs/checks/#ST1003
        - -ST1003
        # The documentation of an exported function should start with the function's name.
        # https://staticcheck.dev/docs/checks/#ST1020
        - -ST1020
        # The documentation of an exported type should start with type's name.
        # https://staticcheck.dev/docs/checks/#ST1021
        - -ST1021
        # The documentation of an exported variable or constant should start with variable's name.
        # https://staticcheck.dev/docs/checks/#ST1022
        - -ST1022
        # Apply De Morgan's law.
        # https://staticcheck.dev/docs/checks/#QF1001
        - -QF1001

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll keep the changes to the existing expressions since some of them are both IMHO and in @katyatitkova opinion more readable now (less unnecessary negation) but I'll disable QF1001

staticcheck:
checks:
- all
- "-QF1001" # disable "Apply De Morgan's law".
# The use of Go ideomatic identifiers is a recommendation, not a law.
- "-ST1003" # disable "Poorly chosen identifier".
- "-QF1008" # disable "Omit embedded fields from selector expression".
errcheck:
exclude-functions:
- (*github.com/spf13/cobra.Command).MarkFlagRequired
forbidigo:
forbid:
- pattern: ([iI][fF][iI]d)|([iI]F[iI][dD])|([iI][fF]i[dD])
msg: spell interface ID as ifID / IfID
- pattern: (?i)interfaceID
msg: spell interface ID as ifID / IfID
- pattern: Trc
msg: spell trust root certificate as trc / TRC
goheader:
values:
regexp:
copyright-lines: |-
(Copyright 20[0-9][0-9] .*)(
Copyright 20[0-9][0-9] .*)*
template: |-
{{copyright-lines}}

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
lll:
line-length: 100
tab-width: 4
exclusions:
generated: lax
presets:
- comments
- common-false-positives
- legacy
- std-error-handling
rules:
- linters:
- lll
source: http[s]://\S{80,}$
- linters:
- lll
source: '`(yaml|toml|json):"[^`]*`$'
- linters:
- goheader
path: pkg/private/util/duration.go
- linters:
- errcheck
- goheader
path: pkg/private/serrors/stack.go
- linters:
- errcheck
- goheader
- lll
path: scion-pki/certs/(certinfo|certformat).go
- linters:
- goheader
path: pkg/scrypto/cms
paths:
- third_party$
- builtin$
- examples$
formatters:
enable:
- gofmt
- goimports
settings:
goimports:
local-prefixes:
- github.com/scionproto/scion
exclusions:
generated: lax
paths:
- third_party$
- builtin$
- examples$
79 changes: 0 additions & 79 deletions .golangcilint.yml

This file was deleted.

2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ GO_BUILD_TAGS_ARG=$(shell bazel info --ui_event_filters=-stdout,-stderr --announ

lint-go-golangci:
$(info ==> $@)
@tools/quiet bazel run --config=quiet @rules_go//go -- run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.8 run --config="${PWD}/.golangcilint.yml" --timeout=3m $(GO_BUILD_TAGS_ARG),lint --exclude-dirs doc ./...
@tools/quiet bazel run --config=quiet @rules_go//go -- run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --config="${PWD}/.golangci.yml" $(GO_BUILD_TAGS_ARG),lint ./...

lint-go-semgrep:
$(info ==> $@)
Expand Down
2 changes: 1 addition & 1 deletion acceptance/router_benchmark/brload/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (c *caseChoice) String() string {
func (c *caseChoice) Set(v string) error {
_, ok := allCases[v]
if !ok {
return errors.New("No such case")
return errors.New("no such case")
}
*c = caseChoice(v)
return nil
Expand Down
3 changes: 3 additions & 0 deletions acceptance/topo_cs_reload/reload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,13 @@ func setupTest(t *testing.T) testState {
s := testState{
extraEnv: []string{"TOPO_CS_RELOAD_CONFIG_DIR=" + tmpDir},
}
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4775).
scionPKI, err := bazel.Runfile(*scionPKILocation)
require.NoError(t, err)
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4775).
cryptoLib, err := bazel.Runfile(*cryptoLibLocation)
require.NoError(t, err)
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4775).
topoFile, err := bazel.Runfile(*topoLocation)
require.NoError(t, err)
s.mustExec(t, *genCryptoLocation, scionPKI, "crypto.tar", topoFile, cryptoLib)
Expand Down
1 change: 1 addition & 0 deletions control/beaconing/extender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func TestDefaultExtenderExtend(t *testing.T) {
// Setup interfaces with active parent, child and one peer interface.
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
for _, peer := range tc.peers {
//nolint:staticcheck // SA1019: Activate is fine for testing.
intfs.Get(peer).Activate(peerRemoteIfs[peer])
}
ext := &beaconing.DefaultExtender{
Expand Down
2 changes: 1 addition & 1 deletion control/beaconing/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (r *WriteScheduler) Run(ctx context.Context) {
}

func (r *WriteScheduler) run(ctx context.Context) error {
if !(r.Tick.Overdue(r.lastWrite) || r.Tick.Passed()) {
if !r.Tick.Overdue(r.lastWrite) && !r.Tick.Passed() {
return nil
}
segments, err := r.Provider.SegmentsToRegister(ctx, r.Type)
Expand Down
4 changes: 4 additions & 0 deletions control/cmd/control/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,8 @@ func realMain(ctx context.Context) error {
},
}
// Periodically check the connection to the CA backend
// SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
//nolint:staticcheck
caHealthChecker := periodic.Start(
periodic.Func{
TaskName: "ca healthcheck",
Expand Down Expand Up @@ -519,6 +521,7 @@ func realMain(ctx context.Context) error {
}

// Frequently regenerate signers to catch problems, and update the metrics.
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
periodic.Start(
periodic.Func{
TaskName: "signer generator",
Expand All @@ -537,6 +540,7 @@ func realMain(ctx context.Context) error {
5*time.Second,
)

//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
trcRunner := periodic.Start(
periodic.Func{
TaskName: "trc expiration updater",
Expand Down
4 changes: 2 additions & 2 deletions control/drkey/grpc/drkey_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,8 @@ func validateHostHostReq(meta drkey.HostHostMeta, localIA addr.IA, peerAddr net.
srcHost := net.ParseIP(meta.SrcHost)
dstHost := net.ParseIP(meta.DstHost)

if !((meta.SrcIA.Equal(localIA) && hostAddr.Equal(srcHost)) ||
(meta.DstIA.Equal(localIA) && hostAddr.Equal(dstHost))) {
if (!meta.SrcIA.Equal(localIA) || !hostAddr.Equal(srcHost)) &&
(!meta.DstIA.Equal(localIA) || !hostAddr.Equal(dstHost)) {
return serrors.New(
"invalid request",
"local_isd_as", localIA,
Expand Down
3 changes: 2 additions & 1 deletion control/drkey/grpc/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ func genCrypto(t testing.TB) string {
"--isd-dir",
"--as-validity", "1y",
})
cmd.SetOutput(&buf)
cmd.SetOut(&buf)
cmd.SetErr(&buf)
err := cmd.Execute()
require.NoError(t, err, buf.String())

Expand Down
6 changes: 6 additions & 0 deletions control/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ func (t *TasksConfig) Originator() *periodic.Runner {
if t.Metrics != nil {
s.Originated = metrics.NewPromCounter(t.Metrics.BeaconingOriginatedTotal)
}
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
return periodic.Start(s, 500*time.Millisecond, t.OriginationInterval)
}

Expand All @@ -118,6 +119,7 @@ func (t *TasksConfig) Propagator() *periodic.Runner {
p.Propagated = metrics.NewPromCounter(t.Metrics.BeaconingPropagatedTotal)
p.InternalErrors = metrics.NewPromCounter(t.Metrics.BeaconingPropagatorInternalErrorsTotal)
}
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
return periodic.Start(p, 500*time.Millisecond, t.PropagationInterval)
}

Expand Down Expand Up @@ -199,6 +201,7 @@ func (t *TasksConfig) segmentWriter(segType seg.Type,
// as we can until we succeed. After succeeding, the task does nothing
// until the end of the interval. The interval itself is used as a
// timeout. If we fail slow we give up at the end of the cycle.
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
return periodic.Start(r, 500*time.Millisecond, t.RegistrationInterval)
}

Expand Down Expand Up @@ -236,6 +239,7 @@ func (t *TasksConfig) DRKeyCleaners() []*periodic.Runner {
cleaners := t.DRKeyEngine.CreateStorageCleaners()
cleanerTasks := make([]*periodic.Runner, len(cleaners))
for i, cleaner := range cleaners {
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
cleanerTasks[i] = periodic.Start(cleaner, cleanerPeriod, cleanerPeriod)
}
return cleanerTasks
Expand All @@ -246,6 +250,7 @@ func (t *TasksConfig) DRKeyPrefetcher() *periodic.Runner {
return nil
}
prefetchPeriod := t.DRKeyEpochInterval / 2
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
return periodic.Start(
&drkey.Prefetcher{
LocalIA: t.IA,
Expand Down Expand Up @@ -276,6 +281,7 @@ func StartTasks(cfg TasksConfig) (*Tasks, error) {
Originator: cfg.Originator(),
Propagator: cfg.Propagator(),
Registrars: cfg.SegmentWriters(),
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
PathCleaner: periodic.Start(
periodic.Func{
Task: func(ctx context.Context) {
Expand Down
3 changes: 2 additions & 1 deletion control/trust/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ func genCrypto(t *testing.T) string {
"--isd-dir",
"--as-validity", "1y",
})
cmd.SetOutput(&buf)
cmd.SetOut(&buf)
cmd.SetErr(&buf)
err := cmd.Execute()
require.NoError(t, err, buf.String())

Expand Down
5 changes: 5 additions & 0 deletions daemon/cmd/daemon/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,11 @@ func realMain(ctx context.Context) error {
})
defer pathDB.Close()
defer revCache.Close()
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
cleaner := periodic.Start(pathdb.NewCleaner(pathDB, "sd_segments"),
300*time.Second, 295*time.Second)
defer cleaner.Stop()
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
rcCleaner := periodic.Start(revcache.NewCleaner(revCache, "sd_revocation"),
10*time.Second, 10*time.Second)
defer rcCleaner.Stop()
Expand Down Expand Up @@ -169,6 +171,7 @@ func realMain(ctx context.Context) error {
Dir: filepath.Join(globalCfg.General.ConfigDir, "certs"),
DB: trustDB,
}
//nolint:staticcheck // SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
trcLoaderTask := periodic.Start(periodic.Func{
Task: func(ctx context.Context) {
res, err := trcLoader.Load(ctx)
Expand Down Expand Up @@ -222,6 +225,8 @@ func realMain(ctx context.Context) error {
}
cleaners := drkeyClientEngine.CreateStorageCleaners()
for _, cleaner := range cleaners {
// SA1019: fix later (https://github.com/scionproto/scion/issues/4776).
//nolint:staticcheck
cleaner_task := periodic.Start(cleaner,
5*time.Minute, 5*time.Minute)
defer cleaner_task.Stop()
Expand Down
Loading
Loading