Skip to content

Conversation

@NehanPathan
Copy link
Contributor

This PR reserves diagnostic IDs 6001-6003 and adds corresponding entries in AnalyzerReleases.Unshipped.md for the new analyzers:

  • LuceneDev6001: String overloads must use StringComparison.Ordinal or OrdinalIgnoreCase (Error)
  • LuceneDev6002: Span overloads should not pass non-Ordinal StringComparison (Warning)
  • LuceneDev6003: Single-character string arguments should use char overload (Info)

These changes are in preparation for implementing the analyzers in the next PR.

Copy link
Contributor

@NightOwl888 NightOwl888 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

I am not an expert on writing code analyzers, but I thought this was going to require more diagnostic IDs. I will give you the benefit of the doubt, though. Having fewer IDs makes reviewing them easier. If it turns out we need more, we can just reserve more later.

Please see my inline comment - just one minor thing to fix before this can be merged to suppress the warnings about the table format in AnalyzerReleases.Unshipped.md.

@NightOwl888 NightOwl888 added the notes:ignore Don't show this PR in the Release Notes label Oct 17, 2025
@NightOwl888 NightOwl888 merged commit d17a264 into apache:main Oct 17, 2025
8 checks passed
@paulirwin
Copy link
Contributor

As long as the message of the diagnostic is the same, and the code fix if applicable, you can have as much logic in the diagnostic (and code fix) as you want. So it's fine to have an analyzer that handles lots of different sub-cases.

Note though that we do not want to update the Unshipped file with analyzers that do not exist yet. The only thing needed to do to reserve an ID is to update the ranges file at this time. Typically you will update the Unshipped file with the analyzers that are included with your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes:ignore Don't show this PR in the Release Notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants