Skip to content

[analyzer plugins] Plugin-reported diagnostics should be ignorable inline #59647

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

Closed
Tracked by #53402
srawlins opened this issue Dec 2, 2024 · 1 comment
Closed
Tracked by #53402
Assignees
Labels
devexp-plugin legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

srawlins commented Dec 2, 2024

The design we've roughly landed on is that // ignore: and // ignore_for_file: comments will supported for plugin-defined diagnostics (lints and warnings) via a plugin-namespaced name:

// ignore: foo.bar

This will direct the analyzer plugin tooling to ignore a report of the bar diagnostic from the foo plugin.

Notice the period (.) delimiter. We discussed also the : delimiter, but that looks confusing with ignore:, especially since there does not need to be whitespace after ignore: (e.g. // ignore:foo:bar). We also considered /, which I believe tslint uses, something like // ignore: foo/bar. We decided that the period looks the most "darty," being reminiscent of property access, like "the bar diagnostic of the foo plugin."

@srawlins srawlins added legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug devexp-plugin labels Dec 2, 2024
@srawlins
Copy link
Member Author

Two updates:

  • I realized that internally at Google we can name packages with period separators (package:foo.bar.baz). So I think we should change the delimiter to the alternate we considered, /.
  • This proposal does not yet address the type=lint|warning bit. If a file contains // ignore_for_file: type=warning, and an analyzer plugin produces a warning in that file, should it be ignored? I think yes. If it wasn't ignored, it would defeat the motivation for type=, which is to let code-generators generate code that complies with some unknown lint rule set.

@srawlins srawlins self-assigned this Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-plugin legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

1 participant