From 5efb28a5052c02b929efbf4d481fe3daaea79dd0 Mon Sep 17 00:00:00 2001 From: ApolloF <98666317+ApolloF@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:52:58 +0100 Subject: [PATCH 1/2] Add golangci-lint config and fix its findings Standard linters (govet, ineffassign, staticcheck, unused) with all staticcheck checks except ST1005 (error strings are shown to users as sentences) and ST1022 (doc comments may read "Name, when set, ..."). errcheck stays off until its ~50 unchecked Close/Remove calls are fixed. Findings fixed: eight De Morgan simplifications (QF1001), a package comment for internal/app, the ModSpec/HoldNewMod doc comments untangled, two dead helpers in internal/steam removed and a test helper moved to the personal-build test file that uses it. --- .golangci.yml | 21 +++++++++++++++++++++ internal/accounts/ops.go | 6 +++--- internal/app/app.go | 21 ++++++++++++--------- internal/backup/backup.go | 2 +- internal/meta/meta.go | 8 ++++---- internal/meta/modsettings_test.go | 16 ++++++++++++++++ internal/meta/owners.go | 4 ++-- internal/meta/owners_test.go | 16 ---------------- internal/steam/steam.go | 19 ------------------- internal/store/store.go | 2 +- 10 files changed, 60 insertions(+), 55 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..c02cf9a --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,21 @@ +version: "2" + +linters: + # The standard set: govet, ineffassign, staticcheck, unused (errcheck is in + # it too, see below). CI pins the golangci-lint version in + # .github/workflows/build.yml; run the same one locally. + default: standard + disable: + # Not enabled yet: it finds about 50 unchecked Close/Remove calls that need + # fixing one by one before it can gate CI. + - errcheck + settings: + staticcheck: + checks: + - all + # Error strings are shown to the user as they are, so they are + # sentences: capitalized and with a full stop. + - -ST1005 + # Doc comments on vars and consts here often read "Name, when set, ..." + # or "Name: ..."; the name still comes first, which is what matters. + - -ST1022 diff --git a/internal/accounts/ops.go b/internal/accounts/ops.go index b1a5cde..aea3639 100644 --- a/internal/accounts/ops.go +++ b/internal/accounts/ops.go @@ -949,7 +949,7 @@ func swap(ctx context.Context, env Env, j *Journal, m *Move) error { } } if m.Step == swapHold { - if !(exists(m.Hold) && !exists(m.Live)) { + if !exists(m.Hold) || exists(m.Live) { if err := os.Rename(m.Live, m.Hold); err != nil { return back(fmt.Errorf("could not move the current saves (is the game running?): %w", err)) } @@ -975,7 +975,7 @@ func swap(ctx context.Context, env Env, j *Journal, m *Move) error { return j.failAt(err) } } - if !(exists(m.Live) && !exists(m.In)) { + if !exists(m.Live) || exists(m.In) { if err := moveDir(m.In, m.Live); err != nil { return back(fmt.Errorf("could not bring in the other saves: %w", err)) } @@ -995,7 +995,7 @@ func swap(ctx context.Context, env Env, j *Journal, m *Move) error { } } if m.Step == swapOut { - if !(exists(m.Out) && !exists(m.Hold)) { + if !exists(m.Out) || exists(m.Hold) { if exists(m.Out) { if _, err := retire(m.Out); err != nil { return j.failAt(err) diff --git a/internal/app/app.go b/internal/app/app.go index 229ae7e..7090472 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -1,3 +1,6 @@ +// Package app is the desktop app: the Wails App whose methods the frontend +// calls, the tray, the background and scheduled runs, the local launcher API +// and the glue that wires the internal packages to the UI. package app import ( @@ -201,13 +204,13 @@ type Overview struct { Target string `json:"target"` LastBackup *store.BackupRun `json:"lastBackup"` // the last attempt, failed or not // LastSuccess is when a backup last finished without errors; zero if never. - LastSuccess time.Time `json:"lastSuccess"` - BackingUp bool `json:"backingUp"` - Gaming bool `json:"gaming"` // automatic backups are holding off right now - Paused bool `json:"paused"` // syncing and automatic backups are paused (until settings.pausedUntil) - Settings store.Settings `json:"settings"` - Version string `json:"version"` - Update *UpdateInfo `json:"update"` // a newer release, if one is out + LastSuccess time.Time `json:"lastSuccess"` + BackingUp bool `json:"backingUp"` + Gaming bool `json:"gaming"` // automatic backups are holding off right now + Paused bool `json:"paused"` // syncing and automatic backups are paused (until settings.pausedUntil) + Settings store.Settings `json:"settings"` + Version string `json:"version"` + Update *UpdateInfo `json:"update"` // a newer release, if one is out // VersionGaps: linked PCs running an older or newer Syncer than this one. VersionGaps []VersionGap `json:"versionGaps"` // Waiting: folders from other PCs that wait for the user's OK. @@ -490,7 +493,7 @@ func normalizeID(id string) (string, error) { return "", errors.New("that doesn't look like a device ID (expected 56 letters/digits)") } for _, ch := range raw { - if !(ch >= 'A' && ch <= 'Z' || ch >= '2' && ch <= '7') { + if (ch < 'A' || ch > 'Z') && (ch < '2' || ch > '7') { return "", errors.New("device IDs only contain letters A–Z and digits 2–7") } } @@ -909,7 +912,7 @@ func applySettingsInput(s *store.Settings, in, old store.Settings) { // modsAutoOn reports whether adding mod folders automatically just started // (or its size limit changed), so it should run now. func modsAutoOn(old, s store.Settings) bool { - return features.Mods && s.FindMods && s.AutoAddMods && (!(old.FindMods && old.AutoAddMods) || s.ModsMaxGB != old.ModsMaxGB) + return features.Mods && s.FindMods && s.AutoAddMods && (!old.FindMods || !old.AutoAddMods || s.ModsMaxGB != old.ModsMaxGB) } // publishSettings shares a change of the mod options with the other PCs now. diff --git a/internal/backup/backup.go b/internal/backup/backup.go index da63bee..0e05b4c 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -275,7 +275,7 @@ func mirror(ctx context.Context, f Folder, opts Options, stamp string, p *Progre } if m.Ignored(rel) || sc.skip(rel, d.IsDir()) { // A negation can reach into an ignored folder. - if d.IsDir() && !(m.HasNegation() && !sc.on) { + if d.IsDir() && (!m.HasNegation() || sc.on) { return filepath.SkipDir } return nil diff --git a/internal/meta/meta.go b/internal/meta/meta.go index 1d3f44d..7f7d236 100644 --- a/internal/meta/meta.go +++ b/internal/meta/meta.go @@ -1062,15 +1062,15 @@ func ModVersionsDir(id string) string { return filepath.Join(paths.Root(paths.Local), "Syncer", "mod-versions", id) } +// HoldNewMod, when set, says whether a new mod folder must start paused +// (its mod manager is open) and remembers to resume it. +var HoldNewMod func(id string) bool + // ModSpec is how a mod folder of kind is added. A staging folder uses // Vortex's own marker as Syncthing's, so no .stfolder appears among the mods // and the folder stops, rather than looking emptied, if its drive is // missing. Replaced files are kept for a week. A PC receiving deployed mods // only takes them from the source, and stays paused between audited updates. -// HoldNewMod, when set, says whether a new mod folder must start paused -// (its mod manager is open) and remembers to resume it. -var HoldNewMod func(id string) bool - func ModSpec(id, label, path, kind string, source bool) FolderSpec { v := syncthing.StaggeredVersioning() v["params"] = map[string]string{"maxAge": "604800", "cleanInterval": "3600"} diff --git a/internal/meta/modsettings_test.go b/internal/meta/modsettings_test.go index aedbc07..7b5448b 100644 --- a/internal/meta/modsettings_test.go +++ b/internal/meta/modsettings_test.go @@ -79,3 +79,19 @@ func TestModSettingsNewestWins(t *testing.T) { t.Fatal("mod options stayed on without finding mods") } } + +// writtenBySelf records PC device's file, as it is now, as last written by +// that PC, as a reconcile finds out from Syncthing. +func writtenBySelf(t *testing.T, device string) { + t.Helper() + sum, ok := fileSum(device) + if !ok { + t.Fatalf("no file of %s", device) + } + writers.Lock() + defer writers.Unlock() + if writers.m == nil { + writers.m = map[string]writerCheck{} + } + writers.m[device] = writerCheck{sum, true} +} diff --git a/internal/meta/owners.go b/internal/meta/owners.go index fbfad54..2e8860b 100644 --- a/internal/meta/owners.go +++ b/internal/meta/owners.go @@ -322,7 +322,7 @@ func noIntroducers(ctx context.Context, c *syncthing.Client, ds []syncthing.Devi for _, d := range ds { id := d.DeviceID _, introduced := st.Introduced[id] - if !d.Introducer || !(pcs[id] || d.IntroducedBy != "" || st.JoinRequests[id] || st.IgnoredPCs[id] || introduced || ours[id]) { + if !d.Introducer || (!pcs[id] && d.IntroducedBy == "" && !st.JoinRequests[id] && !st.IgnoredPCs[id] && !introduced && !ours[id]) { continue } if err := c.PatchDevice(ctx, id, map[string]any{"introducer": false}); err != nil { @@ -465,7 +465,7 @@ func validDeviceID(id string) bool { if ch != '-' { return false } - } else if !(ch >= 'A' && ch <= 'Z' || ch >= '2' && ch <= '7') { + } else if (ch < 'A' || ch > 'Z') && (ch < '2' || ch > '7') { return false } } diff --git a/internal/meta/owners_test.go b/internal/meta/owners_test.go index 7c8df12..8f7d7d7 100644 --- a/internal/meta/owners_test.go +++ b/internal/meta/owners_test.go @@ -177,22 +177,6 @@ func TestWriteVortexListsLeavesAnUnchangedFileAlone(t *testing.T) { } } -// writtenBySelf records PC device's file, as it is now, as last written by -// that PC, as a reconcile finds out from Syncthing. -func writtenBySelf(t *testing.T, device string) { - t.Helper() - sum, ok := fileSum(device) - if !ok { - t.Fatalf("no file of %s", device) - } - writers.Lock() - defer writers.Unlock() - if writers.m == nil { - writers.m = map[string]writerCheck{} - } - writers.m[device] = writerCheck{sum, true} -} - // fakeSyncthing is a Syncthing that, besides Syncer's PCs, syncs a notes // vault with a server and a laptop, as one paired before Syncer was fixed: // every folder offered to every device. diff --git a/internal/steam/steam.go b/internal/steam/steam.go index 014faa7..c517ee5 100644 --- a/internal/steam/steam.go +++ b/internal/steam/steam.go @@ -421,22 +421,3 @@ func isFile(p string) bool { return err == nil && fi.Mode().IsRegular() } -func clean(p string) string { - p = strings.TrimSpace(p) - if p == "" { - return "" - } - return filepath.Clean(filepath.FromSlash(p)) -} - -// within reports whether child is parent or lies below it (case-insensitive). -func within(parent, child string) bool { - parent, child = strings.ToLower(filepath.Clean(parent)), strings.ToLower(filepath.Clean(child)) - if parent == child { - return true - } - if !strings.HasSuffix(parent, `\`) && !strings.HasSuffix(parent, "/") { - parent += string(filepath.Separator) - } - return strings.HasPrefix(child, parent) -} diff --git a/internal/store/store.go b/internal/store/store.go index 41b4562..91447c7 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -648,7 +648,7 @@ var rename = os.Rename func renameRetry(from, to string) error { var err error for i, wait := 0, 10*time.Millisecond; i < 6; i, wait = i+1, wait*2 { - if err = rename(from, to); err == nil || !(errors.Is(err, windows.ERROR_ACCESS_DENIED) || errors.Is(err, windows.ERROR_SHARING_VIOLATION)) { + if err = rename(from, to); err == nil || (!errors.Is(err, windows.ERROR_ACCESS_DENIED) && !errors.Is(err, windows.ERROR_SHARING_VIOLATION)) { return err } time.Sleep(wait) From d6f0e98ea2b2f5be570ed29083e4df1bccb9b8f3 Mon Sep 17 00:00:00 2001 From: ApolloF <98666317+ApolloF@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:53:01 +0100 Subject: [PATCH 2/2] Run vet, lint and the race detector in CI go vet and golangci-lint v2.15.0 for both builds, then go test -race -short for both builds next to the full test runs. The windows job still needs no secret, so Dependabot PRs keep passing; signing stays in the tag-only release job. CONTRIBUTING.md lists the new tools. --- .github/workflows/build.yml | 25 +++++++++++++++++++++++++ CONTRIBUTING.md | 4 +++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2dc611e..8d2618f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -48,6 +48,23 @@ jobs: working-directory: frontend run: npm test + - name: Vet + # Both builds (see internal/features): the personal tag compiles the + # other half of a few files. + run: | + go vet ./... + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + go vet -tags personal ./... + + - name: Lint + uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 + with: + version: v2.15.0 # the version to run locally too (CONTRIBUTING.md) + + - name: Lint (personal build) + # The action above put golangci-lint on PATH. + run: golangci-lint run --build-tags personal ./... + - name: Test run: go test ./... @@ -55,6 +72,14 @@ jobs: # The personal build is tested too (see internal/features). run: go test -tags personal ./... + - name: Test with the race detector + # The sync code is concurrent; -short keeps the randomized crash test + # in internal/accounts small (the full run is above). + run: | + go test -race -short ./... + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + go test -race -short -tags personal ./... + - name: Version # Release builds carry their tag (shown in the app, used by the update # check) and put it in the exe's file properties; others are "dev". diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 503127e..86b30ea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,9 @@ Thanks for helping. Syncer is a one-person project, so small, focused changes ge ```bash go vet ./... && go vet -tags personal ./... + golangci-lint run ./... && golangci-lint run --build-tags personal ./... go test ./... && go test -tags personal ./... + go test -race -short ./... && go test -race -short -tags personal ./... cd frontend && npm ci && npm run check && npx vitest run cd .. && wails build ``` @@ -26,7 +28,7 @@ Thanks for helping. Syncer is a one-person project, so small, focused changes ge ## Build and layout -Requirements: Go 1.26+, Node 22+, [Wails v2](https://wails.io) (`go install github.com/wailsapp/wails/v2/cmd/wails@v2.16.0`). +Requirements: Go 1.26+, Node 22+, [Wails v2](https://wails.io) (`go install github.com/wailsapp/wails/v2/cmd/wails@v2.16.0`), [golangci-lint](https://golangci-lint.run) at the version pinned in `.github/workflows/build.yml` (`go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.15.0`), and a C compiler such as mingw gcc for `go test -race`. ```bash wails dev # live-reloading app