go icon indicating copy to clipboard operation
go copied to clipboard

x/tools/gopls: don't report deprecation diagnostics for type names in type switches or type assertions

Open findleyr opened this issue 4 months ago • 4 comments

While working on gopls, we have a few places where (for example) *ast.Object or *ast.Scope are handled in a type switch or type assertion, and the deprecated analyzer produces informational diagnostics for their usage. These diagnostics are generally not actionable, since the deprecated types may be present in input and still need to be handled. I think we should suppress deprecation notices in type switches or type assertions.

CC @hyangah @adonovan

findleyr avatar Apr 18 '24 14:04 findleyr

While working on gopls, we have a few places where (for example) *ast.Object or *ast.Scope are handled in a type switch or type assertion, and the deprecated analyzer produces informational diagnostics for their usage.

I wonder what is the generalization of this classification of symbol references into two classes that we might call "analytic" (deconstructors: type switches, comparisons, etc) and "synthetic" (constructors: var decls, literals, calls, etc)?

adonovan avatar Apr 18 '24 17:04 adonovan

Is there no case where the deprecation notice is useful in type switch or comparison? Let's say a type A is deprecated in favor of a better new type B in a new release. If I have a type switch on A but don't have B yet, learning about this new deprecation notice will help me update my code based on my application's need - maybe I have to continue having A case handling, or, maybe I am lucky enough to stop thinking about A. On the other hand, it's annoying that there is no suppression mechanism after I already verified the use of deprecated symbol in my type switch code is working as intended.

hyangah avatar Apr 18 '24 17:04 hyangah

I think the case for this notice being useful here is slim -- the example you suggest would be better handled by some sort of search for inexhaustive type switches.

We aim for very low false positives, so I think we should suppress this diagnostic.

findleyr avatar Apr 18 '24 17:04 findleyr

Should the use for type assertion, comparison be also suppressed? Should there be an option?

The current analyzer suppresses information about the usage in the same package for the same reason but I keep receiving questions related to this. Just got another one - https://github.com/golang/go/issues/40447#issuecomment-2064723963 I think this is one of the analyzers where people have different taste and need.

hyangah avatar Apr 18 '24 18:04 hyangah