Fix RoslynAnalyzer crash when analyzing indexer properties in .NET 10.0#121603
Fix RoslynAnalyzer crash when analyzing indexer properties in .NET 10.0#121603
Conversation
Handle case where IParameterSymbol.ContainingSymbol is IPropertySymbol for indexer parameters. When the parameter's containing symbol is a property, get the appropriate accessor method (getter or setter). Use SymbolEqualityComparer for symbol comparison as required by analyzer rules. Co-authored-by: sbomer <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a crash in the RoslynAnalyzer when analyzing indexer properties with default parameter values in .NET 10.0. The analyzer framework is designed for analyzing method parameters based on IL semantics, but Roslyn's operation visitor provides operation blocks for property parameter initializers (indexers), which don't have direct IL equivalents.
Key Changes:
- Added a null check in
GetParameterTargetValueto handle parameters not associated with methods, returningTopValueinstead of crashing - Added comprehensive test coverage for indexer properties with default arguments
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs | Adds null check for parameters not owned by methods (e.g., indexer default parameters) to prevent crash |
| src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs | Adds test case IndexerDefaultArgument to verify the fix handles indexer default parameters correctly |
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs
Outdated
Show resolved
Hide resolved
…alysisVisitor.cs Co-authored-by: Copilot <[email protected]>
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs
Show resolved
Hide resolved
jtschuster
left a comment
There was a problem hiding this comment.
This will fix the crash but we should file an issue for indexer parameters with DAM if we don't fix it here.
- Add clarifying comment - Add more test coverage
|
/backport to release/10.0 |
|
Started backporting to |
|
@sbomer backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Patch format detection failed.
Error: The process '/usr/bin/git' failed with exit code 128 |
Co-authored-by: sbomer <[email protected]>
#121861) Backport of #121603 to release/10.0 ## Customer Impact - [x] Customer reported - [ ] Found internally Reported by customer in #121546 ## Regression - [x] Yes - [] No Regressed in .NET 10 by analyzer changes that touched related code. ## Testing Verified on the provided repro. ## Risk Low. Adds an early return to the analysis in a place that doesn't have downstream effects. <hr /> Backports #121603 to release/10.0. Fixes #121546. The RoslynAnalyzer crashes when analyzing indexer property parameter initializers because it assumes all parameters belong to methods, but Roslyn provides operation blocks for property indexer parameters that don't have a direct IL equivalent. ## Changes - **TrimAnalysisVisitor.cs**: Guard against null `parameterMethod` in `GetParameterTargetValue`. Returns `TopValue` for parameters not owned by methods (e.g., indexer parameter initializers), preventing crashes while preserving correct analysis for method parameters. - **PropertyDataFlow.cs**: Add test coverage for annotated indexer parameters and indexers with default arguments to verify the analyzer handles these cases without crashing. ```csharp // Previously crashed on this pattern: public Type this[[DynamicallyAccessedMembers(...)] Type index] { get { ... } set { ... } } ``` Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: sbomer <[email protected]>
Our analysis framework is designed for analyzing parameters of methods (since it's analyzing based on IL semantics), but the Roslyn operation visitor gives us an operation block for the parameter initializers of properties (indexers). These appear like any other operation block, and don't seem to provide a way to short-circuit the analysis early.
This fixes the issue at the leaf when we visit a parameter reference that's not owned by a method. In this case it doesn't make sense to look for the associated getter/setter because the assignment operation is on the property declaration and has no direct equivalent in IL.
Fixes #121546