Skip to content

Commit 3a5a687

Browse files
paulirwinclaude
paulirwin
and
claude
authored
Implement LuceneDev1007, 1008, 6000 Analyzers & CodeFix with Unit Tests (apache#27)
* Implement LuceneDev1007, 1008, 6000 Analyzers & CodeFix with Unit Tests Adds diagnostics for dictionary indexer usage ported from Java: - LuceneDev1007 (Design/Warning): generic IDictionary<,> / IReadOnlyDictionary<,> indexer read with a value-type value — may throw KeyNotFoundException. - LuceneDev1008 (Design/Warning): same, but reference-type value; fix should also null-check the result. - LuceneDev6000 (Usage/Info): non-generic IDictionary indexer read, which returns null for missing keys and should be reviewed. Includes a code fix for LuceneDev1007/1008 that rewrites the common `return dict[key];` pattern into `return dict.TryGetValue(key, out var value) ? value : default;`. Closes #1168. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address Copilot review feedback on PR apache#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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 614bad6 commit 3a5a687

12 files changed

Lines changed: 1098 additions & 0 deletions

File tree

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

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
using System.Collections.Immutable;
21+
using System.Composition;
22+
using System.Linq;
23+
using System.Threading;
24+
using System.Threading.Tasks;
25+
using Lucene.Net.CodeAnalysis.Dev.Utility;
26+
using Microsoft.CodeAnalysis;
27+
using Microsoft.CodeAnalysis.CodeActions;
28+
using Microsoft.CodeAnalysis.CodeFixes;
29+
using Microsoft.CodeAnalysis.CSharp;
30+
using Microsoft.CodeAnalysis.CSharp.Syntax;
31+
using Microsoft.CodeAnalysis.Formatting;
32+
33+
namespace Lucene.Net.CodeAnalysis.Dev.CodeFixes.LuceneDev1xxx
34+
{
35+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(LuceneDev1007_1008_DictionaryIndexerCodeFixProvider)), Shared]
36+
public sealed class LuceneDev1007_1008_DictionaryIndexerCodeFixProvider : CodeFixProvider
37+
{
38+
private const string TitleReturn = "Use TryGetValue and return default on missing key";
39+
40+
public override ImmutableArray<string> FixableDiagnosticIds =>
41+
ImmutableArray.Create(
42+
Descriptors.LuceneDev1007_GenericDictionaryIndexerValueType.Id,
43+
Descriptors.LuceneDev1008_GenericDictionaryIndexerReferenceType.Id);
44+
45+
public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
46+
47+
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
48+
{
49+
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
50+
if (root == null) return;
51+
52+
var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
53+
if (semanticModel == null) return;
54+
55+
foreach (var diagnostic in context.Diagnostics)
56+
{
57+
var elementAccess = root.FindToken(diagnostic.Location.SourceSpan.Start)
58+
.Parent?
59+
.AncestorsAndSelf()
60+
.OfType<ElementAccessExpressionSyntax>()
61+
.FirstOrDefault(e => e.Span.Contains(diagnostic.Location.SourceSpan));
62+
if (elementAccess == null)
63+
continue;
64+
65+
// Only handle the "return dict[key];" pattern automatically.
66+
if (elementAccess.Parent is not ReturnStatementSyntax returnStmt
67+
|| returnStmt.Expression != elementAccess)
68+
{
69+
continue;
70+
}
71+
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+
77+
context.RegisterCodeFix(
78+
CodeAction.Create(
79+
title: TitleReturn,
80+
createChangedDocument: c => ConvertReturnAsync(context.Document, returnStmt, elementAccess, c),
81+
equivalenceKey: TitleReturn),
82+
diagnostic);
83+
}
84+
}
85+
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+
110+
private static async Task<Document> ConvertReturnAsync(
111+
Document document,
112+
ReturnStatementSyntax returnStmt,
113+
ElementAccessExpressionSyntax elementAccess,
114+
CancellationToken cancellationToken)
115+
{
116+
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
117+
if (root == null) return document;
118+
119+
var receiver = elementAccess.Expression;
120+
var keyArg = elementAccess.ArgumentList.Arguments.FirstOrDefault();
121+
if (keyArg == null) return document;
122+
123+
var outName = PickLocalName(returnStmt);
124+
125+
// receiver.TryGetValue(key, out var <outName>)
126+
var tryGetValueInvocation = SyntaxFactory.InvocationExpression(
127+
SyntaxFactory.MemberAccessExpression(
128+
SyntaxKind.SimpleMemberAccessExpression,
129+
receiver.WithoutTrivia(),
130+
SyntaxFactory.IdentifierName("TryGetValue")),
131+
SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(new[]
132+
{
133+
keyArg.WithoutTrivia(),
134+
SyntaxFactory.Argument(
135+
SyntaxFactory.DeclarationExpression(
136+
SyntaxFactory.IdentifierName(
137+
SyntaxFactory.Identifier("var")),
138+
SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier(outName))))
139+
.WithRefOrOutKeyword(SyntaxFactory.Token(SyntaxKind.OutKeyword))
140+
})));
141+
142+
// tryGetValueInvocation ? <outName> : default
143+
var ternary = SyntaxFactory.ConditionalExpression(
144+
tryGetValueInvocation,
145+
SyntaxFactory.IdentifierName(outName),
146+
SyntaxFactory.LiteralExpression(SyntaxKind.DefaultLiteralExpression,
147+
SyntaxFactory.Token(SyntaxKind.DefaultKeyword)));
148+
149+
var newReturn = returnStmt.WithExpression(ternary).WithAdditionalAnnotations(Formatter.Annotation);
150+
151+
var newRoot = root.ReplaceNode(returnStmt, newReturn);
152+
return document.WithSyntaxRoot(newRoot);
153+
}
154+
155+
private static string PickLocalName(SyntaxNode context)
156+
{
157+
// Avoid collisions with identifiers in the enclosing member.
158+
var member = context.AncestorsAndSelf().OfType<MemberDeclarationSyntax>().FirstOrDefault();
159+
var names = member == null
160+
? ImmutableHashSet<string>.Empty
161+
: member.DescendantTokens()
162+
.Where(t => t.IsKind(SyntaxKind.IdentifierToken))
163+
.Select(t => t.ValueText)
164+
.ToImmutableHashSet();
165+
166+
if (!names.Contains("value"))
167+
return "value";
168+
for (int i = 1; i < 100; i++)
169+
{
170+
var candidate = "value" + i;
171+
if (!names.Contains(candidate))
172+
return candidate;
173+
}
174+
return "value";
175+
}
176+
}
177+
}

‎src/Lucene.Net.CodeAnalysis.Dev.Sample/LuceneDev1xxx/LuceneDev1007_1008_DictionaryIndexerSample.cs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
using System.Collections.Generic;
21+
22+
namespace Lucene.Net.CodeAnalysis.Dev.Sample.LuceneDev1xxx;
23+
24+
public class LuceneDev1007_1008_DictionaryIndexerSample
25+
{
26+
public int GetIntValue(IDictionary<string, int> dict, string key)
27+
{
28+
// LuceneDev1007 (value-type value): indexer may throw KeyNotFoundException.
29+
return dict[key];
30+
}
31+
32+
public string GetStringValue(IDictionary<string, string> dict, string key)
33+
{
34+
// LuceneDev1008 (reference-type value): indexer may throw KeyNotFoundException.
35+
return dict[key];
36+
}
37+
38+
public void ReadOnlyUsage(IReadOnlyDictionary<string, string> dict, string key)
39+
{
40+
// LuceneDev1008: also applies to IReadOnlyDictionary<TKey, TValue>.
41+
var value = dict[key];
42+
}
43+
44+
public void ConcreteDictionaryUsage(Dictionary<string, string> dict, string key)
45+
{
46+
// LuceneDev1008: Dictionary<TKey, TValue> implements IDictionary<TKey, TValue>.
47+
var value = dict[key];
48+
}
49+
50+
public void AssignmentIsFine(Dictionary<string, int> dict, string key)
51+
{
52+
// No diagnostic: indexer setter does not throw.
53+
dict[key] = 42;
54+
}
55+
}

‎src/Lucene.Net.CodeAnalysis.Dev.Sample/LuceneDev6xxx/LuceneDev6000_NonGenericDictionaryIndexerSample.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
using System.Collections;
21+
22+
namespace Lucene.Net.CodeAnalysis.Dev.Sample.LuceneDev6xxx;
23+
24+
public class LuceneDev6000_NonGenericDictionaryIndexerSample
25+
{
26+
public object? GetValue(IDictionary dict, object key)
27+
{
28+
// LuceneDev6000 (Info): non-generic IDictionary indexer may return null for missing keys.
29+
return dict[key];
30+
}
31+
32+
public object? GetValueFromHashtable(Hashtable table, object key)
33+
{
34+
// LuceneDev6000: Hashtable implements non-generic IDictionary.
35+
return table[key];
36+
}
37+
}

‎src/Lucene.Net.CodeAnalysis.Dev/LuceneDev1xxx/LuceneDev1007_1008_DictionaryIndexerAnalyzer.cs‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
using System.Collections.Immutable;
21+
using Lucene.Net.CodeAnalysis.Dev.Utility;
22+
using Microsoft.CodeAnalysis;
23+
using Microsoft.CodeAnalysis.CSharp;
24+
using Microsoft.CodeAnalysis.CSharp.Syntax;
25+
using Microsoft.CodeAnalysis.Diagnostics;
26+
27+
namespace Lucene.Net.CodeAnalysis.Dev.LuceneDev1xxx
28+
{
29+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
30+
public sealed class LuceneDev1007_1008_DictionaryIndexerAnalyzer : DiagnosticAnalyzer
31+
{
32+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
33+
ImmutableArray.Create(
34+
Descriptors.LuceneDev1007_GenericDictionaryIndexerValueType,
35+
Descriptors.LuceneDev1008_GenericDictionaryIndexerReferenceType);
36+
37+
public override void Initialize(AnalysisContext context)
38+
{
39+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze);
40+
context.EnableConcurrentExecution();
41+
context.RegisterSyntaxNodeAction(AnalyzeElementAccess, SyntaxKind.ElementAccessExpression);
42+
}
43+
44+
private static void AnalyzeElementAccess(SyntaxNodeAnalysisContext ctx)
45+
{
46+
var elementAccess = (ElementAccessExpressionSyntax)ctx.Node;
47+
48+
// Skip assignment targets (setter usage does not throw).
49+
if (IsAssignmentTarget(elementAccess))
50+
return;
51+
52+
var symbolInfo = ctx.SemanticModel.GetSymbolInfo(elementAccess, ctx.CancellationToken);
53+
var property = symbolInfo.Symbol as IPropertySymbol;
54+
if (property == null || !property.IsIndexer)
55+
return;
56+
57+
var containing = property.ContainingType;
58+
if (containing == null)
59+
return;
60+
61+
if (!DictionaryTypeHelper.IsGenericDictionaryIndexer(property, containing, out var valueType))
62+
return;
63+
64+
var receiverText = elementAccess.Expression.ToString();
65+
var keyText = elementAccess.ArgumentList.ToString();
66+
var display = receiverText + keyText;
67+
68+
var descriptor = IsValueTypeForDiagnostic(valueType!)
69+
? Descriptors.LuceneDev1007_GenericDictionaryIndexerValueType
70+
: Descriptors.LuceneDev1008_GenericDictionaryIndexerReferenceType;
71+
72+
ctx.ReportDiagnostic(Diagnostic.Create(descriptor, elementAccess.GetLocation(), display));
73+
}
74+
75+
private static bool IsAssignmentTarget(ElementAccessExpressionSyntax elementAccess)
76+
{
77+
// dict[key] = value -> skip
78+
if (elementAccess.Parent is AssignmentExpressionSyntax assignment
79+
&& assignment.Left == elementAccess
80+
&& assignment.IsKind(SyntaxKind.SimpleAssignmentExpression))
81+
{
82+
return true;
83+
}
84+
return false;
85+
}
86+
87+
private static bool IsValueTypeForDiagnostic(ITypeSymbol valueType)
88+
{
89+
// Unconstrained type parameters: treat as reference-like (safer — null check may apply).
90+
if (valueType is ITypeParameterSymbol tp)
91+
{
92+
if (tp.HasValueTypeConstraint)
93+
return true;
94+
return false;
95+
}
96+
return valueType.IsValueType;
97+
}
98+
}
99+
}

0 commit comments

Comments
 (0)