Skip to content

Commit 0c6c007

Browse files
paulirwinclaude
paulirwin
and
claude
committed
Preserve leading comments and trim attribute indent in 4000/4001 fix
The previous remove-attribute logic used SyntaxRemoveOptions.KeepNoTrivia which discarded any comments or blank lines that preceded the attribute, or KeepLeadingTrivia which left a stray indent on the next line. Replace with an approach that operates on the parent member declaration directly: strip the attribute list while moving its leading comments (minus the final whitespace block, which was just the attribute's own indent) onto the next member's leading trivia. Adds tests covering: leading comment preservation, removing one of several attributes inside a single attribute list, and removing one attribute list among multiple sibling lists. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0d017c9 commit 0c6c007

2 files changed

Lines changed: 220 additions & 10 deletions

File tree

‎src/Lucene.Net.CodeAnalysis.Dev.CodeFixes/LuceneDev4xxx/LuceneDev4000_4001_NoInliningOnNoOpCodeFixProvider.cs‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@
1818

1919
using System.Collections.Immutable;
2020
using System.Composition;
21+
using System.Linq;
2122
using System.Threading;
2223
using System.Threading.Tasks;
2324
using Lucene.Net.CodeAnalysis.Dev.Utility;
2425
using Microsoft.CodeAnalysis;
2526
using Microsoft.CodeAnalysis.CodeActions;
2627
using Microsoft.CodeAnalysis.CodeFixes;
28+
using Microsoft.CodeAnalysis.CSharp;
2729
using Microsoft.CodeAnalysis.CSharp.Syntax;
2830

2931
namespace Lucene.Net.CodeAnalysis.Dev.CodeFixes.LuceneDev4xxx
@@ -71,22 +73,60 @@ private static async Task<Document> RemoveAttributeAsync(
7173
if (root is null)
7274
return document;
7375

74-
if (attribute.Parent is AttributeListSyntax attrList)
76+
if (attribute.Parent is not AttributeListSyntax attrList)
77+
return document;
78+
79+
SyntaxNode newRoot;
80+
if (attrList.Attributes.Count > 1)
7581
{
76-
SyntaxNode newRoot;
77-
if (attrList.Attributes.Count == 1)
78-
{
79-
newRoot = root.RemoveNode(attrList, SyntaxRemoveOptions.KeepNoTrivia)!;
80-
}
81-
else
82+
// [Foo, MethodImpl(NoInlining), Bar] → [Foo, Bar]
83+
var newList = attrList.WithAttributes(attrList.Attributes.Remove(attribute));
84+
newRoot = root.ReplaceNode(attrList, newList);
85+
return document.WithSyntaxRoot(newRoot);
86+
}
87+
88+
// Removing the whole [ ... ] list.
89+
//
90+
// Trivia handling: the attribute list's leading trivia is typically
91+
// (newline)(indent)[comment(newline)(indent)]*. We want to keep any
92+
// comments (and the newline that ends each one) but drop the final
93+
// whitespace block — which is just the indentation for the now-removed
94+
// attribute. The token following the list already carries its own
95+
// newline+indent, so leaving that whitespace in would double-indent the
96+
// next line. We move the trimmed trivia onto the next token.
97+
var leading = attrList.GetLeadingTrivia();
98+
int trim = leading.Count;
99+
while (trim > 0 && leading[trim - 1].IsKind(SyntaxKind.WhitespaceTrivia))
100+
{
101+
trim--;
102+
}
103+
var triviaToKeep = SyntaxFactory.TriviaList(leading.Take(trim));
104+
105+
// Locate the parent that owns this attribute list. Use the parent's
106+
// AttributeLists collection (e.g. on MethodDeclarationSyntax) so that
107+
// removing the list and re-attaching trivia happens in a single step
108+
// and preserves indentation of the surrounding declaration.
109+
if (attrList.Parent is MemberDeclarationSyntax member)
110+
{
111+
var newAttrLists = member.AttributeLists.Remove(attrList);
112+
MemberDeclarationSyntax newMember = member.WithAttributeLists(newAttrLists);
113+
114+
// Prepend the trivia we want to keep (e.g. comments) to the new
115+
// first token of the member declaration.
116+
if (triviaToKeep.Count > 0)
82117
{
83-
var newList = attrList.WithAttributes(attrList.Attributes.Remove(attribute));
84-
newRoot = root.ReplaceNode(attrList, newList);
118+
var firstToken = newMember.GetFirstToken();
119+
var combined = triviaToKeep.AddRange(firstToken.LeadingTrivia);
120+
newMember = newMember.ReplaceToken(firstToken, firstToken.WithLeadingTrivia(combined));
85121
}
122+
123+
newRoot = root.ReplaceNode(member, newMember);
86124
return document.WithSyntaxRoot(newRoot);
87125
}
88126

89-
return document;
127+
// Fallback: just remove the list, dropping its trivia.
128+
newRoot = root.RemoveNode(attrList, SyntaxRemoveOptions.KeepNoTrivia)!;
129+
return document.WithSyntaxRoot(newRoot);
90130
}
91131
}
92132
}

‎tests/Lucene.Net.CodeAnalysis.Dev.CodeFixes.Tests/LuceneDev4xxx/TestLuceneDev4000_4001_NoInliningOnNoOpCodeFixProvider.cs‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,5 +115,175 @@ public void DoWork()
115115

