Harden analyzer symbol handling#9666
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes Orleans analyzer implementations by migrating from syntax-based analysis to semantic-based analysis using symbols. This improves reliability and follows analyzer best practices by avoiding error-prone syntax parsing.
Key changes:
- Migration from
SyntaxNodeAnalysisContexttoSymbolAnalysisContextacross multiple analyzers - Introduction of new
SymbolHelpersutility class for semantic operations - Removal of unused syntax-based helper methods from
SyntaxHelpers
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
test/Analyzers.Tests/AbstractPropertiesCannotBeSerializedAnalyzerTest.cs |
Removed UTF-8 BOM character |
src/Orleans.Analyzers/SyntaxHelpers.cs |
Removed unused syntax-based helper methods and made remaining methods private |
src/Orleans.Analyzers/SymbolHelpers.cs |
New utility class with semantic-based helper methods |
src/Orleans.Analyzers/SerializationAttributesHelper.cs |
Updated to use symbol-based analysis and removed unused properties |
src/Orleans.Analyzers/NoRefParamsDiagnosticAnalyzer.cs |
Migrated from syntax to symbol analysis |
src/Orleans.Analyzers/IncorrectAttributeUseAnalyzer.cs |
Migrated from syntax to symbol analysis |
src/Orleans.Analyzers/IdClashAttributeAnalyzer.cs |
Migrated from syntax to symbol analysis with improved type safety |
src/Orleans.Analyzers/GrainInterfaceMethodReturnTypeDiagnosticAnalyzer.cs |
Migrated from syntax to symbol analysis and simplified type checking |
src/Orleans.Analyzers/GenerateGenerateSerializerAttributeAnalyzer.cs |
Migrated from syntax to symbol analysis |
src/Orleans.Analyzers/GenerateAliasAttributesAnalyzer.cs |
Migrated from syntax to symbol analysis |
src/Orleans.Analyzers/AtMostOneOrleansConstructorAnalyzer.cs |
Migrated from syntax to symbol analysis with improved constructor counting |
src/Orleans.Analyzers/AlwaysInterleaveDiagnosticAnalyzer.cs |
Migrated from syntax to symbol analysis |
src/Orleans.Analyzers/AliasClashAttributeAnalyzer.cs |
Migrated from syntax to symbol analysis |
src/Orleans.Analyzers/AbstractPropertiesCannotBeSerializedAnalyzer.cs |
Migrated to operation-based analysis |
ReubenBond
reviewed
Oct 1, 2025
ReubenBond
approved these changes
Oct 1, 2025
9128e39 to
592a2ce
Compare
Avoid indexing DeclaringSyntaxReferences for symbols without source declarations and add a regression test for referenced grain interfaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
923a420 to
d2b232b
Compare
ReubenBond
approved these changes
Feb 15, 2026
Add a dedicated test suite for AtMostOneOrleansConstructorAnalyzer and expand GenerateGenerateSerializerAttributeAnalyzer edge-case coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rkargMsft
pushed a commit
to rkargMsft/orleans
that referenced
this pull request
Feb 27, 2026
* Cleanup AbstractPropertiesCannotBeSerializedAnalyzer * More cleanup * Cleanup AlwaysInterleaveDiagnosticAnalyzer * Cleanup AtMostOneOrleansConstructorAnalyzer * Cleanup GenerateAliasAttributesAnalyzer * Cleanup no longer used helper * Cleanup GenerateGenerateSerializerAttributeAnalyzer * Cleanup GrainInterfaceMethodReturnTypeDiagnosticAnalyzer * Cleanup dead code * Cleanup IncorrectAttributeUseAnalyzer * Cleanup SyntaxHelpers * Cleanup IdClashAttributeAnalyzer * Cleanup NoRefParamsDiagnosticAnalyzer * Apply suggestion from @ReubenBond * Guard GenerateAlias analyzer syntax access Avoid indexing DeclaringSyntaxReferences for symbols without source declarations and add a regression test for referenced grain interfaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add missing analyzer test coverage Add a dedicated test suite for AtMostOneOrleansConstructorAnalyzer and expand GenerateGenerateSerializerAttributeAnalyzer edge-case coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Reuben Bond <203839+ReubenBond@users.noreply.github.com> Co-authored-by: Reuben Bond <reuben.bond@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current implementation of analyzers relies on syntax when it really needs semantics. This is a bad practice and is very error prone.
This PR modernizes few analyzer implementations to follow better practices and be more solid. There is much more to cleanup, but it's a good step so far.