Hi, > Am 19.08.2025 um 06:57 schrieb Axel Wagner <axel.wagner...@googlemail.com>: > > Hi, > > On Fri, 15 Aug 2025 at 13:10, Oliver Eikemeier <eikeme...@fillmore-labs.com > <mailto:eikeme...@fillmore-labs.com>> wrote: >> Static Analysis for Go Error Type Consistency: errortype Linter >> >> I've been investigating a class of subtle bugs in Go error handling related >> to pointer vs. value semantics with errors.As, and developed a static >> analysis tool to detect these issues. I'm seeking feedback from the >> community on both the approach and the tool's effectiveness. >> >> Problem Statement >> >> Consider this code attempting to handle AES key size errors: >> >> key := []byte("My kung fu is better than yours") >> _, err := aes.NewCipher(key) >> >> var kse *aes.KeySizeError >> if errors.As(err, &kse) { >> fmt.Printf("AES keys must be 16, 24 or 32 bytes long, got %d >> bytes.\n", kse) >> } else if err != nil { >> fmt.Println(err) >> } >> The bug: aes.NewCipher returns aes.KeySizeError as a value, but the code >> checks for a pointer to *aes.KeySizeError. This type mismatch causes >> errors.As to fail silently, with no compile-time detection. >> >> Analysis and Solution Approach >> >> The core issue is that Go's error interface allows both pointer and value >> types to implement error, but the dynamic type matching in errors.As >> requires exact type correspondence. This creates opportunities for silent >> failures when the expected and actual types don't align. >> > That is the reason why it is generally recommended that error types use a > pointer receiver. That statically rules out mistakes like this. It is > unfortunate, that crypto/aes did not follow this recommendation.
Your opinion is actually cited as Source 9 in the accompanying blog article <https://blog.fillmore-labs.com/posts/errors-1/>. But, as mentioned, there are lots of existing codebases out there (even in the Go standard library) that use value receivers. >> I propose a two-part mitigation strategy: >> >> 1. Compile-time Intent Declaration >> >> Explicit compile-time assertions to document intended usage patterns: >> >> // MyValueError is intended to be used as a value >> var _ error = MyValueError{} >> >> // MyPointerError is intended to be used as a pointer >> var _ error = (*MyPointerError)(nil) > I think if someone does this, they are aware of the problem and should just > make their Error methods a pointer receiver. This would be an API change. > In my opinion, that is a much better recommendation, as it means this mistake > can't happen to users of a package, even if they don't use your linter. Maybe. As mentioned: “Good error type design is out of scope. While errortype promotes a consistent style, a broader refactor of your error handling strategy may sometimes be the better solution.” The point of the linter is not to guide you to a redesign, but to deal with an existing codebase. > I know that you disagree with the blanket "all error receivers should be > pointers" advice, though. In the Go standard library errors that aren’t structs mostly use value receivers. For new designs I recommended following “Sensible Defaults” <https://blog.fillmore-labs.com/posts/errors-1/#have-sensible-defaults> — mostly pointer receivers for non-empty structs. So we’re not that far apart. My point is that we have existing code bases we can not all redesign and they have existing bugs. I want to improve that situation. >> 2. Static Analysis Tool >> >> I've developed errortype, a static analyzer that detects inconsistencies >> between intended error type usage and actual usage patterns. The tool >> analyzes: >> >> Function return value types >> Type assertions and type switches >> errors.As target parameters >> Method receiver consistency patterns > For clarity: I understand your linter to > 1. Check that a package declaring SomeError uses that consistently - that is, > either exclusively assigns SomeError or exclusively assigns *SomeError to > error. It complains, if not. If you lint that package, yes. If you just use that package, it does check, but does not complain. > 2. If that usage is consistent, checks that any users of that package *also* > only assign the type consistent with that. It complains, if not. For linted packages, yes. Also, if you use an error that has unclear usage in the defining package, it might complain. > 3. Checks that all type-assertions/-switches and errors.As/errors.Is calls > happen consistent with that (if it is unambiguous) Yes, and also notice if the usage is ambiguous. > In particular, AIUI you do not rely on the assignment-statements above to > work, right? No, because that’s not feasible with current code bases. It would be nice when we were there, but we aren’t. I tested the heuristic on >600 open source projects, and it mostly works. The linter (and the heuristic) is experimental though, because we have no definite criteria. I think the linter is useful as-is, but my hope is to find some semi-official criteria which is more deterministic. That’s why I post here. I think “var _ error” is good, but maybe there are better ideas. > That seems like a very useful tool. Thanks. I strive for the tool to be useful, but also like input how to improve the situation in a mostly non-destructive way, meaning not discussion Go 2 errors, but how to deal with what we have. >> Example diagnostic output: >> >> main.go:14:20: Target for value error "crypto/aes.KeySizeError" is a >> pointer-to-pointer, use a pointer to a value instead: "var kse >> aes.KeySizeError; ... errors.As(err, &kse)". (et:err) >> Request for Feedback >> >> I'm particularly interested in feedback on a few points: >> >> Prevalence: Have you encountered this pointer/value ambiguity with error >> types? How common do you think this class of bug is in practice? >> Solution Approach: What are your thoughts on using var _ error = ... >> assertions to declare an error type's intended usage? Is this a practical >> convention to adopt? >> Tooling: The detection heuristics are based on these assertions and usage >> pattern analysis. I would be grateful for any real-world testing on your >> codebases to validate their effectiveness and performance. >> The tool is currently CLI-only while I refine the detection logic based on >> real-world usage patterns. Integration is planned in a later phase. >> >> References >> >> Detailed analysis: https://blog.fillmore-labs.com/posts/errors-1/ >> Tool repository: https://github.com/fillmore-labs/errortype >> Playground example: https://go.dev/play/p/m4SEPqkZ2Zu >> I welcome any insights, particularly from those who have encountered similar >> issues or have thoughts on static analysis approaches for Go error handling >> patterns. >> >> Best regards, Oliver >> Thanks for the feedback, Cheers Oliver -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. To view this discussion visit https://groups.google.com/d/msgid/golang-nuts/F468A7E5-E0EC-4B80-9BD2-29EFC9DFA9EB%40fillmore-labs.com.
