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

Report invalid enforcement comments #59

Open
ImprintNav opened this issue Mar 2, 2023 · 2 comments
Open

Report invalid enforcement comments #59

ImprintNav opened this issue Mar 2, 2023 · 2 comments

Comments

@ImprintNav
Copy link

It is possible for a user to misplace an enforcement comment and thereby think enforcement is going to occur even when it doesn't, leading to bugs. Enforcement comments that don't end up touching a switch or map should raise an error.

The same might be said about any //exhaustive:ignore comments that aren't applied to any valid AST nodes.

@nishanths
Copy link
Owner

nishanths commented May 3, 2023

+1 for the check.

To implement this, it seems that the program would need to visit all AST nodes, not just SwitchStmt (switch statements), CompositeLit (map literals), and GenDecl (enum constants) nodes, as the program currently does. Visiting all nodes may have an effect on the program's performance. So, perhaps, the check for invalid directive comments should be behind a flag, and the flag's description can mention the possible additional performance cost.

@ImprintNav
Copy link
Author

I'm starting to think it would be better to create a separate analyzer just for this purpose. Other analyzers (e.g. exhaustruct) might also have directives that should only be applied to certain nodes. It would be more efficient to do a single analyzer pass to run over all nodes and check for mismatches. It wouldn't allow for as much nuanced validations but that could be handled by individual analyzers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants