From 4ce3e8b85e44c2cf306de529bce39dbb3214b4a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20St=C3=BChmer?= Date: Mon, 3 Aug 2026 19:33:27 +0200 Subject: [PATCH 1/2] feat(NE0002): derive namespace from folders when RootNamespace is empty FolderNamespace.TryResolve no longer requires RootNamespace. When it is absent or empty, the expected namespace is composed from the folder segments below the project directory on their own. A file in the project root with no RootNamespace anchor has nothing to compose from, so the rule stays silent there. --- .../Maintainability/FolderNamespace.cs | 49 +++++++++++++------ .../Maintainability/FolderNamespaceTests.cs | 39 +++++++++++++++ 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/NetEvolve.Analyzer/Maintainability/FolderNamespace.cs b/src/NetEvolve.Analyzer/Maintainability/FolderNamespace.cs index 6ad7f84..b7ddb74 100644 --- a/src/NetEvolve.Analyzer/Maintainability/FolderNamespace.cs +++ b/src/NetEvolve.Analyzer/Maintainability/FolderNamespace.cs @@ -9,8 +9,10 @@ namespace NetEvolve.Analyzer.Maintainability; /// /// Computes the namespace a file should declare from its location relative to the project directory, anchored -/// at the RootNamespace MSBuild property. Shared by NamespaceMatchesFolderAnalyzer (NE0002) and -/// the NE0003 nested-namespace flatten fix, so both derive the same folder-anchored value. +/// at the RootNamespace MSBuild property. When RootNamespace is absent or empty the namespace is +/// composed purely from the folder segments below the project directory. Shared by +/// NamespaceMatchesFolderAnalyzer (NE0002) and the NE0003 nested-namespace flatten fix, so both derive +/// the same folder-anchored value. /// internal static class FolderNamespace { @@ -18,9 +20,10 @@ internal static class FolderNamespace /// /// Resolves the folder-derived namespace for . Returns - /// when the anchor properties (RootNamespace, ProjectDir) are missing, the file lives outside - /// the project directory, or a folder segment is not a valid C# identifier — in all of which cases no - /// reliable mapping exists and the caller should stay silent. + /// when ProjectDir is missing, the file lives outside the project directory, a folder segment is not + /// a valid C# identifier, or the file sits in the project root with no RootNamespace anchor — in all + /// of which cases no reliable mapping exists and the caller should stay silent. RootNamespace is + /// optional: when it is absent or empty the returned namespace is the folder segments joined on their own. /// /// The global analyzer-config options exposing the build properties. /// The absolute (or project-relative) path of the source file. @@ -34,20 +37,22 @@ public static bool TryResolve(AnalyzerConfigOptions globalOptions, string filePa return false; } - if ( - !TryGetNonEmpty(globalOptions, BuildProperty.RootNamespace, out var rootNamespace) - || !TryGetNonEmpty(globalOptions, BuildProperty.ProjectDir, out var projectDir) - ) + if (!TryGetNonEmpty(globalOptions, BuildProperty.ProjectDir, out var projectDir)) { return false; } + // RootNamespace is optional: an absent or empty value means the namespace is composed purely from the + // folder segments below the project directory. + _ = globalOptions.TryGetValue(BuildProperty.RootNamespace, out var rawRootNamespace); + var rootNamespace = rawRootNamespace ?? string.Empty; + var directory = Path.GetDirectoryName(filePath); if (string.IsNullOrEmpty(directory)) { - // The file has no directory component, so it maps to the root namespace exactly. - expected = rootNamespace; - return true; + // The file has no directory component, so it maps to the root namespace exactly — but with no + // RootNamespace anchor there is nothing to compose from, so stay silent. + return TryUseRootNamespace(rootNamespace, ref expected); } if (!TryGetRelativeSegments(projectDir, directory!, out var segments)) @@ -57,9 +62,9 @@ public static bool TryResolve(AnalyzerConfigOptions globalOptions, string filePa if (segments.Count == 0) { - // The file sits directly in the project directory: it maps to the root namespace exactly. - expected = rootNamespace; - return true; + // The file sits directly in the project directory: it maps to the root namespace exactly, or stays + // silent when there is no RootNamespace anchor to compose from. + return TryUseRootNamespace(rootNamespace, ref expected); } if (segments.Any(segment => !SyntaxFacts.IsValidIdentifier(segment))) @@ -67,7 +72,19 @@ public static bool TryResolve(AnalyzerConfigOptions globalOptions, string filePa return false; } - expected = rootNamespace + "." + string.Join(".", segments); + var folderNamespace = string.Join(".", segments); + expected = rootNamespace.Length == 0 ? folderNamespace : rootNamespace + "." + folderNamespace; + return true; + } + + private static bool TryUseRootNamespace(string rootNamespace, ref string expected) + { + if (rootNamespace.Length == 0) + { + return false; + } + + expected = rootNamespace; return true; } diff --git a/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/FolderNamespaceTests.cs b/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/FolderNamespaceTests.cs index 672e581..da9d86a 100644 --- a/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/FolderNamespaceTests.cs +++ b/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/FolderNamespaceTests.cs @@ -76,6 +76,45 @@ out var expected await Assert.That(expected).IsEqualTo("Geometry.Shapes.Primitives"); } + [Test] + public async Task TryResolve_EmptyRootNamespace_SubFolders_JoinsSegmentsAlone() + { + var resolved = FolderNamespace.TryResolve( + Options(("RootNamespace", ""), ("ProjectDir", "/proj")), + "/proj/Shapes/Primitives/Circle.cs", + out var expected + ); + + await Assert.That(resolved).IsTrue(); + await Assert.That(expected).IsEqualTo("Shapes.Primitives"); + } + + [Test] + public async Task TryResolve_MissingRootNamespace_SubFolders_JoinsSegmentsAlone() + { + var resolved = FolderNamespace.TryResolve( + Options(("ProjectDir", "/proj")), + "/proj/Shapes/Circle.cs", + out var expected + ); + + await Assert.That(resolved).IsTrue(); + await Assert.That(expected).IsEqualTo("Shapes"); + } + + [Test] + public async Task TryResolve_EmptyRootNamespace_ProjectRoot_ReturnsFalse() + { + // No folders to compose from and no RootNamespace anchor: nothing reliable to map to, so stay silent. + var resolved = FolderNamespace.TryResolve( + Options(("RootNamespace", ""), ("ProjectDir", "/proj")), + "/proj/Circle.cs", + out _ + ); + + await Assert.That(resolved).IsFalse(); + } + [Test] public async Task TryResolve_OutsideProjectDir_ReturnsFalse() { From 05d88297132b16e60e8c5fb342dac705d1971a7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20St=C3=BChmer?= Date: Mon, 3 Aug 2026 19:33:37 +0200 Subject: [PATCH 2/2] refactor: share file assembly and generalize the sequential Fix-All provider Extract the file-scoped-namespace assembly shared by the NE0001 move-type and NE0003 flatten fixes into NetEvolve.Analyzer.Builders.NamespaceFileBuilder, removing the duplication between the two code fixes. Rename OneTypePerFileFixAllProvider to SequentialFixAllProvider and move it to NetEvolve.Analyzer.Providers. It is now analyzer- and code-fix-agnostic: the code fix and diagnostic ids come from the FixAllContext, and the analyzer used to re-resolve between steps is supplied per rule through the constructor. Both NE0001 and NE0003 now share it, so NE0003 gains batch Fix-All. Each rule owns a lazily created, thread-safe instance. --- .../Builders/NamespaceFileBuilder.cs | 90 ++++++++++++++++++ .../OneTypePerFileCodeFixProvider.cs | 91 ++++--------------- .../SingleNamespacePerFileCodeFixProvider.cs | 76 ++++------------ .../SequentialFixAllProvider.cs} | 72 +++++++++------ .../Maintainability/FixAllRunner.cs | 10 +- .../OneTypePerFileFixAllTests.cs | 8 +- .../SingleNamespacePerFileCodeFixTests.cs | 8 +- .../Maintainability/FixAllRunner.cs | 11 ++- .../OneTypePerFileFixAllTests.cs | 8 +- 9 files changed, 195 insertions(+), 179 deletions(-) create mode 100644 src/NetEvolve.Analyzer/Builders/NamespaceFileBuilder.cs rename src/NetEvolve.Analyzer/{Maintainability/OneTypePerFileFixAllProvider.cs => Providers/SequentialFixAllProvider.cs} (68%) diff --git a/src/NetEvolve.Analyzer/Builders/NamespaceFileBuilder.cs b/src/NetEvolve.Analyzer/Builders/NamespaceFileBuilder.cs new file mode 100644 index 0000000..fa99231 --- /dev/null +++ b/src/NetEvolve.Analyzer/Builders/NamespaceFileBuilder.cs @@ -0,0 +1,90 @@ +namespace NetEvolve.Analyzer.Builders; + +using System.Collections.Generic; +using System.Linq; +using System.Text; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +/// +/// Shared file-text assembly for the file-organization code fixes. Builds a new source file from a set of +/// top-level type declarations under a single file-scoped namespace, rendering each member at column 0 so any +/// indentation from a nested (block) namespace is dropped without re-indenting — which is what corrupted +/// multi-line string literals. Used by NE0001's move-type fix and NE0003's flatten fix so both emit identical +/// layout. +/// +internal static class NamespaceFileBuilder +{ + /// + /// Assembles the new file as text: the file-level usings, an optional file-scoped namespace with the FULL + /// dotted (omitted when empty), then every member in + /// rendered from its full text so leading doc comments travel with it. + /// + public static string Build( + CompilationUnitSyntax root, + string namespaceName, + IReadOnlyList members + ) + { + var builder = new StringBuilder(); + + foreach (var directive in root.Usings) + { + _ = builder.Append(directive.ToString()).Append('\n'); + } + + if (root.Usings.Count != 0) + { + _ = builder.Append('\n'); + } + + if (namespaceName.Length != 0) + { + _ = builder.Append("namespace ").Append(namespaceName).Append(";\n\n"); + } + + return builder.Append(string.Join("\n\n", members.Select(RenderMember))).ToString(); + } + + /// + /// Preserves the original file's final-newline style: trims trailing blank lines left by an edit, then + /// re-adds a single newline only when is . + /// + public static string WithTrailingNewline(string text, bool trailingNewline) => + trailingNewline ? text.TrimEnd() + "\n" : text.TrimEnd(); + + /// The top-level type declarations (block- or file-scoped) of . + public static IEnumerable TopLevelTypeDeclarations(CompilationUnitSyntax root) => + root.DescendantNodes().Where(IsTopLevelTypeDeclaration).Cast(); + + /// + /// Whether is a top-level type declaration — a type or delegate declared directly + /// under a namespace or the compilation unit. + /// + public static bool IsTopLevelTypeDeclaration(SyntaxNode node) => + node is BaseTypeDeclarationSyntax or DelegateDeclarationSyntax + && node.Parent is BaseNamespaceDeclarationSyntax or CompilationUnitSyntax; + + // Renders a member at column 0, keeping its leading doc comments/comments and inner blank lines but dropping + // the surrounding blank lines and the indentation it had in its original (possibly nested) context. + private static string RenderMember(MemberDeclarationSyntax member) + { + var lines = member.ToFullString().Replace("\r\n", "\n").Split('\n').ToList(); + + while (lines.Count != 0 && lines[0].Trim().Length == 0) + { + lines.RemoveAt(0); + } + + while (lines.Count != 0 && lines[lines.Count - 1].Trim().Length == 0) + { + lines.RemoveAt(lines.Count - 1); + } + + var indent = lines[0].Length - lines[0].TrimStart().Length; + return string.Join( + "\n", + lines.Select(line => line.Length >= indent ? line.Substring(indent) : line.TrimStart()) + ); + } +} diff --git a/src/NetEvolve.Analyzer/Maintainability/OneTypePerFileCodeFixProvider.cs b/src/NetEvolve.Analyzer/Maintainability/OneTypePerFileCodeFixProvider.cs index 47dc2e4..a856fbe 100644 --- a/src/NetEvolve.Analyzer/Maintainability/OneTypePerFileCodeFixProvider.cs +++ b/src/NetEvolve.Analyzer/Maintainability/OneTypePerFileCodeFixProvider.cs @@ -1,4 +1,4 @@ -namespace NetEvolve.Analyzer.Maintainability; +namespace NetEvolve.Analyzer.Maintainability; using System; using System.Collections.Generic; @@ -6,15 +6,15 @@ namespace NetEvolve.Analyzer.Maintainability; using System.Composition; using System.IO; using System.Linq; -using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Text; +using NetEvolve.Analyzer.Builders; +using NetEvolve.Analyzer.Providers; /// /// Code fix for NE0001. Offers to rename the file to match its single @@ -25,13 +25,18 @@ namespace NetEvolve.Analyzer.Maintainability; [Shared] public sealed class OneTypePerFileCodeFixProvider : CodeFixProvider { + private static readonly Lazy FixAll = new( + () => new SequentialFixAllProvider(() => new OneTypePerFileAnalyzer()), + LazyThreadSafetyMode.ExecutionAndPublication + ); + /// public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticIds.NE0001); /// // Renaming and adding documents cannot compose through the default batch fixer, so a custom provider // applies the rename/move fixes sequentially and re-resolves diagnostics between each step. - public override FixAllProvider? GetFixAllProvider() => OneTypePerFileFixAllProvider.Instance; + public override FixAllProvider? GetFixAllProvider() => FixAll.Value; /// public override async Task RegisterCodeFixesAsync(CodeFixContext context) @@ -41,7 +46,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) var root = (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false))!; var diagnostic = context.Diagnostics[0]; var declaration = (MemberDeclarationSyntax) - root.FindNode(diagnostic.Location.SourceSpan).AncestorsAndSelf().First(IsTopLevelTypeDeclaration); + root.FindNode(diagnostic.Location.SourceSpan) + .AncestorsAndSelf() + .First(NamespaceFileBuilder.IsTopLevelTypeDeclaration); var expectedName = diagnostic.Properties[OneTypePerFileAnalyzer.ExpectedFileNameProperty]!; var singleType = string.Equals( @@ -111,15 +118,16 @@ CancellationToken cancellationToken var groupGenericOverloads = ReadGroupGenericOverloads(document); var moved = MatchingDeclarations(root, declaration, groupGenericOverloads).ToList(); - // Preserve the original file's final-newline style: trim trailing blank lines left by the edit, then - // re-add a single newline only if the source had one. var endsWithNewline = root.ToFullString().EndsWith("\n", StringComparison.Ordinal); - var newText = WithTrailingNewline(BuildNewFileText(root, NamespaceName(declaration), moved), endsWithNewline); + var newText = NamespaceFileBuilder.WithTrailingNewline( + NamespaceFileBuilder.Build(root, NamespaceName(declaration), moved), + endsWithNewline + ); // Move fires only when the file holds several type groups and exactly one group is relocated, so the // original always keeps at least one type (the last remaining single type becomes a rename instead). var removed = root.RemoveNodes(moved, SyntaxRemoveOptions.KeepNoTrivia)!; - var remainingText = WithTrailingNewline(removed.ToFullString(), endsWithNewline); + var remainingText = NamespaceFileBuilder.WithTrailingNewline(removed.ToFullString(), endsWithNewline); var newName = expectedName + ".cs"; var newDocumentId = DocumentId.CreateNewId(document.Project.Id); @@ -135,62 +143,6 @@ CancellationToken cancellationToken ); } - private static string BuildNewFileText( - CompilationUnitSyntax root, - string namespaceName, - IReadOnlyList moved - ) - { - // Assemble the new file as text. Always emit a file-scoped namespace with the FULL dotted name (so a - // type lifted out of a nested block namespace keeps its real namespace, and no block re-indentation is - // needed — which is what corrupted multi-line string literals). Members are rendered from their full - // text so leading doc comments travel with them. - var builder = new StringBuilder(); - - foreach (var directive in root.Usings) - { - _ = builder.Append(directive.ToString()).Append('\n'); - } - - if (root.Usings.Count != 0) - { - _ = builder.Append('\n'); - } - - if (namespaceName.Length != 0) - { - _ = builder.Append("namespace ").Append(namespaceName).Append(";\n\n"); - } - - return builder.Append(string.Join("\n\n", moved.Select(RenderMember))).ToString(); - } - - private static string WithTrailingNewline(string text, bool trailingNewline) => - trailingNewline ? text.TrimEnd() + "\n" : text.TrimEnd(); - - // Renders a moved member at column 0, keeping its leading doc comments/comments and inner blank lines but - // dropping the surrounding blank lines and the indentation it had in its original (possibly nested) context. - private static string RenderMember(MemberDeclarationSyntax member) - { - var lines = member.ToFullString().Replace("\r\n", "\n").Split('\n').ToList(); - - while (lines.Count != 0 && lines[0].Trim().Length == 0) - { - lines.RemoveAt(0); - } - - while (lines.Count != 0 && lines[lines.Count - 1].Trim().Length == 0) - { - lines.RemoveAt(lines.Count - 1); - } - - var indent = lines[0].Length - lines[0].TrimStart().Length; - return string.Join( - "\n", - lines.Select(line => line.Length >= indent ? line.Substring(indent) : line.TrimStart()) - ); - } - private static IEnumerable MatchingDeclarations( CompilationUnitSyntax root, MemberDeclarationSyntax declaration, @@ -201,9 +153,8 @@ bool groupGenericOverloads var arity = Arity(declaration); var @namespace = NamespaceName(declaration); - return root.DescendantNodes() - .Where(IsTopLevelTypeDeclaration) - .Cast() + return NamespaceFileBuilder + .TopLevelTypeDeclarations(root) .Where(member => string.Equals(Identifier(member).ValueText, name, StringComparison.Ordinal) && string.Equals(NamespaceName(member), @namespace, StringComparison.Ordinal) @@ -224,10 +175,6 @@ private static string SiblingPath(string currentPath, string newName) return string.IsNullOrEmpty(directory) ? newName : Path.Combine(directory, newName); } - private static bool IsTopLevelTypeDeclaration(SyntaxNode node) => - node is BaseTypeDeclarationSyntax or DelegateDeclarationSyntax - && node.Parent is BaseNamespaceDeclarationSyntax or CompilationUnitSyntax; - private static SyntaxToken Identifier(MemberDeclarationSyntax member) => member is BaseTypeDeclarationSyntax type ? type.Identifier : ((DelegateDeclarationSyntax)member).Identifier; diff --git a/src/NetEvolve.Analyzer/Maintainability/SingleNamespacePerFileCodeFixProvider.cs b/src/NetEvolve.Analyzer/Maintainability/SingleNamespacePerFileCodeFixProvider.cs index 9eb30db..91f0197 100644 --- a/src/NetEvolve.Analyzer/Maintainability/SingleNamespacePerFileCodeFixProvider.cs +++ b/src/NetEvolve.Analyzer/Maintainability/SingleNamespacePerFileCodeFixProvider.cs @@ -1,11 +1,10 @@ -namespace NetEvolve.Analyzer.Maintainability; +namespace NetEvolve.Analyzer.Maintainability; using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; using System.Linq; -using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; @@ -13,6 +12,8 @@ namespace NetEvolve.Analyzer.Maintainability; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Text; +using NetEvolve.Analyzer.Builders; +using NetEvolve.Analyzer.Providers; /// /// Code fix for NE0003. Offered only for the nested shape @@ -23,12 +24,18 @@ namespace NetEvolve.Analyzer.Maintainability; [Shared] public sealed class SingleNamespacePerFileCodeFixProvider : CodeFixProvider { + private static readonly Lazy FixAll = new( + () => new SequentialFixAllProvider(() => new SingleNamespacePerFileAnalyzer()), + LazyThreadSafetyMode.ExecutionAndPublication + ); + /// public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticIds.NE0003); /// - // A whole-file rewrite does not compose safely across many diagnostics, so no batch fix-all. - public override FixAllProvider? GetFixAllProvider() => null; + // Flattening a nested file is a whole-file rewrite; the sequential fix-all re-resolves after each file so + // a batch across several files converges to a fixed point. + public override FixAllProvider? GetFixAllProvider() => FixAll.Value; /// public override async Task RegisterCodeFixesAsync(CodeFixContext context) @@ -73,10 +80,12 @@ CancellationToken cancellationToken { var root = (CompilationUnitSyntax)(await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false))!; - // Preserve the original file's final-newline style: trim trailing blank lines left by the rewrite, then - // re-add a single newline only if the source had one. + var members = NamespaceFileBuilder.TopLevelTypeDeclarations(root).ToList(); var endsWithNewline = root.ToFullString().EndsWith("\n", StringComparison.Ordinal); - var newText = WithTrailingNewline(BuildNewFileText(root, target), endsWithNewline); + var newText = NamespaceFileBuilder.WithTrailingNewline( + NamespaceFileBuilder.Build(root, target, members), + endsWithNewline + ); return document.WithText(SourceText.From(newText)); } @@ -91,59 +100,6 @@ private static string ResolveTargetNamespace(Document document, BaseNamespaceDec return FolderNamespace.TryResolve(options, filePath, out var expected) ? expected : NamespaceChain(declaration); } - private static string BuildNewFileText(CompilationUnitSyntax root, string namespaceName) - { - // Assemble the new file as text: the file-level usings, a single file-scoped namespace, then every - // top-level type rendered at column 0 so its (possibly nested) indentation is dropped and leading doc - // comments travel with it. - var builder = new StringBuilder(); - - foreach (var directive in root.Usings) - { - _ = builder.Append(directive.ToString()).Append('\n'); - } - - if (root.Usings.Count != 0) - { - _ = builder.Append('\n'); - } - - _ = builder.Append("namespace ").Append(namespaceName).Append(";\n\n"); - - var members = root.DescendantNodes().Where(IsTopLevelTypeDeclaration).Cast(); - return builder.Append(string.Join("\n\n", members.Select(RenderMember))).ToString(); - } - - private static string WithTrailingNewline(string text, bool trailingNewline) => - trailingNewline ? text.TrimEnd() + "\n" : text.TrimEnd(); - - // Renders a top-level member at column 0, keeping its leading doc comments/comments and inner blank lines but - // dropping the surrounding blank lines and the indentation it had in its original (nested) context. - private static string RenderMember(MemberDeclarationSyntax member) - { - var lines = member.ToFullString().Replace("\r\n", "\n").Split('\n').ToList(); - - while (lines.Count != 0 && lines[0].Trim().Length == 0) - { - lines.RemoveAt(0); - } - - while (lines.Count != 0 && lines[lines.Count - 1].Trim().Length == 0) - { - lines.RemoveAt(lines.Count - 1); - } - - var indent = lines[0].Length - lines[0].TrimStart().Length; - return string.Join( - "\n", - lines.Select(line => line.Length >= indent ? line.Substring(indent) : line.TrimStart()) - ); - } - - private static bool IsTopLevelTypeDeclaration(SyntaxNode node) => - node is BaseTypeDeclarationSyntax or DelegateDeclarationSyntax - && node.Parent is BaseNamespaceDeclarationSyntax or CompilationUnitSyntax; - private static string NamespaceChain(SyntaxNode node) { var segments = new List(); diff --git a/src/NetEvolve.Analyzer/Maintainability/OneTypePerFileFixAllProvider.cs b/src/NetEvolve.Analyzer/Providers/SequentialFixAllProvider.cs similarity index 68% rename from src/NetEvolve.Analyzer/Maintainability/OneTypePerFileFixAllProvider.cs rename to src/NetEvolve.Analyzer/Providers/SequentialFixAllProvider.cs index 5f20928..0091e15 100644 --- a/src/NetEvolve.Analyzer/Maintainability/OneTypePerFileFixAllProvider.cs +++ b/src/NetEvolve.Analyzer/Providers/SequentialFixAllProvider.cs @@ -1,4 +1,4 @@ -namespace NetEvolve.Analyzer.Maintainability; +namespace NetEvolve.Analyzer.Providers; using System; using System.Collections.Generic; @@ -12,15 +12,25 @@ namespace NetEvolve.Analyzer.Maintainability; using Microsoft.CodeAnalysis.Diagnostics; /// -/// Custom fix-all for NE0001. The single-diagnostic fix uses -/// solution-level operations (WithDocumentName, AddDocument) and overlapping edits to a single -/// multi-type file, none of which the default batch fixer can compose. Instead this provider applies the -/// rename/move fixes one at a time and re-resolves diagnostics between each step, iterating to a fixed point. +/// A custom fix-all that applies a code fix one diagnostic at a time, re-resolving diagnostics from the +/// accumulating solution after each step until it reaches a fixed point. Some fixes cannot compose through the +/// default batch fixer: NE0001's rename/move uses solution-level operations (WithDocumentName, +/// AddDocument) and overlapping edits to a single file, and NE0003's flatten is a whole-file rewrite. The +/// code fix to run and the diagnostics to match are taken from the ; the analyzer +/// used to re-resolve between steps is supplied per rule through the constructor. /// -internal sealed class OneTypePerFileFixAllProvider : FixAllProvider +internal sealed class SequentialFixAllProvider : FixAllProvider { - /// The shared instance returned by . - public static OneTypePerFileFixAllProvider Instance { get; } = new OneTypePerFileFixAllProvider(); + private readonly Func _analyzerFactory; + + /// + /// Creates the provider for a single rule. + /// + /// + /// Produces a fresh instance of the rule's analyzer, run after each step to re-resolve diagnostics against + /// the accumulated solution. + /// + public SequentialFixAllProvider(Func analyzerFactory) => _analyzerFactory = analyzerFactory; /// public override IEnumerable GetSupportedFixAllScopes() => @@ -40,9 +50,9 @@ public override IEnumerable GetSupportedFixAllScopes() => } return CodeAction.Create( - $"Fix all '{DiagnosticIds.NE0001}' occurrences", + $"Fix all '{string.Join("', '", fixAllContext.DiagnosticIds)}' occurrences", cancellationToken => FixAllAsync(fixAllContext, cancellationToken), - equivalenceKey: nameof(OneTypePerFileFixAllProvider) + equivalenceKey: nameof(SequentialFixAllProvider) ); } @@ -77,10 +87,10 @@ private static async Task HasFixableDiagnosticsAsync(FixAllContext fixAllC return false; } - // Applies one rename/move at a time, re-resolving diagnostics from the accumulating solution after each - // step. Convergence and the move->rename flip fall out of the re-resolution; the collision case (target - // name equals the current file) registers no action and is therefore passed over without failing. - private static async Task FixAllAsync(FixAllContext fixAllContext, CancellationToken cancellationToken) + // Applies one fix at a time, re-resolving diagnostics from the accumulating solution after each step. + // Convergence (and NE0001's move->rename flip) falls out of the re-resolution; a diagnostic whose fix + // registers no action (e.g. NE0001's collision case) is passed over without failing. + private async Task FixAllAsync(FixAllContext fixAllContext, CancellationToken cancellationToken) { var solution = fixAllContext.Solution; var scope = fixAllContext.Scope; @@ -91,7 +101,7 @@ private static async Task FixAllAsync(FixAllContext fixAllContext, Can { cancellationToken.ThrowIfCancellationRequested(); - var next = await TryApplyOneAsync(solution, scope, documentId, projectId, cancellationToken) + var next = await TryApplyOneAsync(fixAllContext, solution, scope, documentId, projectId, cancellationToken) .ConfigureAwait(false); if (next is null) { @@ -102,7 +112,8 @@ private static async Task FixAllAsync(FixAllContext fixAllContext, Can } } - private static async Task TryApplyOneAsync( + private async Task TryApplyOneAsync( + FixAllContext fixAllContext, Solution solution, FixAllScope scope, DocumentId? documentId, @@ -112,7 +123,8 @@ CancellationToken cancellationToken { foreach (var id in TargetDocumentIds(solution, scope, documentId, projectId)) { - var changed = await TryFixDocumentAsync(solution, id, cancellationToken).ConfigureAwait(false); + var changed = await TryFixDocumentAsync(fixAllContext, solution, id, cancellationToken) + .ConfigureAwait(false); if (changed is not null) { return changed; @@ -122,9 +134,10 @@ CancellationToken cancellationToken return null; } - // Runs the analyzer over the document's project, then applies the first NE0001 diagnostic in the document - // (by source order) that yields an action. Returns null when the document has no applicable fix. - private static async Task TryFixDocumentAsync( + // Re-resolves the rule's diagnostics in the document, then applies the first one (by source order) that + // yields an action through the context's code fix provider. Returns null when the document has no fix. + private async Task TryFixDocumentAsync( + FixAllContext fixAllContext, Solution solution, DocumentId id, CancellationToken cancellationToken @@ -133,9 +146,9 @@ CancellationToken cancellationToken // The id always comes from TargetDocumentIds enumerating the current solution, so the document exists. var document = solution.GetDocument(id)!; - var diagnostics = await ResolveDiagnosticsAsync(solution, document, id, cancellationToken) + var diagnostics = await ResolveDiagnosticsAsync(fixAllContext, solution, document, id, cancellationToken) .ConfigureAwait(false); - var fixProvider = new OneTypePerFileCodeFixProvider(); + var fixProvider = fixAllContext.CodeFixProvider; foreach (var diagnostic in diagnostics) { @@ -150,9 +163,10 @@ CancellationToken cancellationToken return null; } - // The NE0001 diagnostics located in the document, ordered by source position, resolved from a fresh run of - // the analyzer over the current project so each pass sees the accumulated edits. - private static async Task> ResolveDiagnosticsAsync( + // The rule's diagnostics located in the document, ordered by source position, resolved from a fresh run of + // the per-rule analyzer over the current project so each pass sees the accumulated edits. + private async Task> ResolveDiagnosticsAsync( + FixAllContext fixAllContext, Solution solution, Document document, DocumentId id, @@ -166,7 +180,7 @@ CancellationToken cancellationToken // GetAnalyzerDiagnosticsAsync below. #pragma warning disable S8949 var withAnalyzers = compilation.WithAnalyzers( - ImmutableArray.Create(new OneTypePerFileAnalyzer()), + ImmutableArray.Create(_analyzerFactory()), project.AnalyzerOptions ); #pragma warning restore S8949 @@ -174,7 +188,7 @@ CancellationToken cancellationToken var diagnostics = await withAnalyzers.GetAnalyzerDiagnosticsAsync(cancellationToken).ConfigureAwait(false); return diagnostics - .Where(diagnostic => string.Equals(diagnostic.Id, DiagnosticIds.NE0001, StringComparison.Ordinal)) + .Where(diagnostic => fixAllContext.DiagnosticIds.Contains(diagnostic.Id)) .Where(diagnostic => diagnostic.Location.SourceTree is not null && solution.GetDocument(diagnostic.Location.SourceTree)?.Id == id @@ -184,9 +198,9 @@ diagnostic.Location.SourceTree is not null } // Registers the single-diagnostic fix and applies its first change. Returns null when no action is offered - // (the collision case, where the target file equals the current file), skipping without failing the batch. + // (e.g. NE0001's collision case, where the target file equals the current file), skipping without failing. private static async Task TryApplyFixAsync( - OneTypePerFileCodeFixProvider fixProvider, + CodeFixProvider fixProvider, Document document, Diagnostic diagnostic, CancellationToken cancellationToken diff --git a/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/FixAllRunner.cs b/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/FixAllRunner.cs index b51f1ae..55b586f 100644 --- a/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/FixAllRunner.cs +++ b/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/FixAllRunner.cs @@ -15,9 +15,10 @@ namespace NetEvolve.Analyzer.Tests.Integration.Maintainability; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Text; using NetEvolve.Analyzer.Maintainability; +using NetEvolve.Analyzer.Providers; /// -/// Drives end-to-end through a real , +/// Drives end-to-end through a real , /// mirroring but invoking the fix-all pipeline the IDE uses: it builds a project /// from named documents, constructs a for the requested /// backed by a diagnostic provider that runs , obtains the fix-all @@ -101,7 +102,8 @@ CancellationToken cancellationToken var project = solution.GetProject(projectId)!; var context = await CreateContextAsync(project, scope, cancellationToken).ConfigureAwait(false); - var action = await OneTypePerFileFixAllProvider.Instance.GetFixAsync(context).ConfigureAwait(false); + var fixAllProvider = new OneTypePerFileCodeFixProvider().GetFixAllProvider()!; + var action = await fixAllProvider.GetFixAsync(context).ConfigureAwait(false); if (action is null) { return solution; @@ -129,7 +131,7 @@ CancellationToken cancellationToken project, fixProvider, scope, - nameof(OneTypePerFileFixAllProvider), + nameof(SequentialFixAllProvider), diagnosticIds, diagnosticProvider, cancellationToken @@ -141,7 +143,7 @@ CancellationToken cancellationToken trigger, fixProvider, scope, - nameof(OneTypePerFileFixAllProvider), + nameof(SequentialFixAllProvider), diagnosticIds, diagnosticProvider, cancellationToken diff --git a/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/OneTypePerFileFixAllTests.cs b/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/OneTypePerFileFixAllTests.cs index 9ace509..0995e52 100644 --- a/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/OneTypePerFileFixAllTests.cs +++ b/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/OneTypePerFileFixAllTests.cs @@ -5,6 +5,7 @@ namespace NetEvolve.Analyzer.Tests.Integration.Maintainability; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeFixes; using NetEvolve.Analyzer.Maintainability; +using NetEvolve.Analyzer.Providers; using TUnit.Assertions; using TUnit.Assertions.Extensions; using TUnit.Core; @@ -153,7 +154,7 @@ public async Task GetFixAsync_NullContext_ThrowsArgumentNullException() try { - _ = await OneTypePerFileFixAllProvider.Instance.GetFixAsync(null!).ConfigureAwait(false); + _ = await new OneTypePerFileCodeFixProvider().GetFixAllProvider()!.GetFixAsync(null!).ConfigureAwait(false); } catch (ArgumentNullException exception) { @@ -183,7 +184,7 @@ public sealed class Circle { } [Test] public async Task GetSupportedFixAllScopes_AreDocumentProjectSolution() { - var scopes = OneTypePerFileFixAllProvider.Instance.GetSupportedFixAllScopes().ToList(); + var scopes = new OneTypePerFileCodeFixProvider().GetFixAllProvider()!.GetSupportedFixAllScopes().ToList(); await Assert.That(scopes).Contains(FixAllScope.Document); await Assert.That(scopes).Contains(FixAllScope.Project); @@ -195,6 +196,7 @@ public async Task GetFixAllProvider_ReturnsCustomProvider() { var provider = new OneTypePerFileCodeFixProvider().GetFixAllProvider(); - await Assert.That(provider).IsSameReferenceAs(OneTypePerFileFixAllProvider.Instance); + await Assert.That(provider).IsSameReferenceAs(new OneTypePerFileCodeFixProvider().GetFixAllProvider()); + await Assert.That(provider!.GetType()).IsEqualTo(typeof(SequentialFixAllProvider)); } } diff --git a/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/SingleNamespacePerFileCodeFixTests.cs b/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/SingleNamespacePerFileCodeFixTests.cs index 0d64ceb..fd9b59c 100644 --- a/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/SingleNamespacePerFileCodeFixTests.cs +++ b/test/NetEvolve.Analyzer.Tests.Integration/Maintainability/SingleNamespacePerFileCodeFixTests.cs @@ -3,6 +3,7 @@ namespace NetEvolve.Analyzer.Tests.Integration.Maintainability; using System; using System.Threading.Tasks; using NetEvolve.Analyzer.Maintainability; +using NetEvolve.Analyzer.Providers; using TUnit.Assertions; using TUnit.Assertions.Extensions; using TUnit.Core; @@ -163,10 +164,11 @@ public async Task Nested_NoTrailingNewline_PreservesStyleAndUsesFolderNamespace( } [Test] - public async Task GetFixAllProvider_ReturnsNull() + public async Task GetFixAllProvider_ReturnsCustomProvider() { - var provider = new SingleNamespacePerFileCodeFixProvider(); + var provider = new SingleNamespacePerFileCodeFixProvider().GetFixAllProvider(); - await Assert.That(provider.GetFixAllProvider()).IsNull(); + await Assert.That(provider).IsSameReferenceAs(new SingleNamespacePerFileCodeFixProvider().GetFixAllProvider()); + await Assert.That(provider!.GetType()).IsEqualTo(typeof(SequentialFixAllProvider)); } } diff --git a/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/FixAllRunner.cs b/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/FixAllRunner.cs index 8ee1932..80a9e8c 100644 --- a/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/FixAllRunner.cs +++ b/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/FixAllRunner.cs @@ -5,7 +5,6 @@ namespace NetEvolve.Analyzer.Tests.Unit.Maintainability; using System.Collections.Immutable; using System.IO; using System.Linq; -using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; @@ -16,9 +15,10 @@ namespace NetEvolve.Analyzer.Tests.Unit.Maintainability; using Microsoft.CodeAnalysis.Text; using NetEvolve.Analyzer; using NetEvolve.Analyzer.Maintainability; +using NetEvolve.Analyzer.Providers; /// -/// Drives end-to-end through a real from +/// Drives end-to-end through a real from /// the unit suite as well, so the fix-all pipeline is exercised by both the unit and integration flags (a line /// covered by only one flag counts as a partial against the patch coverage gate). It builds a project from named /// documents, constructs a for the requested backed by a @@ -85,7 +85,8 @@ CancellationToken cancellationToken var project = solution.GetProject(projectId)!; var context = await CreateContextAsync(project, scope, cancellationToken).ConfigureAwait(false); - var action = await OneTypePerFileFixAllProvider.Instance.GetFixAsync(context).ConfigureAwait(false); + var fixAllProvider = new OneTypePerFileCodeFixProvider().GetFixAllProvider()!; + var action = await fixAllProvider.GetFixAsync(context).ConfigureAwait(false); if (action is null) { return solution; @@ -111,7 +112,7 @@ CancellationToken cancellationToken project, fixProvider, scope, - nameof(OneTypePerFileFixAllProvider), + nameof(SequentialFixAllProvider), diagnosticIds, diagnosticProvider, cancellationToken @@ -123,7 +124,7 @@ CancellationToken cancellationToken trigger, fixProvider, scope, - nameof(OneTypePerFileFixAllProvider), + nameof(SequentialFixAllProvider), diagnosticIds, diagnosticProvider, cancellationToken diff --git a/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/OneTypePerFileFixAllTests.cs b/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/OneTypePerFileFixAllTests.cs index 465ad37..21b84b3 100644 --- a/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/OneTypePerFileFixAllTests.cs +++ b/test/NetEvolve.Analyzer.Tests.Unit/Maintainability/OneTypePerFileFixAllTests.cs @@ -5,6 +5,7 @@ namespace NetEvolve.Analyzer.Tests.Unit.Maintainability; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeFixes; using NetEvolve.Analyzer.Maintainability; +using NetEvolve.Analyzer.Providers; using TUnit.Assertions; using TUnit.Assertions.Extensions; using TUnit.Core; @@ -123,7 +124,7 @@ public sealed class Item { } [Test] public async Task GetSupportedFixAllScopes_AreDocumentProjectSolution() { - var scopes = OneTypePerFileFixAllProvider.Instance.GetSupportedFixAllScopes().ToList(); + var scopes = new OneTypePerFileCodeFixProvider().GetFixAllProvider()!.GetSupportedFixAllScopes().ToList(); await Assert.That(scopes).Contains(FixAllScope.Document); await Assert.That(scopes).Contains(FixAllScope.Project); @@ -135,7 +136,8 @@ public async Task GetFixAllProvider_ReturnsCustomProvider() { var provider = new OneTypePerFileCodeFixProvider().GetFixAllProvider(); - await Assert.That(provider).IsSameReferenceAs(OneTypePerFileFixAllProvider.Instance); + await Assert.That(provider).IsSameReferenceAs(new OneTypePerFileCodeFixProvider().GetFixAllProvider()); + await Assert.That(provider!.GetType()).IsEqualTo(typeof(SequentialFixAllProvider)); } [Test] @@ -145,7 +147,7 @@ public async Task GetFixAsync_NullContext_ThrowsArgumentNullException() try { - _ = await OneTypePerFileFixAllProvider.Instance.GetFixAsync(null!).ConfigureAwait(false); + _ = await new OneTypePerFileCodeFixProvider().GetFixAllProvider()!.GetFixAsync(null!).ConfigureAwait(false); } catch (ArgumentNullException exception) {