goimports and gofmt fixes are not applied if typecheck-requiring linter is enabled on broken source files

[denik]

Welcome

• Yes, I'm using a binary release within 2 latest releases. Only such installations are supported.
• Yes, I've searched similar issues on GitHub and didn't find any.
• Yes, I've read the typecheck section of the FAQ.
• Yes, I've tried with the standalone linter if available (e.g., gocritic, go vet, etc.).
• I agree to follow this project's Code of Conduct

Description of the problem

Some frequently used linters, like goimports and gofmt do not require the file to be compilation error-free. They can diagnose and fix issues even in presence of errors, This works both standalone and from golangci-lint:

% cat main2.go
package main

import (
        "context"
        "os"
)

func main() {
        ctx          :=         context.Background()
        return 0
}

% golangci-lint --no-config --disable-all --enable gofumpt,goimports,gofmt run main2.go
main2.go:9: File is not `gofmt`-ed with `-s` (gofmt)
        ctx          :=         context.Background()
main2.go:5: File is not `goimports`-ed (goimports)
        "os"

% golangci-lint --no-config --disable-all --enable gofumpt,goimports,gofmt run --fix main2.go

% cat main2.go
package main

import (
        "context"
)

func main() {
        ctx := context.Background()

However, adding another linter into the mix, triggers typecheck errors AND disables previously working linters. I understand typecheck stage is required for the new linter but it is not required for all linters.

% cat main2.go
package main

import (
        "context"
        "os"
)

func main() {
        ctx          :=         context.Background()
        return 0
}
% golangci-lint --no-config --disable-all --enable gofumpt,goimports,gofmt,bodyclose run --fix main2.go
main2.go:10:9: too many return values
        have (number)
        want () (typecheck)
        return 0
               ^
main2.go:9:2: declared and not used: ctx (typecheck)
        ctx          :=         context.Background()
        ^
main2.go:5:2: "os" imported and not used (typecheck)
        "os"
        ^
% cat main2.go
package main

import (
        "context"
        "os"
)

func main() {
        ctx          :=         context.Background()
        return 0
}

Note, this especially becomes a problem when working with some kind of AI that edits your source code but does so with errors. This would also be a problem for people who rely on golangci-lint to fix files on save in their IDE, because gopls does apply the fixes as expected.

Getting the best-effort partial fix from golangci-lint would be very helpful.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.62.2 built with go1.23.3 from 89476e7 on 2024-11-25T14:12:23Z

Configuration

n/a

Go environment

n/a

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
~/work/cli % golangci-lint --no-config --disable-all --enable gofumpt,goimports,gofmt,bodyclose run -v main2.go
INFO golangci-lint has version 1.62.2 built with go1.23.3 from 89476e7 on 2024-11-25T14:12:23Z
INFO [lintersdb] Active 4 linters: [bodyclose gofmt gofumpt goimports]
INFO [loader] Go packages loading at mode 8767 (types_sizes|deps|exports_file|files|compiled_files|imports|name) took 92.468917ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 98.25µs
INFO [linters_context/goanalysis] analyzers took 108.833µs with top 10 stages: buildssa: 22.875µs, gofumpt: 21.25µs, typecheck: 17.625µs, goimports: 17.541µs, bodyclose: 17.083µs, gofmt: 12.459µs
INFO [runner] Issues before processing: 18, after processing: 3
INFO [runner] Processors filtering stat (in/out): sort_results: 3/3, cgo: 18/18, nolint: 18/18, diff: 3/3, max_from_linter: 3/3, path_shortener: 3/3, fixer: 3/3, skip_files: 18/18, skip_dirs: 18/18, uniq_by_line: 18/3, exclude: 18/18, max_per_file_from_linter: 3/3, max_same_issues: 3/3, severity-rules: 3/3, path_prefixer: 3/3, filename_unadjuster: 18/18, invalid_issue: 18/18, identifier_marker: 18/18, source_code: 3/3, path_prettifier: 18/18, autogenerated_exclude: 18/18, exclude-rules: 18/18
INFO [runner] processing took 171.542µs with stages: identifier_marker: 100.458µs, source_code: 32.833µs, path_prettifier: 23.75µs, invalid_issue: 4.709µs, autogenerated_exclude: 2.042µs, max_same_issues: 1.249µs, uniq_by_line: 1.125µs, nolint: 833ns, filename_unadjuster: 750ns, path_shortener: 625ns, cgo: 542ns, exclude-rules: 541ns, skip_dirs: 500ns, max_from_linter: 459ns, max_per_file_from_linter: 292ns, skip_files: 209ns, fixer: 167ns, exclude: 166ns, diff: 125ns, severity-rules: 84ns, path_prefixer: 42ns, sort_results: 41ns
INFO [runner] linters took 25.5515ms with stages: goanalysis_metalinter: 25.341458ms
main2.go:10:9: too many return values
        have (number)
        want () (typecheck)
        return 0
               ^
main2.go:9:2: declared and not used: ctx (typecheck)
        ctx          :=         context.Background()
        ^
main2.go:5:2: "os" imported and not used (typecheck)
        "os"
        ^
INFO File cache stats: 1 entries of total size 115B
INFO Memory: 3 samples, avg is 32.0MB, max is 40.6MB
INFO Execution took 129.440208ms

A minimal reproducible example or link to a public repository

package main

import (
        "context"
        "os"
)

func main() {
        ctx          :=         context.Background()
        return 0
}
// golangci-lint --no-config --disable-all --enable gofumpt,goimports,gofmt,bodyclose run --fix main2.go

Validation

• Yes, I've included all information above (version, config, etc.).

Supporter

• I am a sponsor/backer through GitHub or OpenCollective

[boring-cyborg]

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

[ldez]

Hello,

the linters don't run independently.

If you want to just run a formatter:

golangci-lint run --enable-only="goimports" --fix

[denik]

Thanks @ldez

Is it desired behavior? or is this something that is too complex to implement / not worth it?

[ldez]

Is it desired behavior?

Yes, because golangci-lint runs all the linters "at the same time": the requirements of one run is a kind of "sum" of all the linters requirements.

or is this something that is too complex to implement / not worth it?

TLDR: yes too complex to implement and isn't worth it.

The formatters are something different than linters but currently, they are handled like linters.
I made some changes to formatters but this will not change the way it works in this context.

I have some ideas about this topic, I will communicate about that at the beginning of the year but don't expect magical things.

[denik]

Got it -- and thank you for your work, great project!

[bombsimon]

This is a bit interesting since, as you wrote in your PR in lint.sh, running goimports can actually fix compilation issues if the issue is missing imports. However I don't think it's reasonable for golangci-lint to first fix this to potentially roll it back just to be able to maybe run more analysis.

I wonder if we could add either a shortcut for all formatters like --disable-all --enable-formatters or a separate step from run like golangci-lint format which just like gofmt and goimports with friends could accept the --fix flag to replicate -w.

I don't think something like golangci-lint format --fix && golangci-lint run --fix is an unreasonable workflow (other than not having gofmt and goimports on save in your editor/IDE when you write Go sounds a bit odd to me 😅)

[ldez]

@bombsimon just wait for a few days, I will open 5 issues 😉

They are ready but I want to open them after the 1st.