116116
await test.RunAsync();
117117
}
118+
119+
// -----------------------------------------------------------------
120+
// Regression: removing the attribute must preserve comments and
121+
// blank lines that precede it.
122+
// -----------------------------------------------------------------
123+
124+
[Test]
125+
public async Task Fix_PreservesLeadingCommentBeforeAttribute()
126+
{
127+
var testCode = @"
128+
using System.Runtime.CompilerServices;
129+
130+
public class Sample
131+
{
132+
// Important comment about the method.
133+
[MethodImpl(MethodImplOptions.NoInlining)]
134+
public void DoWork()
135+
{
136+
}
137+
}";
138+
139+
var fixedCode = @"
140+
using System.Runtime.CompilerServices;
141+
142+
public class Sample
143+
{
144+
// Important comment about the method.
145+
public void DoWork()
146+
{
147+
}
148+
}";
149+
150+
var expected = new DiagnosticResult(Descriptors.LuceneDev4001_NoInliningOnEmptyMethod)
151+
.WithSeverity(DiagnosticSeverity.Warning)
152+
.WithSpan(7, 6, 7, 46)
153+
.WithArguments("DoWork");
154+
155+
var test = new InjectableCodeFixTest(
156+
() => new LuceneDev4000_4001_NoInliningOnNoOpAnalyzer(),
157+
() => new LuceneDev4000_4001_NoInliningOnNoOpCodeFixProvider())
158+
{
159+
TestCode = testCode,
160+
FixedCode = fixedCode,
161+
ExpectedDiagnostics = { expected }
162+
};
163+
164+
await test.RunAsync();
165+
}
166+
167+
// -----------------------------------------------------------------
168+
// Multiple attributes: only the [MethodImpl(NoInlining)] attribute
169+
// should be removed; siblings must remain intact.
170+
// -----------------------------------------------------------------
171+
172+
[Test]
173+
public async Task Fix_RemovesOnlyTargetAttribute_WithinSingleAttributeList()
174+
{
175+
// [A, MethodImpl(NoInlining), B] → [A, B]
176+
var testCode = @"
177+
using System;
178+
using System.Runtime.CompilerServices;
179+
180+
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
181+
public class FooAttribute : Attribute { }
182+
183+
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
184+
public class BarAttribute : Attribute { }
185+
186+
public class Sample
187+
{
188+
[Foo, MethodImpl(MethodImplOptions.NoInlining), Bar]
189+
public void DoWork()
190+
{
191+
}
192+
}";
193+
194+
var fixedCode = @"
195+
using System;
196+
using System.Runtime.CompilerServices;
197+
198+
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
199+
public class FooAttribute : Attribute { }
200+
201+
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
202+
public class BarAttribute : Attribute { }
203+
204+
public class Sample
205+
{
206+
[Foo, Bar]
207+
public void DoWork()
208+
{
209+
}
210+
}";
211+
212+
var expected = new DiagnosticResult(Descriptors.LuceneDev4001_NoInliningOnEmptyMethod)
213+
.WithSeverity(DiagnosticSeverity.Warning)
214+
.WithSpan(13, 11, 13, 51)
215+
.WithArguments("DoWork");
216+
217+
var test = new InjectableCodeFixTest(
218+
() => new LuceneDev4000_4001_NoInliningOnNoOpAnalyzer(),
219+
() => new LuceneDev4000_4001_NoInliningOnNoOpCodeFixProvider())
220+
{
221+
TestCode = testCode,
222+
FixedCode = fixedCode,
223+
ExpectedDiagnostics = { expected }
224+
};
225+
226+
await test.RunAsync();
227+
}
228+
229+
[Test]
230+
public async Task Fix_RemovesOnlyTargetAttributeList_AmongMultipleLists()
231+
{
232+
// [A] [MethodImpl(NoInlining)] [B] → [A] [B]
233+
var testCode = @"
234+
using System;
235+
using System.Runtime.CompilerServices;
236+
237+
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
238+
public class FooAttribute : Attribute { }
239+
240+
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
241+
public class BarAttribute : Attribute { }
242+
243+
public class Sample
244+
{
245+
[Foo]
246+
[MethodImpl(MethodImplOptions.NoInlining)]
247+
[Bar]
248+
public void DoWork()
249+
{
250+
}
251+
}";
252+
253+
var fixedCode = @"
254+
using System;
255+
using System.Runtime.CompilerServices;
256+
257+
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
258+
public class FooAttribute : Attribute { }
259+
260+
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
261+
public class BarAttribute : Attribute { }
262+
263+
public class Sample
264+
{
265+
[Foo]
266+
[Bar]
267+
public void DoWork()
268+
{
269+
}
270+
}";
271+
272+
var expected = new DiagnosticResult(Descriptors.LuceneDev4001_NoInliningOnEmptyMethod)
273+
.WithSeverity(DiagnosticSeverity.Warning)
274+
.WithSpan(14, 6, 14, 46)
275+
.WithArguments("DoWork");
276+
277+
var test = new InjectableCodeFixTest(
278+
() => new LuceneDev4000_4001_NoInliningOnNoOpAnalyzer(),
279+
() => new LuceneDev4000_4001_NoInliningOnNoOpCodeFixProvider())
280+
{
281+
TestCode = testCode,
282+
FixedCode = fixedCode,
283+
ExpectedDiagnostics = { expected }
284+
};
285+
286+
await test.RunAsync();
287+
}
118288
}
119289
}

0 commit comments

Comments
 (0)