polyfloyd / go-errorlint

A source code linter that can be used to find code that will cause problems with Go's error wrapping scheme
MIT License
248 stars 19 forks source link

Consider Minimum Go Version in go.mod? #37

Closed tri-adam closed 1 year ago

tri-adam commented 1 year ago

Thank you for the great linter!

I have an issue related to multiple %w verbs that is similar to but different than #36. When the minimum version specified in go.mod (ref) is <1.20, the linter will give a non-wrapping format verb for fmt.Errorf. Use '%w' to format errors recommendation when encountering code such as the following:

e1 := errors.New("a")
e2 := errors.New("b")
e3 := fmt.Errorf("%v: %w", e1, e2)

Following the recommendation, one would replace "%v: %w" with "%w: %w", which works fine with Go 1.20, but is incorrect with Go <1.20. For an example, with this code in the playground:

func main() {
    e1 := errors.New("a")
    e2 := errors.New("b")
    e3 := fmt.Errorf("%w: %w", e1, e2)
    fmt.Println(e3)
}

With Go 1.19, the modified code does run but does not behave as expected, outputting:

a: %!w(*errors.errorString=&{b})

With Go 1.20, it behaves as expected:

a: b

I'm wondering if go-errorlint could/should consider the minimum Go version specified in go.mod before making recommendations to wrap multiple errors?

polyfloyd commented 1 year ago

Hi!

The minimum Go version in the go.mod file only applies to dependencies that are used in the project being compiled. This program does not require features from Go newer than 1.13, even though it does look for features introduced in later Go versions.

While I do like the idea of using go.mod to enable/disable functionality, I can not spot a way to retrieve this info from the analyzer library that errorlint uses. So having a flag or reserved syntax as proposed in #36 are the next best options I think.

tri-adam commented 1 year ago

Hi @polyfloyd , thanks for the quick response.

While I do like the idea of using go.mod to enable/disable functionality, I can not spot a way to retrieve this info from the analyzer library that errorlint uses. So having a flag or reserved syntax as proposed in #36 are the next best options I think.

That makes sense. In the absence of a way to retrieve that info, a flag or reserved syntax seems like a good option.

Thanks again, really appreciate you work!

polyfloyd commented 1 year ago

I have implemented a solution, see my last comment on #36