Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
findleyr opened this issue Apr 18, 2024 · 4 comments
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@findleyr
Copy link
Contributor

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 findleyr added this to the gopls/v0.16.0 milestone Apr 18, 2024
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Apr 18, 2024
@adonovan
Copy link
Member

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)?

@hyangah
Copy link
Contributor

hyangah commented Apr 18, 2024

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.

@findleyr
Copy link
Contributor Author

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.

@hyangah
Copy link
Contributor

hyangah commented Apr 18, 2024

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 - #40447 (comment) I think this is one of the analyzers where people have different taste and need.

@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants