Implement LuceneDev4000-4002 NoInlining Analyzers & CodeFix (#1097) (#30)
* Implement LuceneDev4000-4002 NoInlining Analyzers & CodeFix (#1097)
Adds three Roslyn analyzers under the new Performance category covering
[MethodImpl(MethodImplOptions.NoInlining)] usage:
- LuceneDev4000: Reports when NoInlining is applied to an interface or
abstract method. The MethodImpl attribute is not inherited, so the
attribute has no effect on the implementation.
- LuceneDev4001: Reports when NoInlining is applied to an empty-bodied
method. An empty body cannot appear above any frame in a stack trace,
so preventing inlining provides no benefit.
- LuceneDev4002: Reports when a method referenced by the 2-argument
StackTraceHelper.DoesStackTraceContainMethod(className, methodName)
overload is missing NoInlining. Without it, the JIT may inline the
method out of the stack trace, silently breaking the check.
A code fix is provided for 4000 and 4001 (remove the attribute). 4002
has no automated fix because the diagnostic is reported on a method
declaration triggered by an invocation in another scope, which Roslyn
treats as a non-local diagnostic and refuses to fix automatically.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Split LuceneDev4002 into its own analyzer class
LuceneDev4002 (StackTraceHelper-driven NoInlining requirement) is a
distinct rule from 4000/4001 (NoInlining-as-no-op detection): it has a
different trigger (invocation vs. method declaration) and no code fix.
Extract it into its own analyzer + test class. Shared logic for
recognising the [MethodImpl(MethodImplOptions.NoInlining)] attribute,
empty bodies, and interface/abstract methods moves into
NoInliningAttributeHelper.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add Sample project entries for LuceneDev4000-4002
Demonstrates each NoInlining diagnostic firing in the sample project:
4000 on an interface and an abstract method, 4001 on an empty-bodied
method, and 4002 on a method referenced by the 2-arg
StackTraceHelper.DoesStackTraceContainMethod overload (with a local
stub mirroring the real type so the sample compiles standalone).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* 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>
* Report LuceneDev4002 at the call site instead of the target method
Reporting on the target method declaration produced a non-local
diagnostic — the analyzer was visiting an InvocationExpressionSyntax
but raising the diagnostic on a syntax tree it had not visited. MSBuild
ran a full compilation and surfaced the warning, but the IDE's per-file
live analysis filtered it out, so the warning never appeared in the
editor.
Move the report to the DoesStackTraceContainMethod invocation. The
diagnostic is now local, shows up in the IDE, and opens the door to a
code fix at the call site. The message names the qualified target
(e.g. 'Target.Merge') so it remains clear which method needs the
attribute. Test span updated accordingly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add code fix for LuceneDev4002
Adds [MethodImpl(MethodImplOptions.NoInlining)] to the target method
referenced by a DoesStackTraceContainMethod call. The fix resolves the
target from the call's nameof() (or string-literal) arguments, locates
its declaration in source, and edits that document — even when it lives
in a different file from the call. Adds
'using System.Runtime.CompilerServices;' to the target's compilation
unit if missing, and prepends the new attribute list ahead of any
existing attributes on the method.
Promote NoInliningAttributeHelper from internal to public so the code
fix project can reuse it. Adds tests covering: target with using
already present, target without the using, and target with an existing
attribute.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Match source line-ending convention in 4002 code fix
The fix previously emitted '\n' line terminators unconditionally, which
matched local Mac checkouts (LF) but failed on Linux CI where the
checked-out source uses CRLF. Detect the existing line ending from the
target tree's trivia and reuse it so the fixed output stays consistent
with the surrounding file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Simplify 4002 code fix and rely on Formatter for layout
Replaces hand-rolled trivia construction with a single Formatter pass.
The new attribute list and using directive are built without trivia and
formatted via Formatter.FormatAsync, with the workspace NewLine option
explicitly set from the source file's existing line endings so the
output doesn't mix CRLF and LF on platforms where Environment.NewLine
disagrees with the file (e.g. CRLF source on Linux CI).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address Copilot review on LuceneDev4002
Use a symbol-based attribute check (IMethodSymbol.GetAttributes) instead
of inspecting AttributeSyntax with a SemanticModel from the wrong tree.
The previous code threw "Syntax node is not within syntax tree" whenever
the target method lived in a different file from the call site and had
any attribute. Symbol-based lookup also avoids Compilation.GetSemanticModel
in the analyzer (which trips RS1030).
Also:
- Drop the Contains("NoInlining") string fallback in the syntax-based
helper; the constant-value path already resolves MethodImplOptions.NoInlining
and the fallback false-positives on identifiers like "NotNoInlining".
- Replace the namespace walk in FindSourceTypeByName with
Compilation.GetSymbolsWithName to leverage Roslyn's symbol index.
- Update the analyzer's XML doc — the PR adds a code fix; the previous
comment claimed there was none.
- Add cross-document tests for both analyzer and code fix; the analyzer
test reproduces the SemanticModel bug above.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>This repo contains custom Roslyn analyzers that are used by the Apache Lucene.NET project to enforce code quality and consistency, as well as provide automated code fixes.
These analyzers are intended for use by contributors to the Lucene.NET project only. They are not intended for public use outside of this project. Therefore, any releases or NuGet packages produced from this repository are not official Apache Lucene.NET project artifacts, and are not subject to the same quality control or testing as the official Lucene.NET releases. They are also not subject to the release policy or voting process, as these are not intended to be used “beyond the group that owns it.”
To build the analyzers, you will need to have the .NET 8 SDK installed.
To build from the repo root, run the following command:
dotnet build
To run the tests, you can use the following command:
dotnet test
These analyzers have been tested with Visual Studio 2022 and JetBrains Rider. They should work with any IDE that supports Roslyn analyzers, but your mileage may vary. Importantly, they also work with MSBuild, so they can be used in our CI pipelines, or to help validate your changes when you build before submitting a pull request.
A Lucene.Net.CodeAnalysis.Dev.Sample project has been provided to demonstrate and debug the analyzers and code fixes in the IDE during development of them. After building, you should notice the analyzers producing the expected warnings in the Sample project. You can also debug the analyzers by setting a breakpoint in your analyzer and launching the DebugRoslynAnalyzers target of the Lucene.Net.CodeAnalysis.Dev project. You can also debug them by debugging the unit tests.
Please read and follow the Apache Lucene.NET Contributor's Guide first before proceeding further.
Before creating any analyzers, you'll need a reserved diagnostic ID for your analyzer(s). To avoid multiple contributors attempting to use the same ID at the same time, we have created a simple process to follow. It is important that you follow this process to avoid rework of your PR.
approved-rule label.main branch, as the only file in the commit. DO NOT include any other code or changes in this commit. In the event of a conflict, do not merge this file; discard your changes, pull latest, and try again. Include the issue number in your commit message.Before submitting a pull request to this repo, ensure that each analyzer you're adding:
Lucene.Net.CodeAnalysis.Dev.Sample projectLucene.Net.CodeAnalysis.Dev.Tests project, using existing styles and practices per the other unit tests there