Skip to content

Commit 493bf2e

Browse files
paulirwinclaude
andcommitted
Address Copilot review feedback on PR #27
- Code fix: only offer TryGetValue rewrite when receiver type exposes an accessible public instance TryGetValue(TKey, out TValue) method, to avoid generating non-compiling code when the method is only available via explicit interface implementation. - Add unit test verifying PickLocalName collision-avoidance produces a unique name (value1) when 'value' is already in scope. - Fix misleading comment in DictionaryTypeHelper.IsGenericDictionaryIndexer to match actual behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9e14834 commit 493bf2e

3 files changed

Lines changed: 86 additions & 1 deletion

File tree

src/Lucene.Net.CodeAnalysis.Dev.CodeFixes/LuceneDev1xxx/LuceneDev1007_1008_DictionaryIndexerCodeFixProvider.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
4949
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
5050
if (root == null) return;
5151

52+
var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
53+
if (semanticModel == null) return;
54+
5255
foreach (var diagnostic in context.Diagnostics)
5356
{
5457
var elementAccess = root.FindToken(diagnostic.Location.SourceSpan.Start)
@@ -66,6 +69,11 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
6669
continue;
6770
}
6871

72+
// If the receiver type doesn't expose an accessible TryGetValue method
73+
// (e.g. only via explicit interface implementation), skip — the rewrite would not compile.
74+
if (!HasAccessibleTryGetValue(semanticModel, elementAccess))
75+
continue;
76+
6977
context.RegisterCodeFix(
7078
CodeAction.Create(
7179
title: TitleReturn,
@@ -75,6 +83,30 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
7583
}
7684
}
7785

86+
private static bool HasAccessibleTryGetValue(SemanticModel semanticModel, ElementAccessExpressionSyntax elementAccess)
87+
{
88+
var receiverType = semanticModel.GetTypeInfo(elementAccess.Expression).Type;
89+
if (receiverType == null)
90+
return false;
91+
92+
foreach (var member in receiverType.GetMembers("TryGetValue"))
93+
{
94+
if (member is not IMethodSymbol method)
95+
continue;
96+
if (method.IsStatic)
97+
continue;
98+
if (method.DeclaredAccessibility != Accessibility.Public)
99+
continue;
100+
if (method.Parameters.Length != 2)
101+
continue;
102+
if (method.Parameters[1].RefKind != RefKind.Out)
103+
continue;
104+
return true;
105+
}
106+
107+
return false;
108+
}
109+
78110
private static async Task<Document> ConvertReturnAsync(
79111
Document document,
80112
ReturnStatementSyntax returnStmt,

src/Lucene.Net.CodeAnalysis.Dev/Utility/DictionaryTypeHelper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public static bool IsGenericDictionaryIndexer(IPropertySymbol indexer, INamedTyp
3636
{
3737
valueType = null;
3838

39-
// Must take a single non-object key parameter. (The IDictionary.this[object] variant is handled elsewhere.)
39+
// Must take a single key parameter. (The non-generic IDictionary.this[object] variant is handled elsewhere.)
4040
if (indexer.Parameters.Length != 1)
4141
return false;
4242

tests/Lucene.Net.CodeAnalysis.Dev.CodeFixes.Tests/LuceneDev1xxx/TestLuceneDev1007_1008_DictionaryIndexerCodeFixProvider.cs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,5 +112,58 @@ public int M(IDictionary<string, int> dict, string key)
112112

113113
await test.RunAsync();
114114
}
115+
116+
[Test]
117+
public async Task TestFix_Return_PicksUniqueLocalName_WhenValueIsInScope()
118+
{
119+
var testCode = @"
120+
using System.Collections.Generic;
121+
122+
public class Sample
123+
{
124+
public int M(IDictionary<string, int> dict, string key)
125+
{
126+
int value = 42;
127+
if (value > 0)
128+
{
129+
return dict[key];
130+
}
131+
return value;
132+
}
133+
}";
134+
135+
var fixedCode = @"
136+
using System.Collections.Generic;
137+
138+
public class Sample
139+
{
140+
public int M(IDictionary<string, int> dict, string key)
141+
{
142+
int value = 42;
143+
if (value > 0)
144+
{
145+
return dict.TryGetValue(key, out var value1) ? value1 : default;
146+
}
147+
return value;
148+
}
149+
}";
150+
151+
var expected = new DiagnosticResult(Descriptors.LuceneDev1007_GenericDictionaryIndexerValueType)
152+
.WithSeverity(DiagnosticSeverity.Warning)
153+
.WithMessageFormat(Descriptors.LuceneDev1007_GenericDictionaryIndexerValueType.MessageFormat)
154+
.WithArguments("dict[key]")
155+
.WithLocation("/0/Test0.cs", line: 11, column: 20);
156+
157+
var test = new InjectableCodeFixTest(
158+
() => new LuceneDev1007_1008_DictionaryIndexerAnalyzer(),
159+
() => new LuceneDev1007_1008_DictionaryIndexerCodeFixProvider())
160+
{
161+
TestCode = testCode,
162+
FixedCode = fixedCode,
163+
ExpectedDiagnostics = { expected }
164+
};
165+
166+
await test.RunAsync();
167+
}
115168
}
116169
}

0 commit comments

Comments
 (0)