Fix analyzer RCS1246 - handle generic OrderedDictionary (#1676)

josefpihrt · web-flow · commit a3b471cd0e1f · 2025-08-03T23:17:22.000+02:00
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -28,6 +28,11 @@ jobs:
       - uses: actions/checkout@v4
         with:
           fetch-depth: 0
+      - uses: actions/setup-dotnet@v4
+        with:
+          dotnet-version: |
+            8.0.x
+            9.0.x
       - run: dotnet tool install -g GitVersion.Tool --version 5.11.1
       - name: Resolve version
         id: version
@@ -81,6 +86,11 @@ jobs:
         working-directory: src
     steps:
       - uses: actions/checkout@v4
+      - uses: actions/setup-dotnet@v4
+        with:
+          dotnet-version: |
+            8.0.x
+            9.0.x
       - run: dotnet restore Roslynator.CoreAndTesting.slnf
       - run: dotnet build Roslynator.CoreAndTesting.slnf --no-restore
       - run: dotnet pack Roslynator.CoreAndTesting.slnf --no-build -o _nupkg
@@ -263,7 +273,7 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-dotnet@v4
         with:
-          dotnet-version: 9.0.101
+          dotnet-version: 9.0.x
       - run: dotnet restore
       - run: dotnet build --no-restore
       - run: dotnet pack --no-build
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix analyzer [RCS1246](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1246) ([PR](https://github.com/dotnet/roslynator/pull/1676))
+
 ## [4.14.0] - 2025-07-26
 
 ### Added
diff --git a/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs b/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs
@@ -183,14 +183,12 @@ public static void OptimizeStringJoin(SyntaxNodeAnalysisContext context, in Simp
             return;
 
         if (!parameters[1].IsParameterArrayOf(SpecialType.System_String, SpecialType.System_Object)
-            && !parameters[1].Type.OriginalDefinition.IsIEnumerableOfT())
+            && !parameters[1].Type.OriginalDefinition.IsIEnumerableOfT()
+            && !parameters[1].Type.OriginalDefinition.HasMetadataName(MetadataNames.System_ReadOnlySpan_T))
         {
             return;
         }
 
-        if (firstArgument.Expression is null)
-            return;
-
         if (!CSharpUtility.IsEmptyStringExpression(firstArgument.Expression, semanticModel, cancellationToken))
             return;
 
diff --git a/src/Common/CSharp/Analysis/UseElementAccessAnalysis.cs b/src/Common/CSharp/Analysis/UseElementAccessAnalysis.cs
@@ -21,7 +21,8 @@ public static bool IsFixableElementAt(
         if (invocationExpression.IsParentKind(SyntaxKind.ExpressionStatement))
             return false;
 
-        IMethodSymbol methodSymbol = semanticModel.GetReducedExtensionMethodInfo(invocationExpression, cancellationToken).Symbol;
+        ExtensionMethodSymbolInfo reducedExtensionMethodInfo = semanticModel.GetReducedExtensionMethodInfo(invocationExpression, cancellationToken);
+        IMethodSymbol methodSymbol = reducedExtensionMethodInfo.Symbol;
 
         if (methodSymbol is null)
             return false;
@@ -31,7 +32,7 @@ public static bool IsFixableElementAt(
 
         ITypeSymbol typeSymbol = semanticModel.GetTypeSymbol(invocationInfo.Expression, cancellationToken);
 
-        if (!HasAccessibleIndexer(typeSymbol, semanticModel, invocationExpression.SpanStart))
+        if (!HasAccessibleIndexer(typeSymbol, reducedExtensionMethodInfo.ReducedSymbolOrSymbol.ReturnType, semanticModel, invocationExpression.SpanStart))
             return false;
 
         ElementAccessExpressionSyntax elementAccess = SyntaxFactory.ElementAccessExpression(
@@ -53,7 +54,8 @@ public static bool IsFixableFirst(
         if (invocationInfo.InvocationExpression.IsParentKind(SyntaxKind.ExpressionStatement))
             return false;
 
-        IMethodSymbol methodSymbol = semanticModel.GetReducedExtensionMethodInfo(invocationInfo.InvocationExpression, cancellationToken).Symbol;
+        ExtensionMethodSymbolInfo reducedExtensionMethodInfo = semanticModel.GetReducedExtensionMethodInfo(invocationInfo.InvocationExpression, cancellationToken);
+        IMethodSymbol methodSymbol = reducedExtensionMethodInfo.Symbol;
 
         if (methodSymbol is null)
             return false;
@@ -63,7 +65,7 @@ public static bool IsFixableFirst(
 
         ITypeSymbol typeSymbol = semanticModel.GetTypeSymbol(invocationInfo.Expression, cancellationToken);
 
-        return HasAccessibleIndexer(typeSymbol, semanticModel, invocationInfo.InvocationExpression.SpanStart);
+        return HasAccessibleIndexer(typeSymbol, reducedExtensionMethodInfo.ReducedSymbolOrSymbol.ReturnType, semanticModel, invocationInfo.InvocationExpression.SpanStart);
     }
 
     public static bool IsFixableLast(
@@ -80,7 +82,8 @@ public static bool IsFixableLast(
         if (semanticModel.Compilation.GetTypeByMetadataName("System.Index")?.DeclaredAccessibility != Accessibility.Public)
             return false;
 
-        IMethodSymbol methodSymbol = semanticModel.GetReducedExtensionMethodInfo(invocationInfo.InvocationExpression, cancellationToken).Symbol;
+        ExtensionMethodSymbolInfo reducedExtensionMethodInfo = semanticModel.GetReducedExtensionMethodInfo(invocationInfo.InvocationExpression, cancellationToken);
+        IMethodSymbol methodSymbol = reducedExtensionMethodInfo.Symbol;
 
         if (methodSymbol is null)
             return false;
@@ -90,7 +93,7 @@ public static bool IsFixableLast(
 
         ITypeSymbol typeSymbol = semanticModel.GetTypeSymbol(invocationInfo.Expression, cancellationToken);
 
-        return HasAccessibleIndexer(typeSymbol, semanticModel, invocationInfo.InvocationExpression.SpanStart);
+        return HasAccessibleIndexer(typeSymbol, reducedExtensionMethodInfo.ReducedSymbolOrSymbol.ReturnType, semanticModel, invocationInfo.InvocationExpression.SpanStart);
     }
 
     private static bool CheckInfiniteRecursion(
diff --git a/src/Core/SymbolUtility.cs b/src/Core/SymbolUtility.cs
@@ -93,6 +93,7 @@ public static bool IsEventHandlerMethod(IMethodSymbol methodSymbol)
 
     public static bool HasAccessibleIndexer(
         ITypeSymbol typeSymbol,
+        ITypeSymbol returnType,
         SemanticModel semanticModel,
         int position)
     {
@@ -135,8 +136,12 @@ public static bool HasAccessibleIndexer(
 
             foreach (ISymbol symbol in typeSymbol.GetMembers("this[]"))
             {
-                if (semanticModel.IsAccessible(position, symbol))
+                if (semanticModel.IsAccessible(position, symbol)
+                    && symbol is IPropertySymbol indexerSymbol
+                    && SymbolEqualityComparer.IncludeNullability.Equals(indexerSymbol.Type, returnType))
+                {
                     return true;
+                }
             }
         }
 
diff --git a/src/Refactorings/CSharp/Refactorings/ForEachStatementRefactoring.cs b/src/Refactorings/CSharp/Refactorings/ForEachStatementRefactoring.cs
@@ -32,8 +32,9 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Fo
             semanticModel = await context.GetSemanticModelAsync().ConfigureAwait(false);
 
             ITypeSymbol typeSymbol = semanticModel.GetTypeSymbol(forEachStatement.Expression, context.CancellationToken);
+            ITypeSymbol elementTypeSymbol = semanticModel.GetTypeSymbol(forEachStatement.Type, context.CancellationToken);
 
-            if (SymbolUtility.HasAccessibleIndexer(typeSymbol, semanticModel, forEachStatement.SpanStart))
+            if (SymbolUtility.HasAccessibleIndexer(typeSymbol, elementTypeSymbol, semanticModel, forEachStatement.SpanStart))
             {
                 if (context.IsRefactoringEnabled(RefactoringDescriptors.ConvertForEachToFor))
                 {
diff --git a/src/Tests/Analyzers.Tests/Analyzers.Tests.csproj b/src/Tests/Analyzers.Tests/Analyzers.Tests.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tests/Analyzers.Tests/RCS1246UseElementAccessTests.cs b/src/Tests/Analyzers.Tests/RCS1246UseElementAccessTests.cs
@@ -313,6 +313,27 @@ class C : IReadOnlyList<int>
 
     IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
 }
+");
+    }
+
+    [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UseElementAccess)]
+    public async Task TestNoDiagnostic_UseElementAccessInsteadOf_OrderedDictionary()
+    {
+        await VerifyNoDiagnosticAsync(@"
+using System.Collections.Generic;
+using System.Linq;
+
+class C
+{
+    void M()
+    {
+        OrderedDictionary<string, string> dic = new OrderedDictionary<string, string>();
+
+        KeyValuePair<string, string> first = dic.First();
+        KeyValuePair<string, string> last = dic.Last();
+        KeyValuePair<string, string> elementAt = dic.ElementAt(1);
+     }
+}
 ");
     }
 }
diff --git a/src/Tests/CSharp.Tests/CSharp.Tests.csproj b/src/Tests/CSharp.Tests/CSharp.Tests.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tests/CSharp.Tests/SyntaxKindTests.cs b/src/Tests/CSharp.Tests/SyntaxKindTests.cs
@@ -15,7 +15,7 @@ public static void DetectNewSyntaxKinds()
     {
         List<SyntaxKind> unknownKinds = null;
 
-        foreach (SyntaxKind value in Enum.GetValues(typeof(SyntaxKind)))
+        foreach (SyntaxKind value in Enum.GetValues<SyntaxKind>())
         {
             switch (value)
             {
diff --git a/src/Tests/CSharp.Workspaces.Tests/CSharp.Workspaces.Tests.csproj b/src/Tests/CSharp.Workspaces.Tests/CSharp.Workspaces.Tests.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tests/CodeAnalysis.Analyzers.Tests/CodeAnalysis.Analyzers.Tests.csproj b/src/Tests/CodeAnalysis.Analyzers.Tests/CodeAnalysis.Analyzers.Tests.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tests/CodeFixes.Tests/CodeFixes.Tests.csproj b/src/Tests/CodeFixes.Tests/CodeFixes.Tests.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tests/Core.Tests/Core.Tests.csproj b/src/Tests/Core.Tests/Core.Tests.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tests/Formatting.Analyzers.Tests/Formatting.Analyzers.Tests.csproj b/src/Tests/Formatting.Analyzers.Tests/Formatting.Analyzers.Tests.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tests/Refactorings.Tests/RR0129ConvertForEachToForTests.cs b/src/Tests/Refactorings.Tests/RR0129ConvertForEachToForTests.cs
@@ -0,0 +1,67 @@
+using System.Threading.Tasks;
+using Roslynator.Testing.CSharp;
+using Xunit;
+
+namespace Roslynator.CSharp.Refactorings.Tests;
+
+public class RR0129ConvertForEachToForTests : AbstractCSharpRefactoringVerifier
+{
+    public override string RefactoringId { get; } = RefactoringIdentifiers.ConvertForEachToFor;
+
+    [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.ConvertForEachToFor)]
+    public async Task Test()
+    {
+        await VerifyRefactoringAsync(@"
+using System.Collections.Generic;
+
+class C
+{
+    void M()
+    {
+        List<string> items = new List<string>();
+
+        f[||]oreach (string item in items)
+        {
+            var x = item;
+        }
+    }
+}
+", @"
+using System.Collections.Generic;
+
+class C
+{
+    void M()
+    {
+        List<string> items = new List<string>();
+
+        for (int i = 0; i < items.Count; i++)
+        {
+            var x = items[i];
+        }
+    }
+}
+", equivalenceKey: EquivalenceKey.Create(RefactoringId));
+    }
+
+    [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.ConvertForEachToFor)]
+    public async Task TestNoRefactoring_OrderedDictionary()
+    {
+        await VerifyNoRefactoringAsync(@"
+using System.Collections.Generic;
+
+class C
+{
+    void M()
+    {
+        OrderedDictionary<int, string> dic = new OrderedDictionary<int, string>();
+
+        f[||]oreach (KeyValuePair<int, string> item in dic)
+        {
+            KeyValuePair<int, string> x = item;
+        }
+     }
+}
+", equivalenceKey: EquivalenceKey.Create(RefactoringId));
+    }
+}
diff --git a/src/Tests/Refactorings.Tests/Refactorings.Tests.csproj b/src/Tests/Refactorings.Tests/Refactorings.Tests.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tests/TestConsole/TestConsole.csproj b/src/Tests/TestConsole/TestConsole.csproj
@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tests/TestLibrary/TestLibrary.csproj b/src/Tests/TestLibrary/TestLibrary.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net8.0</TargetFramework>
+    <TargetFramework>net9.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup>
diff --git a/src/Tools/CodeGeneration/Markdown/MarkdownGenerator.cs b/src/Tools/CodeGeneration/Markdown/MarkdownGenerator.cs
@@ -158,7 +158,7 @@ public static string CreateAnalyzerMarkdown(AnalyzerMetadata analyzer, Immutable
 
         static IEnumerable<MElement> CreateSamples(AnalyzerMetadata analyzer)
         {
-            IReadOnlyList<SampleMetadata> samples = analyzer.Samples;
+            List<SampleMetadata> samples = analyzer.Samples;
 
             if (samples.Count > 0)
             {

Original file line number	Diff line number	Diff line change
`@@ -183,14 +183,12 @@ public static void OptimizeStringJoin(SyntaxNodeAnalysisContext context, in Simp`
`183`	`183`	`return;`
`184`	`184`
`185`	`185`	`if (!parameters[1].IsParameterArrayOf(SpecialType.System_String, SpecialType.System_Object)`
`186`		`- && !parameters[1].Type.OriginalDefinition.IsIEnumerableOfT())`
	`186`	`+ && !parameters[1].Type.OriginalDefinition.IsIEnumerableOfT()`
	`187`	`+ && !parameters[1].Type.OriginalDefinition.HasMetadataName(MetadataNames.System_ReadOnlySpan_T))`
`187`	`188`	`{`
`188`	`189`	`return;`
`189`	`190`	`}`
`190`	`191`
`191`		`- if (firstArgument.Expression is null)`
`192`		`- return;`
`193`		`-`
`194`	`192`	`if (!CSharpUtility.IsEmptyStringExpression(firstArgument.Expression, semanticModel, cancellationToken))`
`195`	`193`	`return;`
`196`	`194`
Original file line number	Diff line number	Diff line change
`@@ -93,6 +93,7 @@ public static bool IsEventHandlerMethod(IMethodSymbol methodSymbol)`
`93`	`93`
`94`	`94`	`public static bool HasAccessibleIndexer(`
`95`	`95`	`ITypeSymbol typeSymbol,`
	`96`	`+ ITypeSymbol returnType,`
`96`	`97`	`SemanticModel semanticModel,`
`97`	`98`	`int position)`
`98`	`99`	`{`
`@@ -135,8 +136,12 @@ public static bool HasAccessibleIndexer(`
`135`	`136`
`136`	`137`	`foreach (ISymbol symbol in typeSymbol.GetMembers("this[]"))`
`137`	`138`	`{`
`138`		`- if (semanticModel.IsAccessible(position, symbol))`
	`139`	`+ if (semanticModel.IsAccessible(position, symbol)`
	`140`	`+ && symbol is IPropertySymbol indexerSymbol`
	`141`	`+ && SymbolEqualityComparer.IncludeNullability.Equals(indexerSymbol.Type, returnType))`
	`142`	`+ {`
`139`	`143`	`return true;`
	`144`	`+ }`
`140`	`145`	`}`
`141`	`146`	`}`
`142`	`147`
Original file line number	Diff line number	Diff line change
`@@ -32,8 +32,9 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Fo`
`32`	`32`	`semanticModel = await context.GetSemanticModelAsync().ConfigureAwait(false);`
`33`	`33`
`34`	`34`	`ITypeSymbol typeSymbol = semanticModel.GetTypeSymbol(forEachStatement.Expression, context.CancellationToken);`
	`35`	`+ ITypeSymbol elementTypeSymbol = semanticModel.GetTypeSymbol(forEachStatement.Type, context.CancellationToken);`
`35`	`36`
`36`		`- if (SymbolUtility.HasAccessibleIndexer(typeSymbol, semanticModel, forEachStatement.SpanStart))`
	`37`	`+ if (SymbolUtility.HasAccessibleIndexer(typeSymbol, elementTypeSymbol, semanticModel, forEachStatement.SpanStart))`
`37`	`38`	`{`
`38`	`39`	`if (context.IsRefactoringEnabled(RefactoringDescriptors.ConvertForEachToFor))`
`39`	`40`	`{`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ public static void DetectNewSyntaxKinds()`
`15`	`15`	`{`
`16`	`16`	`List<SyntaxKind> unknownKinds = null;`
`17`	`17`
`18`		`- foreach (SyntaxKind value in Enum.GetValues(typeof(SyntaxKind)))`
	`18`	`+ foreach (SyntaxKind value in Enum.GetValues<SyntaxKind>())`
`19`	`19`	`{`
`20`	`20`	`switch (value)`
`21`	`21`	`{`