From 9ff8eb6efff5522c241b5b69036fd3be8f71db90 Mon Sep 17 00:00:00 2001 From: "fabien.menager" Date: Mon, 16 Mar 2026 18:47:43 +0100 Subject: [PATCH 01/11] Add a code fixer and a code refactorer to transform factory methods into constructors --- samples/ReadmeSample/ReadmeSample.csproj | 1 + ...ctoryMethodToConstructorCodeFixProvider.cs | 70 +++++ ...hodToConstructorCodeRefactoringProvider.cs | 73 +++++ .../FactoryMethodTransformationHelper.cs | 279 ++++++++++++++++++ .../AnalyzerReleases.Shipped.md | 1 + .../Infrastructure/Diagnostics.cs | 9 + .../ProjectionExpressionGenerator.cs | 63 ++++ .../CodeFixTestBase.cs | 2 +- ...essConstructor_WhenNoneExists.verified.txt | 14 + ...onstructor_WhenAlreadyPresent.verified.txt | 11 + ...x_PreservesProjectableOptions.verified.txt | 14 + ...Fix_SimpleStaticFactoryMethod.verified.txt | 16 + ...MethodToConstructorCodeFixProviderTests.cs | 136 +++++++++ ...essConstructor_WhenNoneExists.verified.txt | 14 + ...onstructor_WhenAlreadyPresent.verified.txt | 11 + ...ultipleInitializerAssignments.verified.txt | 18 ++ ...r_PreservesProjectableOptions.verified.txt | 14 + ...nstructor_SimpleFactoryMethod.verified.txt | 16 + ...gMembers_PreservesMemberOrder.verified.txt | 17 ++ ...ConstructorCodeRefactoringProviderTests.cs | 243 +++++++++++++++ .../RefactoringTestBase.cs | 78 +++++ .../FactoryMethodDiagnosticTests.cs | 220 ++++++++++++++ 22 files changed, 1319 insertions(+), 1 deletion(-) create mode 100644 src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeFixProvider.cs create mode 100644 src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs create mode 100644 src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.cs create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.cs create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/RefactoringTestBase.cs create mode 100644 tests/EntityFrameworkCore.Projectables.Generator.Tests/FactoryMethodDiagnosticTests.cs diff --git a/samples/ReadmeSample/ReadmeSample.csproj b/samples/ReadmeSample/ReadmeSample.csproj index b1dc659..72664f6 100644 --- a/samples/ReadmeSample/ReadmeSample.csproj +++ b/samples/ReadmeSample/ReadmeSample.csproj @@ -13,6 +13,7 @@ + diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeFixProvider.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeFixProvider.cs new file mode 100644 index 0000000..1ca4cb1 --- /dev/null +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeFixProvider.cs @@ -0,0 +1,70 @@ +using System.Collections.Immutable; +using System.Composition; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace EntityFrameworkCore.Projectables.CodeFixes; + +/// +/// Code fix provider for EFP0012. +/// Offers two fixes on a [Projectable] factory method that can be a constructor: +/// +/// Convert the factory method to a constructor (current document). +/// Convert the factory method to a constructor and update all +/// callers throughout the solution. +/// +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(FactoryMethodToConstructorCodeFixProvider))] +[Shared] +public sealed class FactoryMethodToConstructorCodeFixProvider : CodeFixProvider +{ + /// + public override ImmutableArray FixableDiagnosticIds => ["EFP0012"]; + + /// + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + /// + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + { + return; + } + + var diagnostic = context.Diagnostics[0]; + var node = root.FindNode(diagnostic.Location.SourceSpan); + var method = node.AncestorsAndSelf().OfType().FirstOrDefault(); + if (method is null) + { + return; + } + + if (!FactoryMethodTransformationHelper.TryGetFactoryMethodPattern(method, out var containingType, out _)) + { + return; + } + + context.RegisterCodeFix( + CodeAction.Create( + title: "Convert [Projectable] factory method to constructor", + createChangedDocument: ct => + FactoryMethodTransformationHelper.ConvertToConstructorAsync( + context.Document, method, containingType!, ct), + equivalenceKey: "EFP0012_FactoryToConstructor"), + diagnostic); + + context.RegisterCodeFix( + CodeAction.Create( + title: "Convert [Projectable] factory method to constructor (and update callers)", + createChangedSolution: ct => + FactoryMethodTransformationHelper.ConvertToConstructorAndUpdateCallersAsync( + context.Document, method, containingType!, ct), + equivalenceKey: "EFP0012_FactoryToConstructorWithCallers"), + diagnostic); + } +} + diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs new file mode 100644 index 0000000..be246f0 --- /dev/null +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs @@ -0,0 +1,73 @@ +using System.Composition; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeRefactorings; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace EntityFrameworkCore.Projectables.CodeFixes; + +/// +/// Code refactoring provider that converts a [Projectable] factory method whose body is +/// an object-initializer expression (=> new T { … }) into a [Projectable] +/// constructor of the same class. +/// +/// Two refactoring actions are offered: +/// +/// Convert the factory method to a constructor (current document only). +/// Convert the factory method to a constructor and replace all +/// callers throughout the solution with new T(…) invocations. +/// +/// +/// +/// This provider is complementary to , +/// which fixes the EFP0012 diagnostic. The refactoring provider remains useful when +/// the diagnostic is suppressed. +/// +/// +/// A public parameterless constructor is automatically inserted when the class does not already +/// have one, preserving the implicit default constructor that would otherwise be lost. +/// +/// +[ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = nameof(FactoryMethodToConstructorCodeRefactoringProvider))] +[Shared] +public sealed class FactoryMethodToConstructorCodeRefactoringProvider : CodeRefactoringProvider +{ + /// + public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + { + return; + } + + var node = root.FindNode(context.Span); + var method = node.AncestorsAndSelf().OfType().FirstOrDefault(); + if (method is null) + { + return; + } + + if (!FactoryMethodTransformationHelper.TryGetFactoryMethodPattern( + method, out var containingType, out _)) + { + return; + } + + context.RegisterRefactoring( + CodeAction.Create( + title: "Convert [Projectable] factory method to constructor", + createChangedDocument: ct => + FactoryMethodTransformationHelper.ConvertToConstructorAsync( + context.Document, method, containingType!, ct), + equivalenceKey: "EFP_FactoryToConstructor")); + + context.RegisterRefactoring( + CodeAction.Create( + title: "Convert [Projectable] factory method to constructor (and update callers)", + createChangedSolution: ct => + FactoryMethodTransformationHelper.ConvertToConstructorAndUpdateCallersAsync( + context.Document, method, containingType!, ct), + equivalenceKey: "EFP_FactoryToConstructorWithCallers")); + } +} diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs new file mode 100644 index 0000000..bdaf5ee --- /dev/null +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs @@ -0,0 +1,279 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.FindSymbols; +using Microsoft.CodeAnalysis.Formatting; + +namespace EntityFrameworkCore.Projectables.CodeFixes; + +/// +/// Shared helpers for the factory-method → projectable-constructor transformation. +/// Used by both and +/// . +/// +static internal class FactoryMethodTransformationHelper +{ + /// + /// Returns when matches the + /// factory-method pattern: + /// + /// Decorated with [Projectable]. + /// Expression body of the form => new ContainingType { … } + /// (object initializer only, no constructor arguments in the new + /// expression). + /// Return type simple name equals the containing class name. + /// + /// + static internal bool TryGetFactoryMethodPattern( + MethodDeclarationSyntax method, + out TypeDeclarationSyntax? containingType, + out ObjectCreationExpressionSyntax? creation) + { + containingType = null; + creation = null; + + if (!ProjectableCodeFixHelper.TryFindProjectableAttribute(method, out _)) + { + return false; + } + + if (method.Parent is not TypeDeclarationSyntax typeDecl) + { + return false; + } + + if (method.ExpressionBody is null) + { + return false; + } + + if (method.ExpressionBody.Expression is not ObjectCreationExpressionSyntax creationExpr) + { + return false; + } + + // Require a pure object-initializer body (no constructor arguments on the new expression). + if (creationExpr.ArgumentList?.Arguments.Count > 0) + { + return false; + } + + if (creationExpr.Initializer is null) + { + return false; + } + + // The return type's simple name must match the containing class name. + var containingTypeName = typeDecl.Identifier.Text; + if (GetSimpleTypeName(method.ReturnType) != containingTypeName + || GetSimpleTypeName(creationExpr.Type) != containingTypeName) + { + return false; + } + + containingType = typeDecl; + creation = creationExpr; + return true; + } + + private static string? GetSimpleTypeName(TypeSyntax type) => + type switch + { + IdentifierNameSyntax id => id.Identifier.Text, + QualifiedNameSyntax qn => qn.Right.Identifier.Text, + _ => null + }; + + /// + /// Fetches a fresh root, applies the factory → constructor transformation, and + /// returns an updated document. + /// + async static internal Task ConvertToConstructorAsync( + Document document, + MethodDeclarationSyntax method, + TypeDeclarationSyntax containingType, + CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + { + return document; + } + + return document.WithSyntaxRoot(BuildRootWithConstructor(root, method, containingType)); + } + + /// + /// Applies the factory → constructor transformation on the declaring document and + /// replaces all instance.FactoryMethod(args) call sites in the solution with + /// new ReturnType(args). + /// + async static internal Task ConvertToConstructorAndUpdateCallersAsync( + Document document, + MethodDeclarationSyntax method, + TypeDeclarationSyntax containingType, + CancellationToken cancellationToken) + { + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + if (semanticModel is null) + { + return document.Project.Solution; + } + + var methodSymbol = semanticModel.GetDeclaredSymbol(method, cancellationToken); + if (methodSymbol is null) + { + return document.Project.Solution; + } + + var solution = document.Project.Solution; + var returnTypeName = containingType.Identifier.Text; + + // Find all callers BEFORE modifying the solution so that spans are still valid. + var references = await SymbolFinder + .FindReferencesAsync(methodSymbol, solution, cancellationToken) + .ConfigureAwait(false); + + // Group locations by document, excluding the declaring document (handled separately below). + var locationsByDoc = new Dictionary>(); + foreach (var referencedSymbol in references) + { + foreach (var refLocation in referencedSymbol.Locations) + { + if (refLocation.Document.Id == document.Id) + { + continue; + } + + if (!locationsByDoc.TryGetValue(refLocation.Document.Id, out var list)) + { + list = []; + locationsByDoc[refLocation.Document.Id] = list; + } + + list.Add(refLocation); + } + } + + // Apply the factory → constructor transformation on the declaring document. + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + { + return solution; + } + + solution = solution.WithDocumentSyntaxRoot( + document.Id, + BuildRootWithConstructor(root, method, containingType)); + + // Replace each invocation with `new ReturnType(args)` in all caller documents. + foreach (var kvp in locationsByDoc) + { + var docId = kvp.Key; + var locations = kvp.Value; + + var callerDoc = solution.GetDocument(docId); + if (callerDoc is null) + { + continue; + } + + var callerRoot = await callerDoc.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (callerRoot is null) + { + continue; + } + + // Process end-to-start so that earlier spans remain valid. + var newCallerRoot = callerRoot; + foreach (var refLocation in locations.OrderByDescending(l => l.Location.SourceSpan.Start)) + { + var refNode = newCallerRoot.FindNode(refLocation.Location.SourceSpan); + var invocation = refNode.AncestorsAndSelf() + .OfType() + .FirstOrDefault(); + + if (invocation is null) + { + continue; + } + + // Rewrite: instance.FactoryMethod(args) → new ReturnType(args) + var newCreation = SyntaxFactory + .ObjectCreationExpression( + SyntaxFactory.Token(SyntaxKind.NewKeyword) + .WithTrailingTrivia(SyntaxFactory.Space), + SyntaxFactory.IdentifierName(returnTypeName), + invocation.ArgumentList, + initializer: null) + .WithLeadingTrivia(invocation.GetLeadingTrivia()) + .WithTrailingTrivia(invocation.GetTrailingTrivia()); + + newCallerRoot = newCallerRoot.ReplaceNode(invocation, newCreation); + } + + solution = solution.WithDocumentSyntaxRoot(docId, newCallerRoot); + } + + return solution; + } + + /// + /// Core transformation: removes the factory method, inserts an equivalent + /// [Projectable] constructor at the same position, and prepends a public + /// parameterless constructor when the class does not already have one. + /// + private static SyntaxNode BuildRootWithConstructor( + SyntaxNode root, + MethodDeclarationSyntax method, + TypeDeclarationSyntax containingType) + { + var creation = (ObjectCreationExpressionSyntax)method.ExpressionBody!.Expression; + var initializer = creation.Initializer!; + + // Convert each object-initializer assignment (Prop = value) to a statement (Prop = value;). + var statements = initializer.Expressions + .OfType() + .Select(a => (StatementSyntax)SyntaxFactory.ExpressionStatement(a)) + .ToArray(); + + var ctor = SyntaxFactory + .ConstructorDeclaration(containingType.Identifier.WithoutTrivia()) + .WithAttributeLists(method.AttributeLists) + .WithModifiers(SyntaxFactory.TokenList( + SyntaxFactory.Token(SyntaxKind.PublicKeyword) + .WithTrailingTrivia(SyntaxFactory.Space))) + .WithParameterList(method.ParameterList) + .WithBody(SyntaxFactory.Block(statements)) + .WithAdditionalAnnotations(Formatter.Annotation) + .WithLeadingTrivia(method.GetLeadingTrivia()); + + var methodIndex = containingType.Members.IndexOf(method); + var newMembers = containingType.Members.RemoveAt(methodIndex); + newMembers = newMembers.Insert(Math.Min(methodIndex, newMembers.Count), ctor); + + // Add an explicit parameterless constructor if the class has none, to avoid breaking + // code that relied on the implicit default constructor. + var hasParamlessCtor = containingType.Members + .OfType() + .Any(c => c.ParameterList.Parameters.Count == 0); + + if (!hasParamlessCtor) + { + var paramlessCtor = SyntaxFactory + .ConstructorDeclaration(containingType.Identifier.WithoutTrivia()) + .WithModifiers(SyntaxFactory.TokenList( + SyntaxFactory.Token(SyntaxKind.PublicKeyword) + .WithTrailingTrivia(SyntaxFactory.Space))) + .WithParameterList(SyntaxFactory.ParameterList()) + .WithBody(SyntaxFactory.Block()) + .WithAdditionalAnnotations(Formatter.Annotation) + .WithLeadingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed); + + newMembers = newMembers.Insert(0, paramlessCtor); + } + + return root.ReplaceNode(containingType, containingType.WithMembers(newMembers)); + } +} + diff --git a/src/EntityFrameworkCore.Projectables.Generator/AnalyzerReleases.Shipped.md b/src/EntityFrameworkCore.Projectables.Generator/AnalyzerReleases.Shipped.md index c148638..5c81204 100644 --- a/src/EntityFrameworkCore.Projectables.Generator/AnalyzerReleases.Shipped.md +++ b/src/EntityFrameworkCore.Projectables.Generator/AnalyzerReleases.Shipped.md @@ -13,6 +13,7 @@ EFP0008 | Design | Error | Target class is missing a parameterless construc EFP0009 | Design | Error | Delegated constructor cannot be analyzed for projection EFP0010 | Design | Error | UseMemberBody target member not found EFP0011 | Design | Error | UseMemberBody target member is incompatible +EFP0012 | Design | Info | [Projectable] factory method can be converted to a constructor ### Changed Rules diff --git a/src/EntityFrameworkCore.Projectables.Generator/Infrastructure/Diagnostics.cs b/src/EntityFrameworkCore.Projectables.Generator/Infrastructure/Diagnostics.cs index 17bd189..ba87028 100644 --- a/src/EntityFrameworkCore.Projectables.Generator/Infrastructure/Diagnostics.cs +++ b/src/EntityFrameworkCore.Projectables.Generator/Infrastructure/Diagnostics.cs @@ -91,4 +91,13 @@ static internal class Diagnostics category: "Design", DiagnosticSeverity.Error, isEnabledByDefault: true); + + public readonly static DiagnosticDescriptor FactoryMethodShouldBeConstructor = new DiagnosticDescriptor( + id: "EFP0012", + title: "[Projectable] factory method can be converted to a constructor", + messageFormat: "Factory method '{0}' creates and returns an instance of the containing class via object initializer. Consider converting it to a [Projectable] constructor.", + category: "Design", + DiagnosticSeverity.Info, + isEnabledByDefault: true, + helpLinkUri: "https://github.com/koenbeuk/EntityFrameworkCore.Projectables"); } \ No newline at end of file diff --git a/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs b/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs index a408787..6e3608f 100644 --- a/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs +++ b/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs @@ -184,6 +184,12 @@ private static void Execute( { throw new InvalidOperationException("Expected a memberName here"); } + + // Report EFP0012 when a [Projectable] method is a factory that could be a constructor. + if (member is MethodDeclarationSyntax factoryCandidate) + { + ReportFactoryMethodDiagnosticIfApplicable(factoryCandidate, context); + } var generatedClassName = ProjectionExpressionClassNameGenerator.GenerateName(projectable.ClassNamespace, projectable.NestedInClassNames, projectable.MemberName, projectable.ParameterTypeNames); var generatedFileName = projectable.ClassTypeParameterList is not null ? $"{generatedClassName}-{projectable.ClassTypeParameterList.ChildNodes().Count()}.g.cs" : $"{generatedClassName}.g.cs"; @@ -284,6 +290,63 @@ static TypeArgumentListSyntax GetLambdaTypeArgumentListSyntax(ProjectableDescrip } } + /// + /// Reports EFP0012 when is a [Projectable] factory + /// method whose expression body is a pure object-initializer expression targeting the + /// containing class (e.g. public static MyObj Create(…) => new MyObj { … }). + /// + private static void ReportFactoryMethodDiagnosticIfApplicable( + MethodDeclarationSyntax method, + SourceProductionContext context) + { + if (method.Parent is not TypeDeclarationSyntax containingType) + { + return; + } + + if (method.ExpressionBody is null) + { + return; + } + + if (method.ExpressionBody.Expression is not ObjectCreationExpressionSyntax creation) + { + return; + } + + // Only pure object-initializer bodies — no constructor arguments on the new expression. + if (creation.ArgumentList?.Arguments.Count > 0) + { + return; + } + + if (creation.Initializer is null) + { + return; + } + + // The return type's simple name must equal the containing class name. + var containingTypeName = containingType.Identifier.Text; + if (GetFactorySimpleTypeName(method.ReturnType) != containingTypeName + || GetFactorySimpleTypeName(creation.Type) != containingTypeName) + { + return; + } + + context.ReportDiagnostic(Diagnostic.Create( + Infrastructure.Diagnostics.FactoryMethodShouldBeConstructor, + method.Identifier.GetLocation(), + method.Identifier.Text)); + } + + private static string? GetFactorySimpleTypeName(TypeSyntax type) => + type switch + { + IdentifierNameSyntax id => id.Identifier.Text, + QualifiedNameSyntax qn => qn.Right.Identifier.Text, + _ => null + }; + /// /// Extracts a from a member declaration. /// Returns null when the member does not have [Projectable], is an extension member, diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/CodeFixTestBase.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/CodeFixTestBase.cs index 9bc523b..aa39e8c 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/CodeFixTestBase.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/CodeFixTestBase.cs @@ -15,7 +15,7 @@ namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; /// public abstract class CodeFixTestBase { - private static Document CreateDocument(string source) + protected static Document CreateDocument(string source) { var workspace = new AdhocWorkspace(); var projectId = ProjectId.CreateNewId(); diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt new file mode 100644 index 0000000..57a096b --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt @@ -0,0 +1,14 @@ + +namespace Foo { + class Input { } + class Output { + public Output() + { + } + + [Projectable] + public Output(Input i) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt new file mode 100644 index 0000000..d460104 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt @@ -0,0 +1,11 @@ + +namespace Foo { + class Input { } + class Output { + public Output() { } + [Projectable] + public Output(Input i) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt new file mode 100644 index 0000000..c6e87af --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt @@ -0,0 +1,14 @@ + +namespace Foo { + class OtherObj { } + class MyObj { + public MyObj() + { + } + + [Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore)] + public MyObj(OtherObj obj) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt new file mode 100644 index 0000000..96789f6 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt @@ -0,0 +1,16 @@ + +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public MyObj() + { + } + + public string Prop1 { get; set; } + [Projectable] + public MyObj(OtherObj obj) + { + Prop1 = obj.Prop1; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.cs new file mode 100644 index 0000000..456b008 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.cs @@ -0,0 +1,136 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; +using VerifyXunit; +using Xunit; + +namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; + +/// +/// Tests for (EFP0012). +/// Verifies the code fix output via Verify.Xunit snapshots. +/// +[UsesVerify] +public class FactoryMethodToConstructorCodeFixProviderTests : CodeFixTestBase +{ + private static readonly FactoryMethodToConstructorCodeFixProvider _provider = new(); + + private static TextSpan FirstMethodIdentifierSpan(SyntaxNode root) => + root.DescendantNodes() + .OfType() + .First() + .Identifier + .Span; + + [Fact] + public void FixableDiagnosticIds_ContainsEFP0012() => + Assert.Contains("EFP0012", _provider.FixableDiagnosticIds); + + [Fact] + public Task CodeFix_SimpleStaticFactoryMethod() => + Verifier.Verify( + ApplyCodeFixAsync( + @" +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public string Prop1 { get; set; } + [Projectable] + public static MyObj Create(OtherObj obj) => new MyObj { Prop1 = obj.Prop1 }; + } +}", + "EFP0012", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task CodeFix_PreservesProjectableOptions() => + Verifier.Verify( + ApplyCodeFixAsync( + @" +namespace Foo { + class OtherObj { } + class MyObj { + [Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore)] + public static MyObj Create(OtherObj obj) => new MyObj { }; + } +}", + "EFP0012", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task CodeFix_AddsParameterlessConstructor_WhenNoneExists() => + Verifier.Verify( + ApplyCodeFixAsync( + @" +namespace Foo { + class Input { } + class Output { + [Projectable] + public static Output Create(Input i) => new Output { }; + } +}", + "EFP0012", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent() => + Verifier.Verify( + ApplyCodeFixAsync( + @" +namespace Foo { + class Input { } + class Output { + public Output() { } + [Projectable] + public static Output Create(Input i) => new Output { }; + } +}", + "EFP0012", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public async Task TwoCodeFixActionsAreOffered() + { + var actions = await GetCodeFixActionsAsync( + @" +namespace Foo { + class MyObj { + [Projectable] + public static MyObj Create() => new MyObj { }; + } +}", + "EFP0012", + FirstMethodIdentifierSpan, + _provider); + + Assert.Equal(2, actions.Count); + Assert.Contains("constructor", actions[0].Title, StringComparison.OrdinalIgnoreCase); + Assert.Contains("callers", actions[1].Title, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task NoCodeFix_WhenPatternDoesNotMatch() + { + // Without [Projectable] the pattern check returns false → no fix registered. + var actions = await GetCodeFixActionsAsync( + @" +namespace Foo { + class MyObj { + public static MyObj Create() => new MyObj { }; + } +}", + "EFP0012", + FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } +} diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt new file mode 100644 index 0000000..57a096b --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt @@ -0,0 +1,14 @@ + +namespace Foo { + class Input { } + class Output { + public Output() + { + } + + [Projectable] + public Output(Input i) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt new file mode 100644 index 0000000..d460104 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt @@ -0,0 +1,11 @@ + +namespace Foo { + class Input { } + class Output { + public Output() { } + [Projectable] + public Output(Input i) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt new file mode 100644 index 0000000..60e08bd --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt @@ -0,0 +1,18 @@ + +namespace Foo { + class Src { public int A { get; set; } public int B { get; set; } } + class Dest { + public Dest() + { + } + + public int A { get; set; } + public int B { get; set; } + [Projectable] + public Dest(Src src) + { + A = src.A; + B = src.B; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt new file mode 100644 index 0000000..c6e87af --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt @@ -0,0 +1,14 @@ + +namespace Foo { + class OtherObj { } + class MyObj { + public MyObj() + { + } + + [Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore)] + public MyObj(OtherObj obj) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt new file mode 100644 index 0000000..96789f6 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt @@ -0,0 +1,16 @@ + +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public MyObj() + { + } + + public string Prop1 { get; set; } + [Projectable] + public MyObj(OtherObj obj) + { + Prop1 = obj.Prop1; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt new file mode 100644 index 0000000..82d32e6 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt @@ -0,0 +1,17 @@ + +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public Output() + { + } + + public int Value { get; set; } + public string Name { get; set; } + [Projectable] + public Output(Input i) + { + Value = i.Value; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.cs new file mode 100644 index 0000000..72ee275 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.cs @@ -0,0 +1,243 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; +using VerifyXunit; +using Xunit; + +namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; + +/// +/// Tests for . +/// Each test verifies via Verify.Xunit snapshots (.verified.txt files). +/// +[UsesVerify] +public class FactoryMethodToConstructorCodeRefactoringProviderTests : RefactoringTestBase +{ + private static readonly FactoryMethodToConstructorCodeRefactoringProvider _provider = new(); + + // Locates the span of the first method identifier — the provider walks up to MethodDeclarationSyntax. + private static TextSpan FirstMethodIdentifierSpan(SyntaxNode root) => + root.DescendantNodes() + .OfType() + .First() + .Identifier + .Span; + + // ──────────────────────────────────────────────────────────────────────────── + // Action 0 — convert factory method to constructor (document only) + // ──────────────────────────────────────────────────────────────────────────── + + [Fact] + public Task ConvertToConstructor_SimpleFactoryMethod() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public string Prop1 { get; set; } + [Projectable] + public static MyObj Create(OtherObj obj) => new MyObj { Prop1 = obj.Prop1 }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task ConvertToConstructor_PreservesProjectableOptions() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class OtherObj { } + class MyObj { + [Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore)] + public static MyObj Create(OtherObj obj) => new MyObj { }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task ConvertToConstructor_MultipleInitializerAssignments() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class Src { public int A { get; set; } public int B { get; set; } } + class Dest { + public int A { get; set; } + public int B { get; set; } + [Projectable] + public static Dest Map(Src src) => new Dest { A = src.A, B = src.B }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class Input { } + class Output { + [Projectable] + public static Output Create(Input i) => new Output { }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class Input { } + class Output { + public Output() { } + [Projectable] + public static Output Create(Input i) => new Output { }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task ConvertToConstructor_WithExistingMembers_PreservesMemberOrder() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public int Value { get; set; } + public string Name { get; set; } + [Projectable] + public static Output From(Input i) => new Output { Value = i.Value }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + // ──────────────────────────────────────────────────────────────────────────── + // Guard: no refactoring should be offered in inapplicable situations + // ──────────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task NoRefactoring_WhenMethodHasNoProjectableAttribute() + { + var actions = await GetRefactoringActionsAsync( + @" +namespace Foo { + class MyObj { + public MyObj Create() => new MyObj { }; + } +}", + FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + + [Fact] + public async Task NoRefactoring_WhenReturnTypeDoesNotMatchContainingClass() + { + var actions = await GetRefactoringActionsAsync( + @" +namespace Foo { + class Other { } + class MyObj { + [Projectable] + public Other Create() => new Other { }; + } +}", + FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + + [Fact] + public async Task NoRefactoring_WhenBodyHasConstructorArguments() + { + var actions = await GetRefactoringActionsAsync( + @" +namespace Foo { + class MyObj { + [Projectable] + public MyObj Create(int x) => new MyObj(x) { }; + } +}", + FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + + [Fact] + public async Task NoRefactoring_WhenBodyIsNotObjectCreation() + { + var actions = await GetRefactoringActionsAsync( + @" +namespace Foo { + class MyObj { + [Projectable] + public MyObj Create() => default; + } +}", + FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + + [Fact] + public async Task NoRefactoring_WhenBodyHasNoInitializer() + { + var actions = await GetRefactoringActionsAsync( + @" +namespace Foo { + class MyObj { + [Projectable] + public MyObj Create() => new MyObj(); + } +}", + FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + + // ──────────────────────────────────────────────────────────────────────────── + // Action titles + // ──────────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task TwoActionsAreOffered_WithCorrectTitles() + { + var actions = await GetRefactoringActionsAsync( + @" +namespace Foo { + class MyObj { + [Projectable] + public static MyObj Create() => new MyObj { }; + } +}", + FirstMethodIdentifierSpan, + _provider); + + Assert.Equal(2, actions.Count); + Assert.Contains("constructor", actions[0].Title, StringComparison.OrdinalIgnoreCase); + Assert.Contains("callers", actions[1].Title, StringComparison.OrdinalIgnoreCase); + } +} diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/RefactoringTestBase.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/RefactoringTestBase.cs new file mode 100644 index 0000000..93684e6 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/RefactoringTestBase.cs @@ -0,0 +1,78 @@ +using System.Diagnostics.CodeAnalysis; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeRefactorings; +using Microsoft.CodeAnalysis.Text; +using Xunit; + +namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; + +/// +/// Base class providing helpers for tests. +/// Builds an in-memory document, invokes the provider at the supplied span, and +/// optionally applies one of the offered refactoring actions. +/// +public abstract class RefactoringTestBase : CodeFixTestBase +{ + private async static Task<(Document Document, IReadOnlyList Actions)> CollectRefactoringActionsAsync( + [StringSyntax("csharp")] + string source, + Func locateSpan, + CodeRefactoringProvider provider) + { + var document = CreateDocument(source); + var root = await document.GetSyntaxRootAsync(); + var span = locateSpan(root!); + + var actions = new List(); + var context = new CodeRefactoringContext( + document, + span, + action => actions.Add(action), + CancellationToken.None); + + await provider.ComputeRefactoringsAsync(context); + return (document, actions); + } + + /// + /// Returns all instances offered by + /// for the span returned by . + /// + protected async static Task> GetRefactoringActionsAsync( + [StringSyntax("csharp")] + string source, + Func locateSpan, + CodeRefactoringProvider provider) + { + var (_, actions) = await CollectRefactoringActionsAsync(source, locateSpan, provider); + return actions; + } + + /// + /// Applies the refactoring action at and returns the full + /// source text of the primary (originating) document after the change. + /// + protected async static Task ApplyRefactoringAsync( + [StringSyntax("csharp")] + string source, + Func locateSpan, + CodeRefactoringProvider provider, + int actionIndex = 0) + { + var (document, actions) = await CollectRefactoringActionsAsync(source, locateSpan, provider); + + Assert.True( + actions.Count > actionIndex, + $"Expected at least {actionIndex + 1} refactoring action(s) but only {actions.Count} were registered."); + + var action = actions[actionIndex]; + var operations = await action.GetOperationsAsync(CancellationToken.None); + var applyOp = operations.OfType().Single(); + + var newDocument = applyOp.ChangedSolution.GetDocument(document.Id)!; + var newRoot = await newDocument.GetSyntaxRootAsync(); + return newRoot!.ToFullString(); + } +} + diff --git a/tests/EntityFrameworkCore.Projectables.Generator.Tests/FactoryMethodDiagnosticTests.cs b/tests/EntityFrameworkCore.Projectables.Generator.Tests/FactoryMethodDiagnosticTests.cs new file mode 100644 index 0000000..33b491b --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.Generator.Tests/FactoryMethodDiagnosticTests.cs @@ -0,0 +1,220 @@ +using System.Linq; +using Microsoft.CodeAnalysis; +using Xunit; +using Xunit.Abstractions; + +namespace EntityFrameworkCore.Projectables.Generator.Tests; + +/// +/// Tests that the generator reports EFP0012 (Info) when a [Projectable] method +/// matches the factory-method pattern (expression body => new ContainingType { … }), +/// and that it does NOT report the diagnostic for methods that do not match. +/// +/// Unlike the previous standalone DiagnosticAnalyzer, the diagnostic is now emitted +/// directly in so that it is part of the +/// same incremental pipeline that produces the expression tree — no separate analysis pass needed. +/// +/// +public class FactoryMethodDiagnosticTests : ProjectionExpressionGeneratorTestsBase +{ + public FactoryMethodDiagnosticTests(ITestOutputHelper testOutputHelper) : base(testOutputHelper) { } + + // ──────────────────────────────────────────────────────────────────────── + // EFP0012 is reported + // ──────────────────────────────────────────────────────────────────────── + + [Fact] + public void ReportsEFP0012_OnStaticFactoryMethod() + { + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class OtherObj { } + class MyObj { + [Projectable] + public static MyObj Create(OtherObj o) => new MyObj { }; + } +}"); + var result = RunGenerator(compilation); + + var diag = Assert.Single(result.Diagnostics); + Assert.Equal("EFP0012", diag.Id); + Assert.Equal(DiagnosticSeverity.Info, diag.Severity); + Assert.Equal("Create", diag.Location.SourceTree! + .GetRoot().FindToken(diag.Location.SourceSpan.Start).ValueText); + } + + [Fact] + public void ReportsEFP0012_OnInstanceFactoryMethod() + { + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class OtherObj { } + class MyObj { + [Projectable] + public MyObj Create(OtherObj o) => new MyObj { }; + } +}"); + var result = RunGenerator(compilation); + + var diag = Assert.Single(result.Diagnostics); + Assert.Equal("EFP0012", diag.Id); + } + + [Fact] + public void ReportsEFP0012_WithMultipleInitializerAssignments() + { + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class Src { public int A { get; set; } public int B { get; set; } } + class Dest { + public int A { get; set; } + public int B { get; set; } + [Projectable] + public static Dest Map(Src s) => new Dest { A = s.A, B = s.B }; + } +}"); + var result = RunGenerator(compilation); + + var diag = Assert.Single(result.Diagnostics); + Assert.Equal("EFP0012", diag.Id); + } + + [Fact] + public void ReportsEFP0012_AndStillGeneratesExpressionTree() + { + // EFP0012 is Info — generation must not be blocked. + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public int Value { get; set; } + [Projectable] + public static Output From(Input i) => new Output { Value = i.Value }; + } +}"); + var result = RunGenerator(compilation); + + Assert.Single(result.Diagnostics.Where(d => d.Id == "EFP0012")); + Assert.Single(result.GeneratedTrees); // expression tree is still generated + } + + [Fact] + public void ReportsEFP0012_WithProjectableOptions_Preserved() + { + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class OtherObj { } + class MyObj { + [Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore)] + public static MyObj Create(OtherObj o) => new MyObj { }; + } +}"); + var result = RunGenerator(compilation); + + var diag = Assert.Single(result.Diagnostics); + Assert.Equal("EFP0012", diag.Id); + } + + // ──────────────────────────────────────────────────────────────────────── + // EFP0012 is NOT reported + // ──────────────────────────────────────────────────────────────────────── + + [Fact] + public void NoEFP0012_WhenBodyHasConstructorArguments() + { + // new MyObj(x) { } — has constructor args, not a pure initializer + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class MyObj { + public MyObj() { } + public MyObj(int x) { } + [Projectable] + public static MyObj Create(int x) => new MyObj(x) { }; + } +}"); + var result = RunGenerator(compilation); + + Assert.DoesNotContain(result.Diagnostics, d => d.Id == "EFP0012"); + } + + [Fact] + public void NoEFP0012_WhenBodyHasNoInitializer() + { + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class MyObj { + public MyObj() { } + [Projectable] + public static MyObj Create() => new MyObj(); + } +}"); + var result = RunGenerator(compilation); + + Assert.DoesNotContain(result.Diagnostics, d => d.Id == "EFP0012"); + } + + [Fact] + public void NoEFP0012_WhenReturnTypeDoesNotMatchContainingClass() + { + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class Other { } + class MyObj { + [Projectable] + public static Other Create() => new Other { }; + } +}"); + var result = RunGenerator(compilation); + + Assert.DoesNotContain(result.Diagnostics, d => d.Id == "EFP0012"); + } + + [Fact] + public void NoEFP0012_WhenBodyIsNotObjectCreation() + { + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class MyObj { + public int Value { get; set; } + [Projectable] + public int Computed => Value * 2; + } +}"); + var result = RunGenerator(compilation); + + Assert.Empty(result.Diagnostics); + } + + [Fact] + public void NoEFP0012_ForProjectableConstructor() + { + var compilation = CreateCompilation(@" +using EntityFrameworkCore.Projectables; +namespace Foo { + class OtherObj { public int X { get; set; } } + class MyObj { + public int X { get; set; } + public MyObj() { } + [Projectable] + public MyObj(OtherObj o) { + X = o.X; + } + } +}"); + var result = RunGenerator(compilation); + + // A [Projectable] constructor is not a factory method — EFP0012 must not be reported. + Assert.DoesNotContain(result.Diagnostics, d => d.Id == "EFP0012"); + } +} + + From b12e078d893e56c8aa7052cc42f453425fa3a733 Mon Sep 17 00:00:00 2001 From: "fabien.menager" Date: Mon, 16 Mar 2026 21:43:53 +0100 Subject: [PATCH 02/11] Reduce filename size --- .../FactoryMethodToConstructorCodeRefactoringProvider.cs | 2 +- ...FixProvider.cs => FactoryMethodToCtorCodeFixProvider.cs} | 4 ++-- .../FactoryMethodTransformationHelper.cs | 2 +- ...ddsParameterlessConstructor_WhenNoneExists.verified.txt} | 0 ...arameterlessConstructor_WhenAlreadyPresent.verified.txt} | 0 ...rTests.CodeFix_PreservesProjectableOptions.verified.txt} | 0 ...derTests.CodeFix_SimpleStaticFactoryMethod.verified.txt} | 0 ...rTests.cs => FactoryMethodToCtorCodeFixProviderTests.cs} | 6 +++--- ...ddsParameterlessConstructor_WhenNoneExists.verified.txt} | 0 ...arameterlessConstructor_WhenAlreadyPresent.verified.txt} | 0 ...Constructor_MultipleInitializerAssignments.verified.txt} | 0 ...tToConstructor_PreservesProjectableOptions.verified.txt} | 0 ...s.ConvertToConstructor_SimpleFactoryMethod.verified.txt} | 0 ...r_WithExistingMembers_PreservesMemberOrder.verified.txt} | 0 ...rTests.cs => FactoryMethodToCtorCodeRefProviderTests.cs} | 2 +- 15 files changed, 8 insertions(+), 8 deletions(-) rename src/EntityFrameworkCore.Projectables.CodeFixes/{FactoryMethodToConstructorCodeFixProvider.cs => FactoryMethodToCtorCodeFixProvider.cs} (95%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt => FactoryMethodToCtorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt => FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt => FactoryMethodToCtorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt => FactoryMethodToCtorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeFixProviderTests.cs => FactoryMethodToCtorCodeFixProviderTests.cs} (93%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt => FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt => FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt => FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt => FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt => FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt => FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt} (100%) rename tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/{FactoryMethodToConstructorCodeRefactoringProviderTests.cs => FactoryMethodToCtorCodeRefProviderTests.cs} (98%) diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs index be246f0..cc52d1f 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs @@ -19,7 +19,7 @@ namespace EntityFrameworkCore.Projectables.CodeFixes; /// /// /// -/// This provider is complementary to , +/// This provider is complementary to , /// which fixes the EFP0012 diagnostic. The refactoring provider remains useful when /// the diagnostic is suppressed. /// diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeFixProvider.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToCtorCodeFixProvider.cs similarity index 95% rename from src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeFixProvider.cs rename to src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToCtorCodeFixProvider.cs index 1ca4cb1..d248b79 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeFixProvider.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToCtorCodeFixProvider.cs @@ -16,9 +16,9 @@ namespace EntityFrameworkCore.Projectables.CodeFixes; /// callers throughout the solution. /// /// -[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(FactoryMethodToConstructorCodeFixProvider))] +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(FactoryMethodToCtorCodeFixProvider))] [Shared] -public sealed class FactoryMethodToConstructorCodeFixProvider : CodeFixProvider +public sealed class FactoryMethodToCtorCodeFixProvider : CodeFixProvider { /// public override ImmutableArray FixableDiagnosticIds => ["EFP0012"]; diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs index bdaf5ee..c1adeae 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs @@ -9,7 +9,7 @@ namespace EntityFrameworkCore.Projectables.CodeFixes; /// /// Shared helpers for the factory-method → projectable-constructor transformation. /// Used by both and -/// . +/// . /// static internal class FactoryMethodTransformationHelper { diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_AddsParameterlessConstructor_WhenNoneExists.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_PreservesProjectableOptions.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_SimpleStaticFactoryMethod.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs similarity index 93% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.cs rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs index 456b008..c805728 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeFixProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs @@ -7,13 +7,13 @@ namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; /// -/// Tests for (EFP0012). +/// Tests for (EFP0012). /// Verifies the code fix output via Verify.Xunit snapshots. /// [UsesVerify] -public class FactoryMethodToConstructorCodeFixProviderTests : CodeFixTestBase +public class FactoryMethodToCtorCodeFixProviderTests : CodeFixTestBase { - private static readonly FactoryMethodToConstructorCodeFixProvider _provider = new(); + private static readonly FactoryMethodToCtorCodeFixProvider _provider = new(); private static TextSpan FirstMethodIdentifierSpan(SyntaxNode root) => root.DescendantNodes() diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesProjectableOptions.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_SimpleFactoryMethod.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt similarity index 100% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_WithExistingMembers_PreservesMemberOrder.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs similarity index 98% rename from tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.cs rename to tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs index 72ee275..2ee3629 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToConstructorCodeRefactoringProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs @@ -11,7 +11,7 @@ namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; /// Each test verifies via Verify.Xunit snapshots (.verified.txt files). /// [UsesVerify] -public class FactoryMethodToConstructorCodeRefactoringProviderTests : RefactoringTestBase +public class FactoryMethodToCtorCodeRefProviderTests : RefactoringTestBase { private static readonly FactoryMethodToConstructorCodeRefactoringProvider _provider = new(); From 8c11bc83055b8f108b2d33c773d8f0cf166a3fe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabien=20M=C3=A9nager?= Date: Mon, 16 Mar 2026 21:47:37 +0100 Subject: [PATCH 03/11] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../FactoryMethodTransformationHelper.cs | 61 ++++++++++++++----- .../ProjectionExpressionGenerator.cs | 33 ++++++---- 2 files changed, 67 insertions(+), 27 deletions(-) diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs index bdaf5ee..61ab4f0 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs @@ -3,6 +3,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.FindSymbols; using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Simplification; namespace EntityFrameworkCore.Projectables.CodeFixes; @@ -42,6 +43,13 @@ static internal bool TryGetFactoryMethodPattern( return false; } + // Only support static factory methods; instance factories would drop the receiver + // when transformed to a constructor call, which can change semantics. + if (!method.Modifiers.Any(SyntaxKind.StaticKeyword)) + { + return false; + } + if (method.ExpressionBody is null) { return false; @@ -127,24 +135,22 @@ async static internal Task ConvertToConstructorAndUpdateCallersAsync( } var solution = document.Project.Solution; - var returnTypeName = containingType.Identifier.Text; + var returnType = methodSymbol.ReturnType; + var returnTypeSyntax = SyntaxFactory + .ParseTypeName(returnType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)) + .WithAdditionalAnnotations(Simplifier.Annotation); // Find all callers BEFORE modifying the solution so that spans are still valid. var references = await SymbolFinder .FindReferencesAsync(methodSymbol, solution, cancellationToken) .ConfigureAwait(false); - // Group locations by document, excluding the declaring document (handled separately below). + // Group locations by document (including the declaring document). var locationsByDoc = new Dictionary>(); foreach (var referencedSymbol in references) { foreach (var refLocation in referencedSymbol.Locations) { - if (refLocation.Document.Id == document.Id) - { - continue; - } - if (!locationsByDoc.TryGetValue(refLocation.Document.Id, out var list)) { list = []; @@ -198,12 +204,19 @@ async static internal Task ConvertToConstructorAndUpdateCallersAsync( continue; } + // Skip conditional-access invocations like x?.FactoryMethod(...) + // to avoid producing invalid syntax such as x?.new ReturnType(...). + if (invocation.Parent is ConditionalAccessExpressionSyntax) + { + continue; + } + // Rewrite: instance.FactoryMethod(args) → new ReturnType(args) var newCreation = SyntaxFactory .ObjectCreationExpression( SyntaxFactory.Token(SyntaxKind.NewKeyword) .WithTrailingTrivia(SyntaxFactory.Space), - SyntaxFactory.IdentifierName(returnTypeName), + returnTypeSyntax, invocation.ArgumentList, initializer: null) .WithLeadingTrivia(invocation.GetLeadingTrivia()) @@ -231,18 +244,26 @@ private static SyntaxNode BuildRootWithConstructor( var creation = (ObjectCreationExpressionSyntax)method.ExpressionBody!.Expression; var initializer = creation.Initializer!; + // Only support simple object-initializer assignments (Prop = value). If there are + // other initializer forms (e.g., collection initializers), bail out to avoid + // producing a constructor that does not preserve behavior. + if (initializer.Expressions.Any(e => e is not AssignmentExpressionSyntax)) + { + return root; + } + // Convert each object-initializer assignment (Prop = value) to a statement (Prop = value;). var statements = initializer.Expressions .OfType() .Select(a => (StatementSyntax)SyntaxFactory.ExpressionStatement(a)) .ToArray(); + var ctorModifiers = GetConstructorModifiers(method); + var ctor = SyntaxFactory .ConstructorDeclaration(containingType.Identifier.WithoutTrivia()) .WithAttributeLists(method.AttributeLists) - .WithModifiers(SyntaxFactory.TokenList( - SyntaxFactory.Token(SyntaxKind.PublicKeyword) - .WithTrailingTrivia(SyntaxFactory.Space))) + .WithModifiers(ctorModifiers) .WithParameterList(method.ParameterList) .WithBody(SyntaxFactory.Block(statements)) .WithAdditionalAnnotations(Formatter.Annotation) @@ -262,9 +283,7 @@ private static SyntaxNode BuildRootWithConstructor( { var paramlessCtor = SyntaxFactory .ConstructorDeclaration(containingType.Identifier.WithoutTrivia()) - .WithModifiers(SyntaxFactory.TokenList( - SyntaxFactory.Token(SyntaxKind.PublicKeyword) - .WithTrailingTrivia(SyntaxFactory.Space))) + .WithModifiers(ctorModifiers) .WithParameterList(SyntaxFactory.ParameterList()) .WithBody(SyntaxFactory.Block()) .WithAdditionalAnnotations(Formatter.Annotation) @@ -275,5 +294,19 @@ private static SyntaxNode BuildRootWithConstructor( return root.ReplaceNode(containingType, containingType.WithMembers(newMembers)); } + + private static SyntaxTokenList GetConstructorModifiers(MethodDeclarationSyntax method) + { + // Derive constructor modifiers from the factory method, dropping modifiers that are + // invalid or meaningless for instance constructors (e.g., static, async, extern, unsafe). + var filteredModifiers = method.Modifiers + .Where(m => + m.Kind() != SyntaxKind.StaticKeyword && + m.Kind() != SyntaxKind.AsyncKeyword && + m.Kind() != SyntaxKind.ExternKeyword && + m.Kind() != SyntaxKind.UnsafeKeyword); + + return SyntaxFactory.TokenList(filteredModifiers); + } } diff --git a/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs b/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs index 6e3608f..41fbfe4 100644 --- a/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs +++ b/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs @@ -299,7 +299,7 @@ private static void ReportFactoryMethodDiagnosticIfApplicable( MethodDeclarationSyntax method, SourceProductionContext context) { - if (method.Parent is not TypeDeclarationSyntax containingType) + if (method.Parent is not TypeDeclarationSyntax) { return; } @@ -325,10 +325,25 @@ private static void ReportFactoryMethodDiagnosticIfApplicable( return; } - // The return type's simple name must equal the containing class name. - var containingTypeName = containingType.Identifier.Text; - if (GetFactorySimpleTypeName(method.ReturnType) != containingTypeName - || GetFactorySimpleTypeName(creation.Type) != containingTypeName) + if (memberSymbol is not IMethodSymbol methodSymbol) + { + return; + } + + var containingTypeSymbol = methodSymbol.ContainingType; + if (containingTypeSymbol is null) + { + return; + } + + var createdTypeSymbol = semanticModel.GetTypeInfo(creation).Type; + if (createdTypeSymbol is null) + { + return; + } + + if (!SymbolEqualityComparer.Default.Equals(methodSymbol.ReturnType, containingTypeSymbol) + || !SymbolEqualityComparer.Default.Equals(createdTypeSymbol, containingTypeSymbol)) { return; } @@ -339,14 +354,6 @@ private static void ReportFactoryMethodDiagnosticIfApplicable( method.Identifier.Text)); } - private static string? GetFactorySimpleTypeName(TypeSyntax type) => - type switch - { - IdentifierNameSyntax id => id.Identifier.Text, - QualifiedNameSyntax qn => qn.Right.Identifier.Text, - _ => null - }; - /// /// Extracts a from a member declaration. /// Returns null when the member does not have [Projectable], is an extension member, From f404028e24ac9adea84ef5b417c0bb9e6d2b07a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabien=20M=C3=A9nager?= Date: Mon, 16 Mar 2026 21:51:16 +0100 Subject: [PATCH 04/11] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../FactoryMethodTransformationHelper.cs | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs index 61ab4f0..6432621 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs @@ -57,7 +57,36 @@ static internal bool TryGetFactoryMethodPattern( if (method.ExpressionBody.Expression is not ObjectCreationExpressionSyntax creationExpr) { - return false; + if (method.ExpressionBody.Expression is not ImplicitObjectCreationExpressionSyntax implicitCreation) + { + return false; + } + + if (implicitCreation.ArgumentList?.Arguments.Count > 0) + { + return false; + } + + if (implicitCreation.Initializer is null) + { + return false; + } + + var containingTypeName = typeDecl.Identifier.Text; + if (GetSimpleTypeName(method.ReturnType) != containingTypeName) + { + return false; + } + + var typeSyntax = SyntaxFactory.IdentifierName(containingTypeName); + var objectCreation = SyntaxFactory.ObjectCreationExpression( + typeSyntax, + implicitCreation.ArgumentList ?? SyntaxFactory.ArgumentList(), + implicitCreation.Initializer); + + containingType = typeDecl; + creation = objectCreation; + return true; } // Require a pure object-initializer body (no constructor arguments on the new expression). From 7bbcd06defe0bbb4ee3a244b91b8ac2add07947f Mon Sep 17 00:00:00 2001 From: "fabien.menager" Date: Mon, 16 Mar 2026 22:53:31 +0100 Subject: [PATCH 05/11] Factorize code --- ...hodToConstructorCodeRefactoringProvider.cs | 13 +- .../FactoryMethodToCtorCodeFixProvider.cs | 19 +-- .../FactoryMethodTransformationHelper.cs | 115 +----------------- .../ProjectableCodeFixHelper.cs | 24 ++++ .../SyntaxHelpers.cs | 68 +++++++++++ ...rameworkCore.Projectables.Generator.csproj | 1 + .../Infrastructure/Diagnostics.cs | 3 +- .../ProjectionExpressionGenerator.cs | 72 +---------- 8 files changed, 113 insertions(+), 202 deletions(-) create mode 100644 src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs index cc52d1f..4a8fb20 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToConstructorCodeRefactoringProvider.cs @@ -2,7 +2,6 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeRefactorings; -using Microsoft.CodeAnalysis.CSharp.Syntax; namespace EntityFrameworkCore.Projectables.CodeFixes; @@ -42,14 +41,8 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte } var node = root.FindNode(context.Span); - var method = node.AncestorsAndSelf().OfType().FirstOrDefault(); - if (method is null) - { - return; - } - if (!FactoryMethodTransformationHelper.TryGetFactoryMethodPattern( - method, out var containingType, out _)) + if (!ProjectableCodeFixHelper.TryGetFixableFactoryMethodPattern(node, out var containingType, out var method)) { return; } @@ -59,7 +52,7 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte title: "Convert [Projectable] factory method to constructor", createChangedDocument: ct => FactoryMethodTransformationHelper.ConvertToConstructorAsync( - context.Document, method, containingType!, ct), + context.Document, method!, containingType!, ct), equivalenceKey: "EFP_FactoryToConstructor")); context.RegisterRefactoring( @@ -67,7 +60,7 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte title: "Convert [Projectable] factory method to constructor (and update callers)", createChangedSolution: ct => FactoryMethodTransformationHelper.ConvertToConstructorAndUpdateCallersAsync( - context.Document, method, containingType!, ct), + context.Document, method!, containingType!, ct), equivalenceKey: "EFP_FactoryToConstructorWithCallers")); } } diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToCtorCodeFixProvider.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToCtorCodeFixProvider.cs index d248b79..8136254 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToCtorCodeFixProvider.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodToCtorCodeFixProvider.cs @@ -3,7 +3,6 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp.Syntax; namespace EntityFrameworkCore.Projectables.CodeFixes; @@ -35,15 +34,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) return; } - var diagnostic = context.Diagnostics[0]; - var node = root.FindNode(diagnostic.Location.SourceSpan); - var method = node.AncestorsAndSelf().OfType().FirstOrDefault(); - if (method is null) - { - return; - } + var node = root.FindNode(context.Span); - if (!FactoryMethodTransformationHelper.TryGetFactoryMethodPattern(method, out var containingType, out _)) + if (!ProjectableCodeFixHelper.TryGetFixableFactoryMethodPattern(node, out var containingType, out var method)) { return; } @@ -53,18 +46,18 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) title: "Convert [Projectable] factory method to constructor", createChangedDocument: ct => FactoryMethodTransformationHelper.ConvertToConstructorAsync( - context.Document, method, containingType!, ct), + context.Document, method!, containingType!, ct), equivalenceKey: "EFP0012_FactoryToConstructor"), - diagnostic); + context.Diagnostics[0]); context.RegisterCodeFix( CodeAction.Create( title: "Convert [Projectable] factory method to constructor (and update callers)", createChangedSolution: ct => FactoryMethodTransformationHelper.ConvertToConstructorAndUpdateCallersAsync( - context.Document, method, containingType!, ct), + context.Document, method!, containingType!, ct), equivalenceKey: "EFP0012_FactoryToConstructorWithCallers"), - diagnostic); + context.Diagnostics[0]); } } diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs index c479cff..62da78b 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs @@ -14,113 +14,6 @@ namespace EntityFrameworkCore.Projectables.CodeFixes; /// static internal class FactoryMethodTransformationHelper { - /// - /// Returns when matches the - /// factory-method pattern: - /// - /// Decorated with [Projectable]. - /// Expression body of the form => new ContainingType { … } - /// (object initializer only, no constructor arguments in the new - /// expression). - /// Return type simple name equals the containing class name. - /// - /// - static internal bool TryGetFactoryMethodPattern( - MethodDeclarationSyntax method, - out TypeDeclarationSyntax? containingType, - out ObjectCreationExpressionSyntax? creation) - { - containingType = null; - creation = null; - - if (!ProjectableCodeFixHelper.TryFindProjectableAttribute(method, out _)) - { - return false; - } - - if (method.Parent is not TypeDeclarationSyntax typeDecl) - { - return false; - } - - // Only support static factory methods; instance factories would drop the receiver - // when transformed to a constructor call, which can change semantics. - if (!method.Modifiers.Any(SyntaxKind.StaticKeyword)) - { - return false; - } - - if (method.ExpressionBody is null) - { - return false; - } - - if (method.ExpressionBody.Expression is not ObjectCreationExpressionSyntax creationExpr) - { - if (method.ExpressionBody.Expression is not ImplicitObjectCreationExpressionSyntax implicitCreation) - { - return false; - } - - if (implicitCreation.ArgumentList?.Arguments.Count > 0) - { - return false; - } - - if (implicitCreation.Initializer is null) - { - return false; - } - - var containingTypeName = typeDecl.Identifier.Text; - if (GetSimpleTypeName(method.ReturnType) != containingTypeName) - { - return false; - } - - var typeSyntax = SyntaxFactory.IdentifierName(containingTypeName); - var objectCreation = SyntaxFactory.ObjectCreationExpression( - typeSyntax, - implicitCreation.ArgumentList ?? SyntaxFactory.ArgumentList(), - implicitCreation.Initializer); - - containingType = typeDecl; - creation = objectCreation; - return true; - } - - // Require a pure object-initializer body (no constructor arguments on the new expression). - if (creationExpr.ArgumentList?.Arguments.Count > 0) - { - return false; - } - - if (creationExpr.Initializer is null) - { - return false; - } - - // The return type's simple name must match the containing class name. - var containingTypeName = typeDecl.Identifier.Text; - if (GetSimpleTypeName(method.ReturnType) != containingTypeName - || GetSimpleTypeName(creationExpr.Type) != containingTypeName) - { - return false; - } - - containingType = typeDecl; - creation = creationExpr; - return true; - } - - private static string? GetSimpleTypeName(TypeSyntax type) => - type switch - { - IdentifierNameSyntax id => id.Identifier.Text, - QualifiedNameSyntax qn => qn.Right.Identifier.Text, - _ => null - }; - /// /// Fetches a fresh root, applies the factory → constructor transformation, and /// returns an updated document. @@ -330,10 +223,10 @@ private static SyntaxTokenList GetConstructorModifiers(MethodDeclarationSyntax m // invalid or meaningless for instance constructors (e.g., static, async, extern, unsafe). var filteredModifiers = method.Modifiers .Where(m => - m.Kind() != SyntaxKind.StaticKeyword && - m.Kind() != SyntaxKind.AsyncKeyword && - m.Kind() != SyntaxKind.ExternKeyword && - m.Kind() != SyntaxKind.UnsafeKeyword); + !m.IsKind(SyntaxKind.StaticKeyword) && + !m.IsKind(SyntaxKind.AsyncKeyword) && + !m.IsKind(SyntaxKind.ExternKeyword) && + !m.IsKind(SyntaxKind.UnsafeKeyword)); return SyntaxFactory.TokenList(filteredModifiers); } diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/ProjectableCodeFixHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/ProjectableCodeFixHelper.cs index 0821e64..c06cd6c 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/ProjectableCodeFixHelper.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/ProjectableCodeFixHelper.cs @@ -21,6 +21,30 @@ static internal bool TryFindProjectableAttribute(MemberDeclarationSyntax member, return attribute is not null; } + + static internal bool TryGetFixableFactoryMethodPattern( + SyntaxNode methodNode, + out TypeDeclarationSyntax? containingType, + out MethodDeclarationSyntax? method) + { + containingType = null; + method = null; + + var localMethod = methodNode.AncestorsAndSelf().OfType().FirstOrDefault(); + if (localMethod is null) + { + return false; + } + + if (!SyntaxHelpers.TryGetFactoryMethodPattern(localMethod, out containingType)) + { + return false; + } + + method = localMethod; + + return TryFindProjectableAttribute(localMethod, out _); + } /// /// Adds or replaces a named argument on the [Projectable] attribute of . diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs new file mode 100644 index 0000000..2a4eb92 --- /dev/null +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs @@ -0,0 +1,68 @@ +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace EntityFrameworkCore.Projectables.CodeFixes; + +static internal class SyntaxHelpers +{ + /// + /// Returns when matches the + /// factory-method pattern: + /// + /// Decorated with [Projectable]. + /// Expression body of the form => new ContainingType { … } + /// (object initializer only, no constructor arguments in the new + /// expression). + /// Return type simple name equals the containing class name. + /// + /// + static internal bool TryGetFactoryMethodPattern( + MethodDeclarationSyntax method, + out TypeDeclarationSyntax? containingType) + { + containingType = null; + + if (method.Parent is not TypeDeclarationSyntax parentType) + { + return false; + } + + if (method.ExpressionBody is null) + { + return false; + } + + if (method.ExpressionBody.Expression is not BaseObjectCreationExpressionSyntax creation) + { + return false; + } + + // Only pure object-initializer bodies — no constructor arguments on the new expression. + if (creation.ArgumentList?.Arguments.Count > 0) + { + return false; + } + + if (creation.Initializer is null) + { + return false; + } + + // The method's return type must match the containing type (syntax-level name comparison). + var returnTypeName = method.ReturnType switch + { + IdentifierNameSyntax id => id.Identifier.Text, + QualifiedNameSyntax { Right: IdentifierNameSyntax right } => right.Identifier.Text, + GenericNameSyntax generic => generic.Identifier.Text, + _ => null + }; + + if (returnTypeName is null || returnTypeName != parentType.Identifier.Text) + { + return false; + } + + containingType = parentType; + + return true; + } +} \ No newline at end of file diff --git a/src/EntityFrameworkCore.Projectables.Generator/EntityFrameworkCore.Projectables.Generator.csproj b/src/EntityFrameworkCore.Projectables.Generator/EntityFrameworkCore.Projectables.Generator.csproj index af7739e..d3a9683 100644 --- a/src/EntityFrameworkCore.Projectables.Generator/EntityFrameworkCore.Projectables.Generator.csproj +++ b/src/EntityFrameworkCore.Projectables.Generator/EntityFrameworkCore.Projectables.Generator.csproj @@ -11,6 +11,7 @@ + diff --git a/src/EntityFrameworkCore.Projectables.Generator/Infrastructure/Diagnostics.cs b/src/EntityFrameworkCore.Projectables.Generator/Infrastructure/Diagnostics.cs index ba87028..9fab2f2 100644 --- a/src/EntityFrameworkCore.Projectables.Generator/Infrastructure/Diagnostics.cs +++ b/src/EntityFrameworkCore.Projectables.Generator/Infrastructure/Diagnostics.cs @@ -98,6 +98,5 @@ static internal class Diagnostics messageFormat: "Factory method '{0}' creates and returns an instance of the containing class via object initializer. Consider converting it to a [Projectable] constructor.", category: "Design", DiagnosticSeverity.Info, - isEnabledByDefault: true, - helpLinkUri: "https://github.com/koenbeuk/EntityFrameworkCore.Projectables"); + isEnabledByDefault: true); } \ No newline at end of file diff --git a/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs b/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs index 41fbfe4..38821b1 100644 --- a/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs +++ b/src/EntityFrameworkCore.Projectables.Generator/ProjectionExpressionGenerator.cs @@ -5,6 +5,7 @@ using Microsoft.CodeAnalysis.Text; using System.Collections.Immutable; using System.Text; +using EntityFrameworkCore.Projectables.CodeFixes; using EntityFrameworkCore.Projectables.Generator.Comparers; using EntityFrameworkCore.Projectables.Generator.Interpretation; using EntityFrameworkCore.Projectables.Generator.Models; @@ -186,9 +187,12 @@ private static void Execute( } // Report EFP0012 when a [Projectable] method is a factory that could be a constructor. - if (member is MethodDeclarationSyntax factoryCandidate) + if (member is MethodDeclarationSyntax factoryCandidate && SyntaxHelpers.TryGetFactoryMethodPattern(factoryCandidate, out _)) { - ReportFactoryMethodDiagnosticIfApplicable(factoryCandidate, context); + context.ReportDiagnostic(Diagnostic.Create( + Infrastructure.Diagnostics.FactoryMethodShouldBeConstructor, + factoryCandidate.Identifier.GetLocation(), + factoryCandidate.Identifier.Text)); } var generatedClassName = ProjectionExpressionClassNameGenerator.GenerateName(projectable.ClassNamespace, projectable.NestedInClassNames, projectable.MemberName, projectable.ParameterTypeNames); @@ -290,70 +294,6 @@ static TypeArgumentListSyntax GetLambdaTypeArgumentListSyntax(ProjectableDescrip } } - /// - /// Reports EFP0012 when is a [Projectable] factory - /// method whose expression body is a pure object-initializer expression targeting the - /// containing class (e.g. public static MyObj Create(…) => new MyObj { … }). - /// - private static void ReportFactoryMethodDiagnosticIfApplicable( - MethodDeclarationSyntax method, - SourceProductionContext context) - { - if (method.Parent is not TypeDeclarationSyntax) - { - return; - } - - if (method.ExpressionBody is null) - { - return; - } - - if (method.ExpressionBody.Expression is not ObjectCreationExpressionSyntax creation) - { - return; - } - - // Only pure object-initializer bodies — no constructor arguments on the new expression. - if (creation.ArgumentList?.Arguments.Count > 0) - { - return; - } - - if (creation.Initializer is null) - { - return; - } - - if (memberSymbol is not IMethodSymbol methodSymbol) - { - return; - } - - var containingTypeSymbol = methodSymbol.ContainingType; - if (containingTypeSymbol is null) - { - return; - } - - var createdTypeSymbol = semanticModel.GetTypeInfo(creation).Type; - if (createdTypeSymbol is null) - { - return; - } - - if (!SymbolEqualityComparer.Default.Equals(methodSymbol.ReturnType, containingTypeSymbol) - || !SymbolEqualityComparer.Default.Equals(createdTypeSymbol, containingTypeSymbol)) - { - return; - } - - context.ReportDiagnostic(Diagnostic.Create( - Infrastructure.Diagnostics.FactoryMethodShouldBeConstructor, - method.Identifier.GetLocation(), - method.Identifier.Text)); - } - /// /// Extracts a from a member declaration. /// Returns null when the member does not have [Projectable], is an extension member, From 4b9af34e35230665a1918783aecdee5a079d6db3 Mon Sep 17 00:00:00 2001 From: "fabien.menager" Date: Mon, 16 Mar 2026 23:19:50 +0100 Subject: [PATCH 06/11] Fix refactorings in the same file --- .../FactoryMethodTransformationHelper.cs | 107 +++++++++++++- .../CodeFixTestBase.cs | 38 ++++- ...pertyExpressions_ArePreserved.verified.txt | 25 ++++ ...rvesInitializerInlineComments.verified.txt | 20 +++ ...PreservesLeadingXmlDocComment.verified.txt | 18 +++ ...llers_PreservesCallSiteTrivia.verified.txt | 19 +++ ...t_OnlyReplacesFactoryCallSite.verified.txt | 24 ++++ ...FactoryMethodToCtorCodeRefProviderTests.cs | 135 ++++++++++++++++++ .../RefactoringTestBase.cs | 37 ++++- 9 files changed, 412 insertions(+), 11 deletions(-) create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_ComplexPropertyExpressions_ArePreserved.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesInitializerInlineComments.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesLeadingXmlDocComment.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_PreservesCallSiteTrivia.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_SameDocument_OnlyReplacesFactoryCallSite.verified.txt diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs index 62da78b..22573d8 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs @@ -83,21 +83,118 @@ async static internal Task ConvertToConstructorAndUpdateCallersAsync( } } - // Apply the factory → constructor transformation on the declaring document. var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); if (root is null) { return solution; } - solution = solution.WithDocumentSyntaxRoot( - document.Id, - BuildRootWithConstructor(root, method, containingType)); + // Map annotation → data needed to build the replacement node. + var invocationAnnotations = new Dictionary(); - // Replace each invocation with `new ReturnType(args)` in all caller documents. + var workingRoot = root; + + if (locationsByDoc.TryGetValue(document.Id, out var declaringDocLocations)) + { + // Use a Dictionary keyed by invocation node to deduplicate: multiple + // reference spans can resolve to the same InvocationExpressionSyntax. + var annotationByInvocation = new Dictionary(); + + foreach (var refLocation in declaringDocLocations) + { + var refNode = root.FindNode(refLocation.Location.SourceSpan); + var invocation = refNode.AncestorsAndSelf() + .OfType() + .FirstOrDefault(); + + if (invocation is null || invocation.Parent is ConditionalAccessExpressionSyntax) + { + continue; + } + + if (annotationByInvocation.ContainsKey(invocation)) + { + // Same invocation reached via a different reference span — skip. + continue; + } + + var ann = new SyntaxAnnotation(); + invocationAnnotations[ann] = ( + invocation.ArgumentList, + invocation.GetLeadingTrivia(), + invocation.GetTrailingTrivia()); + annotationByInvocation[invocation] = ann; + } + + if (annotationByInvocation.Count > 0) + { + // Annotate all unique invocations in one ReplaceNodes pass. + // This does NOT shift spans — only metadata is added. + workingRoot = root.ReplaceNodes( + annotationByInvocation.Keys, + (original, _) => original.WithAdditionalAnnotations(annotationByInvocation[original])); + } + } + + // Re-find method and containingType in workingRoot by their original spans + // (safe because adding annotations does not shift spans). + var currentMethod = workingRoot.FindNode(method.Span) as MethodDeclarationSyntax ?? method; + var currentContainingType = workingRoot.FindNode(containingType.Span) as TypeDeclarationSyntax ?? containingType; + + // Apply the factory → constructor transformation. + // Annotated call-site nodes that live OUTSIDE the transformed type survive + // untouched (annotations are preserved by ReplaceNode). + var transformedRoot = BuildRootWithConstructor(workingRoot, currentMethod, currentContainingType); + + // Replace annotated invocations — found by annotation, not by span. + var finalDeclaringRoot = transformedRoot; + foreach (var annEntry in invocationAnnotations) + { + var ann = annEntry.Key; + var argList = annEntry.Value.ArgList; + var leading = annEntry.Value.Leading; + var trailing = annEntry.Value.Trailing; + + var annotatedInvocation = finalDeclaringRoot + .GetAnnotatedNodes(ann) + .OfType() + .FirstOrDefault(); + + if (annotatedInvocation is null) + { + continue; + } + + // Rewrite: instance.FactoryMethod(args) → new ReturnType(args) + var newCreation = SyntaxFactory + .ObjectCreationExpression( + SyntaxFactory.Token(SyntaxKind.NewKeyword) + .WithTrailingTrivia(SyntaxFactory.Space), + returnTypeSyntax, + argList, + initializer: null) + .WithLeadingTrivia(leading) + .WithTrailingTrivia(trailing); + + finalDeclaringRoot = finalDeclaringRoot.ReplaceNode(annotatedInvocation, newCreation); + } + + solution = solution.WithDocumentSyntaxRoot(document.Id, finalDeclaringRoot); + + // ----------------------------------------------------------------------- + // Other caller documents — spans in these roots are still the original + // unmodified spans, so the existing end-to-start approach is correct. + // ----------------------------------------------------------------------- foreach (var kvp in locationsByDoc) { var docId = kvp.Key; + if (docId == document.Id) + { + // Already handled above via annotations. + continue; + } + var locations = kvp.Value; var callerDoc = solution.GetDocument(docId); diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/CodeFixTestBase.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/CodeFixTestBase.cs index aa39e8c..e0ab23c 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/CodeFixTestBase.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/CodeFixTestBase.cs @@ -15,7 +15,7 @@ namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; /// public abstract class CodeFixTestBase { - protected static Document CreateDocument(string source) + protected static Document CreateDocument([StringSyntax("csharp")] string source) { var workspace = new AdhocWorkspace(); var projectId = ProjectId.CreateNewId(); @@ -31,8 +31,37 @@ protected static Document CreateDocument(string source) return solution.GetDocument(documentId)!; } + /// + /// Like , but also adds all trusted platform assemblies + /// so that the workspace's semantic model can resolve symbols fully. + /// Required by refactoring actions that call + /// SymbolFinder.FindReferencesAsync (e.g. "update callers"). + /// + protected static Document CreateDocumentWithReferences([StringSyntax("csharp")] string source) + { + var workspace = new AdhocWorkspace(); + var projectId = ProjectId.CreateNewId(); + var documentId = DocumentId.CreateNewId(projectId); + + var trustedAssemblies = ((string?)AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") ?? string.Empty) + .Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries) + .Select(p => MetadataReference.CreateFromFile(p)) + .Cast() + .ToList(); + + var solution = workspace.CurrentSolution + .AddProject(projectId, "TestProject", "TestProject", LanguageNames.CSharp) + .WithProjectCompilationOptions( + projectId, + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) + .WithProjectMetadataReferences(projectId, trustedAssemblies) + .AddDocument(documentId, "Test.cs", SourceText.From(source)); + + return solution.GetDocument(documentId)!; + } + private async static Task<(Document Document, IReadOnlyList Actions)> CollectActionsAsync( - string source, + [StringSyntax("csharp")] string source, string diagnosticId, Func locateDiagnosticSpan, CodeFixProvider provider) @@ -70,7 +99,7 @@ protected static Document CreateDocument(string source) /// returned by . /// protected async static Task> GetCodeFixActionsAsync( - string source, + [StringSyntax("csharp")] string source, string diagnosticId, Func locateDiagnosticSpan, CodeFixProvider provider) @@ -84,8 +113,7 @@ protected async static Task> GetCodeFixActionsAsync( /// source text of the resulting document. /// protected async static Task ApplyCodeFixAsync( - [StringSyntax("csharp")] - string source, + [StringSyntax("csharp")] string source, string diagnosticId, Func locateDiagnosticSpan, CodeFixProvider provider, diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_ComplexPropertyExpressions_ArePreserved.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_ComplexPropertyExpressions_ArePreserved.verified.txt new file mode 100644 index 0000000..603d167 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_ComplexPropertyExpressions_ArePreserved.verified.txt @@ -0,0 +1,25 @@ + +namespace Foo { + class Src { + public int X { get; set; } + public int Y { get; set; } + public bool IsActive { get; set; } + public string Name { get; set; } + } + class Dest { + public Dest() + { + } + + public int Sum { get; set; } + public int Toggle { get; set; } + public string Label { get; set; } + [Projectable] + public Dest(Src src) + { + Sum = src.X + src.Y; + Toggle = src.IsActive ? 1 : 0; + Label = src.Name ?? "unknown"; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesInitializerInlineComments.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesInitializerInlineComments.verified.txt new file mode 100644 index 0000000..70831ad --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesInitializerInlineComments.verified.txt @@ -0,0 +1,20 @@ + +namespace Foo { + class Src { public int A { get; set; } public int B { get; set; } } + class Dest { + public Dest() + { + } + + public int A { get; set; } + public int B { get; set; } + [Projectable] + public Dest(Src src) + { + // primary field + A = src.A; + B = src.B // secondary field +; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesLeadingXmlDocComment.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesLeadingXmlDocComment.verified.txt new file mode 100644 index 0000000..b3b81b9 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesLeadingXmlDocComment.verified.txt @@ -0,0 +1,18 @@ + +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() + { + } + + public int A { get; set; } + /// Creates a new from a . + /// The source object. + [Projectable] + public Dest(Src src) + { + A = src.A; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_PreservesCallSiteTrivia.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_PreservesCallSiteTrivia.verified.txt new file mode 100644 index 0000000..397a09f --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_PreservesCallSiteTrivia.verified.txt @@ -0,0 +1,19 @@ + +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + [Projectable] + public Dest(Src src) + { + A = src.A; + } + } + class Consumer { + Dest Use(Src src) { + // map the source + return new Dest(src); // inline comment + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_SameDocument_OnlyReplacesFactoryCallSite.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_SameDocument_OnlyReplacesFactoryCallSite.verified.txt new file mode 100644 index 0000000..17ae24f --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_SameDocument_OnlyReplacesFactoryCallSite.verified.txt @@ -0,0 +1,24 @@ + +namespace Foo { + class Src { public int A { get; set; } public int B { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + public int B { get; set; } + [Projectable] + public Dest(Src src) + { + A = src.A; + B = src.B; + } + } + class Other { + public static int Compute() => 42; + } + class Consumer { + void Setup() { + var d = new Dest(new Src { A = 1, B = 2 }); + var x = Other.Compute(); + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs index 2ee3629..f7a476d 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs @@ -240,4 +240,139 @@ class MyObj { Assert.Contains("constructor", actions[0].Title, StringComparison.OrdinalIgnoreCase); Assert.Contains("callers", actions[1].Title, StringComparison.OrdinalIgnoreCase); } + + // ──────────────────────────────────────────────────────────────────────────── + // Action 1 — convert factory method to constructor AND update callers + // ──────────────────────────────────────────────────────────────────────────── + + /// + /// Regression test: when the declaring document also contains call sites, + /// BuildRootWithConstructor shifts all spans (it removes the factory method and + /// inserts a constructor). Only Dest.Map(…) must be rewritten — + /// unrelated invocations such as Other.Compute() must be left intact. + /// + [Fact] + public Task UpdateCallers_SameDocument_OnlyReplacesFactoryCallSite() => + Verifier.Verify( + ApplyRefactoringAsync( + CreateDocumentWithReferences(@" +namespace Foo { + class Src { public int A { get; set; } public int B { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + public int B { get; set; } + [Projectable] + public static Dest Map(Src src) => new Dest { A = src.A, B = src.B }; + } + class Other { + public static int Compute() => 42; + } + class Consumer { + void Setup() { + var d = Dest.Map(new Src { A = 1, B = 2 }); + var x = Other.Compute(); + } + } +}"), + FirstMethodIdentifierSpan, + _provider, + actionIndex: 1)); + + [Fact] + public Task UpdateCallers_PreservesCallSiteTrivia() => + Verifier.Verify( + ApplyRefactoringAsync( + CreateDocumentWithReferences(@" +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + [Projectable] + public static Dest Map(Src src) => new Dest { A = src.A }; + } + class Consumer { + Dest Use(Src src) { + // map the source + return Dest.Map(src); // inline comment + } + } +}"), + FirstMethodIdentifierSpan, + _provider, + actionIndex: 1)); + + // ──────────────────────────────────────────────────────────────────────────── + // Complex initializer expressions and trivia preservation (action 0) + // ──────────────────────────────────────────────────────────────────────────── + + [Fact] + public Task ConvertToConstructor_ComplexPropertyExpressions_ArePreserved() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class Src { + public int X { get; set; } + public int Y { get; set; } + public bool IsActive { get; set; } + public string Name { get; set; } + } + class Dest { + public int Sum { get; set; } + public int Toggle { get; set; } + public string Label { get; set; } + [Projectable] + public static Dest Map(Src src) => new Dest { + Sum = src.X + src.Y, + Toggle = src.IsActive ? 1 : 0, + Label = src.Name ?? ""unknown"" + }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task ConvertToConstructor_PreservesLeadingXmlDocComment() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public int A { get; set; } + /// Creates a new from a . + /// The source object. + [Projectable] + public static Dest Map(Src src) => new Dest { A = src.A }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + [Fact] + public Task ConvertToConstructor_PreservesInitializerInlineComments() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class Src { public int A { get; set; } public int B { get; set; } } + class Dest { + public int A { get; set; } + public int B { get; set; } + [Projectable] + public static Dest Map(Src src) => new Dest { + // primary field + A = src.A, + B = src.B // secondary field + }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); } diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/RefactoringTestBase.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/RefactoringTestBase.cs index 93684e6..f2d0756 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/RefactoringTestBase.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/RefactoringTestBase.cs @@ -74,5 +74,40 @@ protected async static Task ApplyRefactoringAsync( var newRoot = await newDocument.GetSyntaxRootAsync(); return newRoot!.ToFullString(); } -} + /// + /// Overload that accepts an already-created . + /// Use this when the document must carry metadata references (e.g. for + /// "update callers" actions that rely on SymbolFinder.FindReferencesAsync). + /// + protected async static Task ApplyRefactoringAsync( + Document document, + Func locateSpan, + CodeRefactoringProvider provider, + int actionIndex = 0) + { + var root = await document.GetSyntaxRootAsync(); + var span = locateSpan(root!); + + var actions = new List(); + var context = new CodeRefactoringContext( + document, + span, + action => actions.Add(action), + CancellationToken.None); + + await provider.ComputeRefactoringsAsync(context); + + Assert.True( + actions.Count > actionIndex, + $"Expected at least {actionIndex + 1} refactoring action(s) but only {actions.Count} were registered."); + + var action = actions[actionIndex]; + var operations = await action.GetOperationsAsync(CancellationToken.None); + var applyOp = operations.OfType().Single(); + + var newDocument = applyOp.ChangedSolution.GetDocument(document.Id)!; + var newRoot = await newDocument.GetSyntaxRootAsync(); + return newRoot!.ToFullString(); + } +} From 5c76c32930b4789f3e2d58e5e4d51614bd4fb539 Mon Sep 17 00:00:00 2001 From: "fabien.menager" Date: Tue, 17 Mar 2026 21:39:14 +0100 Subject: [PATCH 07/11] Add support for method group refactoring --- .../FactoryMethodTransformationHelper.cs | 199 ++++++++++++++---- ...ethodGroup_MultipleParameters.verified.txt | 20 ++ ...s_MethodGroup_SingleParameter.verified.txt | 18 ++ ...irectInvocationAndMethodGroup.verified.txt | 21 ++ ...FactoryMethodToCtorCodeRefProviderTests.cs | 90 ++++++++ 5 files changed, 303 insertions(+), 45 deletions(-) create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_MultipleParameters.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_SingleParameter.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MixedDirectInvocationAndMethodGroup.verified.txt diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs index 22573d8..62cf986 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs @@ -57,6 +57,7 @@ async static internal Task ConvertToConstructorAndUpdateCallersAsync( } var solution = document.Project.Solution; + var methodParamNames = methodSymbol.Parameters.Select(p => p.Name).ToArray(); var returnType = methodSymbol.ReturnType; var returnTypeSyntax = SyntaxFactory .ParseTypeName(returnType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)) @@ -92,48 +93,83 @@ async static internal Task ConvertToConstructorAndUpdateCallersAsync( // Map annotation → data needed to build the replacement node. var invocationAnnotations = new Dictionary(); + var methodGroupAnnotations = new Dictionary(); var workingRoot = root; if (locationsByDoc.TryGetValue(document.Id, out var declaringDocLocations)) { - // Use a Dictionary keyed by invocation node to deduplicate: multiple - // reference spans can resolve to the same InvocationExpressionSyntax. + // Use Dictionaries keyed by node to deduplicate: multiple reference spans + // can resolve to the same syntax node. var annotationByInvocation = new Dictionary(); + var annotationByMethodGroup = new Dictionary(); foreach (var refLocation in declaringDocLocations) { var refNode = root.FindNode(refLocation.Location.SourceSpan); - var invocation = refNode.AncestorsAndSelf() - .OfType() - .FirstOrDefault(); - if (invocation is null || invocation.Parent is ConditionalAccessExpressionSyntax) + // Determine the method-reference expression: qualified (Class.Method) or simple (Method). + var methodRefExpr = refNode.Parent is MemberAccessExpressionSyntax maExpr && maExpr.Name == refNode + ? maExpr + : (ExpressionSyntax)refNode; + + if (methodRefExpr.Parent is InvocationExpressionSyntax invocation + && invocation.Expression == methodRefExpr) { - continue; + // Direct invocation: Class.Method(args) → new ReturnType(args) + if (invocation.Parent is ConditionalAccessExpressionSyntax) + { + continue; + } + + if (annotationByInvocation.ContainsKey(invocation)) + { + // Same invocation reached via a different reference span — skip. + continue; + } + + var ann = new SyntaxAnnotation(); + invocationAnnotations[ann] = ( + invocation.ArgumentList, + invocation.GetLeadingTrivia(), + invocation.GetTrailingTrivia()); + annotationByInvocation[invocation] = ann; } - - if (annotationByInvocation.ContainsKey(invocation)) + else { - // Same invocation reached via a different reference span — skip. - continue; + // Method group: Class.Method → p1 => new ReturnType(p1) + if (annotationByMethodGroup.ContainsKey(methodRefExpr)) + { + continue; + } + + var ann = new SyntaxAnnotation(); + methodGroupAnnotations[ann] = ( + methodRefExpr.GetLeadingTrivia(), + methodRefExpr.GetTrailingTrivia()); + annotationByMethodGroup[methodRefExpr] = ann; } + } - var ann = new SyntaxAnnotation(); - invocationAnnotations[ann] = ( - invocation.ArgumentList, - invocation.GetLeadingTrivia(), - invocation.GetTrailingTrivia()); - annotationByInvocation[invocation] = ann; + // Merge all nodes-to-annotate into one ReplaceNodes pass (does NOT shift spans). + var nodesToAnnotate = new Dictionary( + annotationByInvocation.Count + annotationByMethodGroup.Count); + foreach (var kvp in annotationByInvocation) + { + nodesToAnnotate[kvp.Key] = kvp.Value; } - if (annotationByInvocation.Count > 0) + foreach (var kvp in annotationByMethodGroup) + { + nodesToAnnotate[kvp.Key] = kvp.Value; + } + + if (nodesToAnnotate.Count > 0) { - // Annotate all unique invocations in one ReplaceNodes pass. - // This does NOT shift spans — only metadata is added. workingRoot = root.ReplaceNodes( - annotationByInvocation.Keys, - (original, _) => original.WithAdditionalAnnotations(annotationByInvocation[original])); + nodesToAnnotate.Keys, + (original, _) => original.WithAdditionalAnnotations(nodesToAnnotate[original])); } } @@ -180,6 +216,29 @@ async static internal Task ConvertToConstructorAndUpdateCallersAsync( finalDeclaringRoot = finalDeclaringRoot.ReplaceNode(annotatedInvocation, newCreation); } + // Replace annotated method groups with lambdas: Class.Method → p => new ReturnType(p) + foreach (var annEntry in methodGroupAnnotations) + { + var ann = annEntry.Key; + var leading = annEntry.Value.Leading; + var trailing = annEntry.Value.Trailing; + + var annotatedMethodGroup = finalDeclaringRoot + .GetAnnotatedNodes(ann) + .FirstOrDefault(); + + if (annotatedMethodGroup is null) + { + continue; + } + + var lambda = BuildMethodGroupLambda(methodParamNames, returnTypeSyntax) + .WithLeadingTrivia(leading) + .WithTrailingTrivia(trailing); + + finalDeclaringRoot = finalDeclaringRoot.ReplaceNode(annotatedMethodGroup, lambda); + } + solution = solution.WithDocumentSyntaxRoot(document.Id, finalDeclaringRoot); // ----------------------------------------------------------------------- @@ -214,34 +273,44 @@ async static internal Task ConvertToConstructorAndUpdateCallersAsync( foreach (var refLocation in locations.OrderByDescending(l => l.Location.SourceSpan.Start)) { var refNode = newCallerRoot.FindNode(refLocation.Location.SourceSpan); - var invocation = refNode.AncestorsAndSelf() - .OfType() - .FirstOrDefault(); - if (invocation is null) - { - continue; - } + // Determine the method-reference expression: qualified (Class.Method) or simple (Method). + var methodRefExpr = refNode.Parent is MemberAccessExpressionSyntax maExpr && maExpr.Name == refNode + ? maExpr + : (ExpressionSyntax)refNode; - // Skip conditional-access invocations like x?.FactoryMethod(...) - // to avoid producing invalid syntax such as x?.new ReturnType(...). - if (invocation.Parent is ConditionalAccessExpressionSyntax) + if (methodRefExpr.Parent is InvocationExpressionSyntax invocation + && invocation.Expression == methodRefExpr) { - continue; + // Skip conditional-access invocations like x?.FactoryMethod(...) + // to avoid producing invalid syntax such as x?.new ReturnType(...). + if (invocation.Parent is ConditionalAccessExpressionSyntax) + { + continue; + } + + // Rewrite: Class.Method(args) → new ReturnType(args) + var newCreation = SyntaxFactory + .ObjectCreationExpression( + SyntaxFactory.Token(SyntaxKind.NewKeyword) + .WithTrailingTrivia(SyntaxFactory.Space), + returnTypeSyntax, + invocation.ArgumentList, + initializer: null) + .WithLeadingTrivia(invocation.GetLeadingTrivia()) + .WithTrailingTrivia(invocation.GetTrailingTrivia()); + + newCallerRoot = newCallerRoot.ReplaceNode(invocation, newCreation); } + else + { + // Method group: Class.Method → p => new ReturnType(p) + var lambda = BuildMethodGroupLambda(methodParamNames, returnTypeSyntax) + .WithLeadingTrivia(methodRefExpr.GetLeadingTrivia()) + .WithTrailingTrivia(methodRefExpr.GetTrailingTrivia()); - // Rewrite: instance.FactoryMethod(args) → new ReturnType(args) - var newCreation = SyntaxFactory - .ObjectCreationExpression( - SyntaxFactory.Token(SyntaxKind.NewKeyword) - .WithTrailingTrivia(SyntaxFactory.Space), - returnTypeSyntax, - invocation.ArgumentList, - initializer: null) - .WithLeadingTrivia(invocation.GetLeadingTrivia()) - .WithTrailingTrivia(invocation.GetTrailingTrivia()); - - newCallerRoot = newCallerRoot.ReplaceNode(invocation, newCreation); + newCallerRoot = newCallerRoot.ReplaceNode(methodRefExpr, lambda); + } } solution = solution.WithDocumentSyntaxRoot(docId, newCallerRoot); @@ -327,5 +396,45 @@ private static SyntaxTokenList GetConstructorModifiers(MethodDeclarationSyntax m return SyntaxFactory.TokenList(filteredModifiers); } + + /// + /// Builds a lambda expression that wraps a constructor call, for use when a factory + /// method is referenced as a method group (e.g. Select(MyType.Create)). + /// + /// Single parameter → simple lambda: p => new ReturnType(p) + /// Multiple parameters → parenthesised lambda: (p1, p2) => new ReturnType(p1, p2) + /// + /// + private static LambdaExpressionSyntax BuildMethodGroupLambda( + string[] paramNames, + TypeSyntax returnTypeSyntax) + { + var parameters = paramNames + .Select(name => SyntaxFactory.Parameter(SyntaxFactory.Identifier(name))) + .ToArray(); + + var arguments = paramNames + .Select(name => SyntaxFactory.Argument(SyntaxFactory.IdentifierName(name))) + .ToArray(); + + var objectCreation = SyntaxFactory.ObjectCreationExpression( + SyntaxFactory.Token(SyntaxKind.NewKeyword).WithTrailingTrivia(SyntaxFactory.Space), + returnTypeSyntax, + SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(arguments)), + initializer: null); + + if (parameters.Length == 1) + { + return SyntaxFactory + .SimpleLambdaExpression(parameters[0], objectCreation) + .WithAdditionalAnnotations(Formatter.Annotation); + } + + return SyntaxFactory + .ParenthesizedLambdaExpression( + SyntaxFactory.ParameterList(SyntaxFactory.SeparatedList(parameters)), + objectCreation) + .WithAdditionalAnnotations(Formatter.Annotation); + } } diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_MultipleParameters.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_MultipleParameters.verified.txt new file mode 100644 index 0000000..be21978 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_MultipleParameters.verified.txt @@ -0,0 +1,20 @@ + +using System; +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + public int Offset { get; set; } + [Projectable] + public Dest(Src src, int offset) + { + A = src.A; + Offset = offset; + } + } + class Consumer { + void Register(Func factory) { } + void Setup() => Register((src, offset) => new Dest(src, offset)); + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_SingleParameter.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_SingleParameter.verified.txt new file mode 100644 index 0000000..5d3fa21 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_SingleParameter.verified.txt @@ -0,0 +1,18 @@ + +using System.Collections.Generic; +using System.Linq; +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + [Projectable] + public Dest(Src src) + { + A = src.A; + } + } + class Consumer { + Dest[] Use(IEnumerable items) => items.Select(src => new Dest(src)).ToArray(); + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MixedDirectInvocationAndMethodGroup.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MixedDirectInvocationAndMethodGroup.verified.txt new file mode 100644 index 0000000..f6a1c1d --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MixedDirectInvocationAndMethodGroup.verified.txt @@ -0,0 +1,21 @@ + +using System.Collections.Generic; +using System.Linq; +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + [Projectable] + public Dest(Src src) + { + A = src.A; + } + } + class Consumer { + void Setup(IEnumerable items) { + var d = new Dest(new Src { A = 1 }); // direct invocation + var all = items.Select(src => new Dest(src)).ToArray(); // method group + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs index f7a476d..4ea7928 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs @@ -303,6 +303,96 @@ Dest Use(Src src) { _provider, actionIndex: 1)); + // ──────────────────────────────────────────────────────────────────────────── + // Action 1 — method group callers (e.g. .Select(Dest.Map)) + // ──────────────────────────────────────────────────────────────────────────── + + /// + /// A single-parameter method group passed to Select must be rewritten to a + /// simple lambda: Dest.Map → src => new Dest(src). + /// + [Fact] + public Task UpdateCallers_MethodGroup_SingleParameter() => + Verifier.Verify( + ApplyRefactoringAsync( + CreateDocumentWithReferences(@" +using System.Collections.Generic; +using System.Linq; +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + [Projectable] + public static Dest Map(Src src) => new Dest { A = src.A }; + } + class Consumer { + Dest[] Use(IEnumerable items) => items.Select(Dest.Map).ToArray(); + } +}"), + FirstMethodIdentifierSpan, + _provider, + actionIndex: 1)); + + /// + /// A multi-parameter method group assigned to a delegate variable must be rewritten + /// to a parenthesised lambda: Dest.Map → (src, offset) => new Dest(src, offset). + /// + [Fact] + public Task UpdateCallers_MethodGroup_MultipleParameters() => + Verifier.Verify( + ApplyRefactoringAsync( + CreateDocumentWithReferences(@" +using System; +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + public int Offset { get; set; } + [Projectable] + public static Dest Map(Src src, int offset) => new Dest { A = src.A, Offset = offset }; + } + class Consumer { + void Register(Func factory) { } + void Setup() => Register(Dest.Map); + } +}"), + FirstMethodIdentifierSpan, + _provider, + actionIndex: 1)); + + /// + /// When the same document has both a direct invocation and a method-group + /// reference to the same factory method, both must be rewritten correctly and + /// independently. + /// + [Fact] + public Task UpdateCallers_MixedDirectInvocationAndMethodGroup() => + Verifier.Verify( + ApplyRefactoringAsync( + CreateDocumentWithReferences(@" +using System.Collections.Generic; +using System.Linq; +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + [Projectable] + public static Dest Map(Src src) => new Dest { A = src.A }; + } + class Consumer { + void Setup(IEnumerable items) { + var d = Dest.Map(new Src { A = 1 }); // direct invocation + var all = items.Select(Dest.Map).ToArray(); // method group + } + } +}"), + FirstMethodIdentifierSpan, + _provider, + actionIndex: 1)); + // ──────────────────────────────────────────────────────────────────────────── // Complex initializer expressions and trivia preservation (action 0) // ──────────────────────────────────────────────────────────────────────────── From b37266b73342ae13a7605c6259017b408a9226fa Mon Sep 17 00:00:00 2001 From: "fabien.menager" Date: Tue, 17 Mar 2026 22:21:44 +0100 Subject: [PATCH 08/11] Apply code review suggestions --- .../FactoryMethodTransformationHelper.cs | 90 +++++++++-- .../SyntaxHelpers.cs | 38 ++++- ...r_WhenOtherExplicitCtorExists.verified.txt | 13 ++ ...odeFix_ImplicitObjectCreation.verified.txt | 16 ++ ...rameterlessCtorIsAlwaysPublic.verified.txt | 15 ++ ...FactoryMethodToCtorCodeFixProviderTests.cs | 71 +++++++++ ...r_WhenOtherExplicitCtorExists.verified.txt | 13 ++ ...ructor_ImplicitObjectCreation.verified.txt | 16 ++ ...rameterlessCtorIsAlwaysPublic.verified.txt | 15 ++ ...ultipleInitializerAssignments.verified.txt | 3 +- ...rvesInitializerInlineComments.verified.txt | 3 +- ...ethodGroup_MultipleParameters.verified.txt | 3 +- ...ameOfReference_IsNotRewritten.verified.txt | 16 ++ ...t_OnlyReplacesFactoryCallSite.verified.txt | 3 +- ...FactoryMethodToCtorCodeRefProviderTests.cs | 140 ++++++++++++++++++ .../FactoryMethodDiagnosticTests.cs | 18 --- 16 files changed, 433 insertions(+), 40 deletions(-) create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_ImplicitObjectCreation.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_InsertedParameterlessCtorIsAlwaysPublic.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_ImplicitObjectCreation.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_InsertedParameterlessCtorIsAlwaysPublic.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_NameOfReference_IsNotRewritten.verified.txt diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs index 62cf986..7a39598 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs @@ -139,6 +139,13 @@ async static internal Task ConvertToConstructorAndUpdateCallersAsync( else { // Method group: Class.Method → p1 => new ReturnType(p1) + // Skip nameof(Class.Method): SymbolFinder returns these locations but + // replacing them with a lambda would produce invalid C#. + if (IsInsideNameOf(methodRefExpr)) + { + continue; + } + if (annotationByMethodGroup.ContainsKey(methodRefExpr)) { continue; @@ -305,6 +312,13 @@ async static internal Task ConvertToConstructorAndUpdateCallersAsync( else { // Method group: Class.Method → p => new ReturnType(p) + // Skip nameof(Class.Method): SymbolFinder returns these locations but + // replacing them with a lambda would produce invalid C#. + if (IsInsideNameOf(methodRefExpr)) + { + continue; + } + var lambda = BuildMethodGroupLambda(methodParamNames, returnTypeSyntax) .WithLeadingTrivia(methodRefExpr.GetLeadingTrivia()) .WithTrailingTrivia(methodRefExpr.GetTrailingTrivia()); @@ -329,7 +343,7 @@ private static SyntaxNode BuildRootWithConstructor( MethodDeclarationSyntax method, TypeDeclarationSyntax containingType) { - var creation = (ObjectCreationExpressionSyntax)method.ExpressionBody!.Expression; + var creation = (BaseObjectCreationExpressionSyntax)method.ExpressionBody!.Expression; var initializer = creation.Initializer!; // Only support simple object-initializer assignments (Prop = value). If there are @@ -341,10 +355,32 @@ private static SyntaxNode BuildRootWithConstructor( } // Convert each object-initializer assignment (Prop = value) to a statement (Prop = value;). - var statements = initializer.Expressions - .OfType() - .Select(a => (StatementSyntax)SyntaxFactory.ExpressionStatement(a)) - .ToArray(); + // Two trivia sources must be preserved: + // 1. The expression's own trailing trivia (e.g. an inline "// comment") must move onto + // the semicolon token so it stays on the same line as the statement terminator. + // 2. The separator comma's trailing trivia (e.g. "\r\n ") carries the newline and + // indentation that separates adjacent items in the initializer list. That trivia + // is lost when we extract only the expressions, so we append it to the semicolon as + // well so the next statement starts on its own correctly-indented line. + static StatementSyntax ToStatement(AssignmentExpressionSyntax a, SyntaxTriviaList separatorTrailing) + { + var exprTrailing = a.GetTrailingTrivia(); + var semicolonTrailing = exprTrailing.AddRange(separatorTrailing); + return SyntaxFactory.ExpressionStatement( + a.WithoutTrailingTrivia(), + SyntaxFactory.Token(SyntaxKind.SemicolonToken).WithTrailingTrivia(semicolonTrailing)); + } + + var exprs = initializer.Expressions; + var statements = new StatementSyntax[exprs.Count]; + for (var i = 0; i < exprs.Count; i++) + { + var a = (AssignmentExpressionSyntax)exprs[i]; + var sepTrivia = i < exprs.SeparatorCount + ? exprs.GetSeparator(i).TrailingTrivia + : default; + statements[i] = ToStatement(a, sepTrivia); + } var ctorModifiers = GetConstructorModifiers(method); @@ -361,17 +397,24 @@ private static SyntaxNode BuildRootWithConstructor( var newMembers = containingType.Members.RemoveAt(methodIndex); newMembers = newMembers.Insert(Math.Min(methodIndex, newMembers.Count), ctor); - // Add an explicit parameterless constructor if the class has none, to avoid breaking - // code that relied on the implicit default constructor. - var hasParamlessCtor = containingType.Members - .OfType() - .Any(c => c.ParameterList.Parameters.Count == 0); + // Add an explicit parameterless constructor only when the class originally had NO + // explicit constructors at all. The C# compiler emits an implicit parameterless + // constructor solely in that case (C# spec §10.11.4), and it is always declared public. + // If the class already had other user-declared constructors the implicit default was + // already suppressed, so we must not introduce a new public overload. + var existingCtors = containingType.Members.OfType().ToArray(); + var hasParamlessCtor = existingCtors.Any(c => c.ParameterList.Parameters.Count == 0); + var hadNoExplicitCtors = existingCtors.Length == 0; - if (!hasParamlessCtor) + if (!hasParamlessCtor && hadNoExplicitCtors) { + // The implicit default ctor is always public (C# spec §10.11.4) regardless of the + // factory method's own accessibility, so hard-code public here. var paramlessCtor = SyntaxFactory .ConstructorDeclaration(containingType.Identifier.WithoutTrivia()) - .WithModifiers(ctorModifiers) + .WithModifiers(SyntaxFactory.TokenList( + SyntaxFactory.Token(SyntaxKind.PublicKeyword) + .WithTrailingTrivia(SyntaxFactory.Space))) .WithParameterList(SyntaxFactory.ParameterList()) .WithBody(SyntaxFactory.Block()) .WithAdditionalAnnotations(Formatter.Annotation) @@ -397,6 +440,29 @@ private static SyntaxTokenList GetConstructorModifiers(MethodDeclarationSyntax m return SyntaxFactory.TokenList(filteredModifiers); } + /// + /// Returns when is the argument of a + /// nameof(…) expression. + /// + /// Roslyn parses nameof(X.Y) as an whose + /// callee is an with the text nameof. + /// still returns such + /// locations, but replacing them with a lambda or object-creation expression would produce + /// invalid C# — they must be skipped. + /// + /// + private static bool IsInsideNameOf(ExpressionSyntax expr) => + expr.Parent is ArgumentSyntax + { + Parent: ArgumentListSyntax + { + Parent: InvocationExpressionSyntax + { + Expression: IdentifierNameSyntax { Identifier.Text: "nameof" } + } + } + }; + /// /// Builds a lambda expression that wraps a constructor call, for use when a factory /// method is referenced as a method group (e.g. Select(MyType.Create)). diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs index 2a4eb92..139a2a1 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs @@ -1,4 +1,6 @@ -using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; namespace EntityFrameworkCore.Projectables.CodeFixes; @@ -8,11 +10,16 @@ static internal class SyntaxHelpers /// Returns when matches the /// factory-method pattern: /// - /// Decorated with [Projectable]. /// Expression body of the form => new ContainingType { … } /// (object initializer only, no constructor arguments in the new /// expression). + /// Static method. /// Return type simple name equals the containing class name. + /// For explicit new T { … }, T must be an unqualified + /// identifier that matches the containing class name. Qualified names such as + /// new Other.MyObj { … } or new global::Other.MyObj { … } are + /// rejected because they cannot be confirmed as the same type without a semantic + /// model. /// /// static internal bool TryGetFactoryMethodPattern( @@ -47,6 +54,33 @@ static internal bool TryGetFactoryMethodPattern( return false; } + // Only allow static factory methods, to keep the code fix simpler + if (!method.Modifiers.Any(m => m.IsKind(SyntaxKind.StaticKeyword))) + { + return false; + } + + // For explicit new T { … }, verify the type is an unqualified name matching the + // containing class. Qualified names (new Other.MyObj { }, new global::Other.MyObj { }) + // are rejected: without a semantic model we cannot confirm they resolve to the same + // type, so this is the conservative safe choice. + // For implicit new() { }, the compiler infers the type from the method's return type, + // which is already validated below, so no additional check is needed here. + if (creation is ObjectCreationExpressionSyntax { Type: var createdType }) + { + var createdTypeName = createdType switch + { + IdentifierNameSyntax id => id.Identifier.Text, + GenericNameSyntax generic => generic.Identifier.Text, + _ => null // QualifiedNameSyntax, AliasQualifiedNameSyntax, etc. — reject + }; + + if (createdTypeName is null || createdTypeName != parentType.Identifier.Text) + { + return false; + } + } + // The method's return type must match the containing type (syntax-level name comparison). var returnTypeName = method.ReturnType switch { diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt new file mode 100644 index 0000000..ad2d0cc --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt @@ -0,0 +1,13 @@ + +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public int Value { get; set; } + public Output(string name) { } + [Projectable] + public Output(Input i) + { + Value = i.Value; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_ImplicitObjectCreation.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_ImplicitObjectCreation.verified.txt new file mode 100644 index 0000000..96789f6 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_ImplicitObjectCreation.verified.txt @@ -0,0 +1,16 @@ + +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public MyObj() + { + } + + public string Prop1 { get; set; } + [Projectable] + public MyObj(OtherObj obj) + { + Prop1 = obj.Prop1; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_InsertedParameterlessCtorIsAlwaysPublic.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_InsertedParameterlessCtorIsAlwaysPublic.verified.txt new file mode 100644 index 0000000..4850265 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_InsertedParameterlessCtorIsAlwaysPublic.verified.txt @@ -0,0 +1,15 @@ + +namespace Foo { + class Input { } + class Output { + public Output() + { + } + + public int Value { get; set; } + [Projectable] + internal Output(Input i) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs index c805728..10a0a32 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs @@ -96,6 +96,77 @@ public Output() { } _provider, actionIndex: 0)); + /// + /// When the class already has at least one explicit constructor (but no parameterless one), + /// the C# compiler did NOT generate an implicit default constructor — so the transformation + /// must NOT insert one, which would unintentionally widen the public surface area. + /// + [Fact] + public Task CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists() => + Verifier.Verify( + ApplyCodeFixAsync( + @" +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public int Value { get; set; } + public Output(string name) { } + [Projectable] + public static Output Create(Input i) => new Output { Value = i.Value }; + } +}", + "EFP0012", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + /// + /// The implicit default constructor is always public (C# spec §10.11.4) regardless of the + /// factory method's accessibility. The inserted explicit parameterless ctor must therefore + /// be public too, even when the factory method is internal or protected. + /// + [Fact] + public Task CodeFix_InsertedParameterlessCtorIsAlwaysPublic() => + Verifier.Verify( + ApplyCodeFixAsync( + @" +namespace Foo { + class Input { } + class Output { + public int Value { get; set; } + [Projectable] + internal static Output Create(Input i) => new Output { }; + } +}", + "EFP0012", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + /// + /// Regression test: implicit object creation (new() { … }) must not throw + /// an — + /// must treat it as + /// rather than casting to the explicit ObjectCreationExpressionSyntax. + /// + [Fact] + public Task CodeFix_ImplicitObjectCreation() => + Verifier.Verify( + ApplyCodeFixAsync( + @" +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public string Prop1 { get; set; } + [Projectable] + public static MyObj Create(OtherObj obj) => new() { Prop1 = obj.Prop1 }; + } +}", + "EFP0012", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + [Fact] public async Task TwoCodeFixActionsAreOffered() { diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt new file mode 100644 index 0000000..ad2d0cc --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt @@ -0,0 +1,13 @@ + +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public int Value { get; set; } + public Output(string name) { } + [Projectable] + public Output(Input i) + { + Value = i.Value; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_ImplicitObjectCreation.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_ImplicitObjectCreation.verified.txt new file mode 100644 index 0000000..96789f6 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_ImplicitObjectCreation.verified.txt @@ -0,0 +1,16 @@ + +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public MyObj() + { + } + + public string Prop1 { get; set; } + [Projectable] + public MyObj(OtherObj obj) + { + Prop1 = obj.Prop1; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_InsertedParameterlessCtorIsAlwaysPublic.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_InsertedParameterlessCtorIsAlwaysPublic.verified.txt new file mode 100644 index 0000000..4850265 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_InsertedParameterlessCtorIsAlwaysPublic.verified.txt @@ -0,0 +1,15 @@ + +namespace Foo { + class Input { } + class Output { + public Output() + { + } + + public int Value { get; set; } + [Projectable] + internal Output(Input i) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt index 60e08bd..28bfeea 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_MultipleInitializerAssignments.verified.txt @@ -11,8 +11,7 @@ namespace Foo { [Projectable] public Dest(Src src) { - A = src.A; - B = src.B; + A = src.A; B = src.B; } } } \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesInitializerInlineComments.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesInitializerInlineComments.verified.txt index 70831ad..6a2348c 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesInitializerInlineComments.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_PreservesInitializerInlineComments.verified.txt @@ -13,8 +13,7 @@ namespace Foo { { // primary field A = src.A; - B = src.B // secondary field -; + B = src.B; // secondary field } } } \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_MultipleParameters.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_MultipleParameters.verified.txt index be21978..c504378 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_MultipleParameters.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_MethodGroup_MultipleParameters.verified.txt @@ -9,8 +9,7 @@ namespace Foo { [Projectable] public Dest(Src src, int offset) { - A = src.A; - Offset = offset; + A = src.A; Offset = offset; } } class Consumer { diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_NameOfReference_IsNotRewritten.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_NameOfReference_IsNotRewritten.verified.txt new file mode 100644 index 0000000..6acb34a --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_NameOfReference_IsNotRewritten.verified.txt @@ -0,0 +1,16 @@ + +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + [Projectable] + public Dest(Src src) + { + A = src.A; + } + } + class Consumer { + string GetMethodName() => nameof(Dest.Map); + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_SameDocument_OnlyReplacesFactoryCallSite.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_SameDocument_OnlyReplacesFactoryCallSite.verified.txt index 17ae24f..0f7363c 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_SameDocument_OnlyReplacesFactoryCallSite.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.UpdateCallers_SameDocument_OnlyReplacesFactoryCallSite.verified.txt @@ -8,8 +8,7 @@ namespace Foo { [Projectable] public Dest(Src src) { - A = src.A; - B = src.B; + A = src.A; B = src.B; } } class Other { diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs index 4ea7928..199d003 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs @@ -111,6 +111,51 @@ public Output() { } _provider, actionIndex: 0)); + /// + /// When the class already has at least one explicit constructor (but no parameterless one), + /// the C# compiler did NOT generate an implicit default constructor — so the transformation + /// must NOT insert one, which would unintentionally widen the public surface area. + /// + [Fact] + public Task ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public int Value { get; set; } + public Output(string name) { } + [Projectable] + public static Output Create(Input i) => new Output { Value = i.Value }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + + /// + /// The implicit default constructor is always public (C# spec §10.11.4) regardless of the + /// factory method's accessibility. The inserted explicit parameterless ctor must therefore + /// be public too, even when the factory method is internal or protected. + /// + [Fact] + public Task ConvertToConstructor_InsertedParameterlessCtorIsAlwaysPublic() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class Input { } + class Output { + public int Value { get; set; } + [Projectable] + internal static Output Create(Input i) => new Output { }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + [Fact] public Task ConvertToConstructor_WithExistingMembers_PreservesMemberOrder() => Verifier.Verify( @@ -218,6 +263,51 @@ class MyObj { Assert.Empty(actions); } + /// + /// Regression: new Other.MyObj { } has a qualified type name that cannot be + /// confirmed as the containing type without a semantic model. The pattern must reject + /// it to avoid a false-positive transformation that would corrupt the class. + /// + [Fact] + public async Task NoRefactoring_WhenCreatedTypeIsQualifiedName() + { + var actions = await GetRefactoringActionsAsync( + @" +namespace Other { class MyObj { } } +namespace Foo { + class MyObj { + [Projectable] + public static MyObj Create() => new Other.MyObj { }; + } +}", + FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + + /// + /// Regression: new global::Other.MyObj { } uses an alias-qualified name that + /// cannot be confirmed as the containing type without a semantic model. + /// + [Fact] + public async Task NoRefactoring_WhenCreatedTypeIsAliasQualifiedName() + { + var actions = await GetRefactoringActionsAsync( + @" +namespace Other { class MyObj { } } +namespace Foo { + class MyObj { + [Projectable] + public static MyObj Create() => new global::Other.MyObj { }; + } +}", + FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + // ──────────────────────────────────────────────────────────────────────────── // Action titles // ──────────────────────────────────────────────────────────────────────────── @@ -393,6 +483,33 @@ void Setup(IEnumerable items) { _provider, actionIndex: 1)); + /// + /// nameof(Dest.Map) is returned by SymbolFinder.FindReferencesAsync as a + /// reference location. It must NOT be rewritten to a lambda — it should be left unchanged + /// (producing a compile-time error that the user can fix manually), rather than generating + /// invalid C# like p => new Dest(p) inside a nameof argument. + /// + [Fact] + public Task UpdateCallers_NameOfReference_IsNotRewritten() => + Verifier.Verify( + ApplyRefactoringAsync( + CreateDocumentWithReferences(@" +namespace Foo { + class Src { public int A { get; set; } } + class Dest { + public Dest() { } + public int A { get; set; } + [Projectable] + public static Dest Map(Src src) => new Dest { A = src.A }; + } + class Consumer { + string GetMethodName() => nameof(Dest.Map); + } +}"), + FirstMethodIdentifierSpan, + _provider, + actionIndex: 1)); + // ──────────────────────────────────────────────────────────────────────────── // Complex initializer expressions and trivia preservation (action 0) // ──────────────────────────────────────────────────────────────────────────── @@ -444,6 +561,29 @@ class Dest { _provider, actionIndex: 0)); + /// + /// Regression test: implicit object creation (new() { … }) must not throw + /// an — + /// must treat it as + /// rather than casting to the explicit ObjectCreationExpressionSyntax. + /// + [Fact] + public Task ConvertToConstructor_ImplicitObjectCreation() => + Verifier.Verify( + ApplyRefactoringAsync( + @" +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public string Prop1 { get; set; } + [Projectable] + public static MyObj Create(OtherObj obj) => new() { Prop1 = obj.Prop1 }; + } +}", + FirstMethodIdentifierSpan, + _provider, + actionIndex: 0)); + [Fact] public Task ConvertToConstructor_PreservesInitializerInlineComments() => Verifier.Verify( diff --git a/tests/EntityFrameworkCore.Projectables.Generator.Tests/FactoryMethodDiagnosticTests.cs b/tests/EntityFrameworkCore.Projectables.Generator.Tests/FactoryMethodDiagnosticTests.cs index 33b491b..da2c2bf 100644 --- a/tests/EntityFrameworkCore.Projectables.Generator.Tests/FactoryMethodDiagnosticTests.cs +++ b/tests/EntityFrameworkCore.Projectables.Generator.Tests/FactoryMethodDiagnosticTests.cs @@ -44,24 +44,6 @@ class MyObj { .GetRoot().FindToken(diag.Location.SourceSpan.Start).ValueText); } - [Fact] - public void ReportsEFP0012_OnInstanceFactoryMethod() - { - var compilation = CreateCompilation(@" -using EntityFrameworkCore.Projectables; -namespace Foo { - class OtherObj { } - class MyObj { - [Projectable] - public MyObj Create(OtherObj o) => new MyObj { }; - } -}"); - var result = RunGenerator(compilation); - - var diag = Assert.Single(result.Diagnostics); - Assert.Equal("EFP0012", diag.Id); - } - [Fact] public void ReportsEFP0012_WithMultipleInitializerAssignments() { From 5dc0521938d89cab02a6d1b0cd28c771dc3e1d6c Mon Sep 17 00:00:00 2001 From: "fabien.menager" Date: Tue, 17 Mar 2026 22:40:20 +0100 Subject: [PATCH 09/11] Factorize test souurces --- ...onstructor_WhenAlreadyPresent.verified.txt | 11 -- ...r_WhenOtherExplicitCtorExists.verified.txt | 13 -- ...FactoryMethodToCtorCodeFixProviderTests.cs | 116 +++----------- ...essConstructor_WhenNoneExists.verified.txt | 14 -- ...onstructor_WhenAlreadyPresent.verified.txt | 11 -- ...r_WhenOtherExplicitCtorExists.verified.txt | 13 -- ...FactoryMethodToCtorCodeRefProviderTests.cs | 151 +++++------------- .../FactoryMethodToCtorSources.cs | 105 ++++++++++++ 8 files changed, 166 insertions(+), 268 deletions(-) delete mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt delete mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt delete mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt delete mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt delete mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorSources.cs diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt deleted file mode 100644 index d460104..0000000 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt +++ /dev/null @@ -1,11 +0,0 @@ - -namespace Foo { - class Input { } - class Output { - public Output() { } - [Projectable] - public Output(Input i) - { - } - } -} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt deleted file mode 100644 index ad2d0cc..0000000 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt +++ /dev/null @@ -1,13 +0,0 @@ - -namespace Foo { - class Input { public int Value { get; set; } } - class Output { - public int Value { get; set; } - public Output(string name) { } - [Projectable] - public Output(Input i) - { - Value = i.Value; - } - } -} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs index 10a0a32..97867a4 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs @@ -1,10 +1,4 @@ -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Text; -using VerifyXunit; -using Xunit; - -namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; +namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; /// /// Tests for (EFP0012). @@ -13,14 +7,7 @@ namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; [UsesVerify] public class FactoryMethodToCtorCodeFixProviderTests : CodeFixTestBase { - private static readonly FactoryMethodToCtorCodeFixProvider _provider = new(); - - private static TextSpan FirstMethodIdentifierSpan(SyntaxNode root) => - root.DescendantNodes() - .OfType() - .First() - .Identifier - .Span; + private readonly static FactoryMethodToCtorCodeFixProvider _provider = new(); [Fact] public void FixableDiagnosticIds_ContainsEFP0012() => @@ -30,17 +17,9 @@ public void FixableDiagnosticIds_ContainsEFP0012() => public Task CodeFix_SimpleStaticFactoryMethod() => Verifier.Verify( ApplyCodeFixAsync( - @" -namespace Foo { - class OtherObj { public string Prop1 { get; set; } } - class MyObj { - public string Prop1 { get; set; } - [Projectable] - public static MyObj Create(OtherObj obj) => new MyObj { Prop1 = obj.Prop1 }; - } -}", + FactoryMethodToCtorSources.SimpleStaticFactoryMethod, "EFP0012", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -48,16 +27,9 @@ class MyObj { public Task CodeFix_PreservesProjectableOptions() => Verifier.Verify( ApplyCodeFixAsync( - @" -namespace Foo { - class OtherObj { } - class MyObj { - [Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore)] - public static MyObj Create(OtherObj obj) => new MyObj { }; - } -}", + FactoryMethodToCtorSources.PreservesProjectableOptions, "EFP0012", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -65,34 +37,19 @@ class MyObj { public Task CodeFix_AddsParameterlessConstructor_WhenNoneExists() => Verifier.Verify( ApplyCodeFixAsync( - @" -namespace Foo { - class Input { } - class Output { - [Projectable] - public static Output Create(Input i) => new Output { }; - } -}", + FactoryMethodToCtorSources.AddsParameterlessConstructor, "EFP0012", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); [Fact] - public Task CodeFix_DoesNotAddParameterlessConstructor_WhenAlreadyPresent() => + public Task CodeFix_DoesNotAddParamLessCtor_WhenAlreadyPresent() => Verifier.Verify( ApplyCodeFixAsync( - @" -namespace Foo { - class Input { } - class Output { - public Output() { } - [Projectable] - public static Output Create(Input i) => new Output { }; - } -}", + FactoryMethodToCtorSources.ParameterlessConstructorAlreadyPresent, "EFP0012", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -102,21 +59,12 @@ public Output() { } /// must NOT insert one, which would unintentionally widen the public surface area. /// [Fact] - public Task CodeFix_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists() => + public Task CodeFix_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists() => Verifier.Verify( ApplyCodeFixAsync( - @" -namespace Foo { - class Input { public int Value { get; set; } } - class Output { - public int Value { get; set; } - public Output(string name) { } - [Projectable] - public static Output Create(Input i) => new Output { Value = i.Value }; - } -}", + FactoryMethodToCtorSources.OtherExplicitCtorExists, "EFP0012", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -129,17 +77,9 @@ public Output(string name) { } public Task CodeFix_InsertedParameterlessCtorIsAlwaysPublic() => Verifier.Verify( ApplyCodeFixAsync( - @" -namespace Foo { - class Input { } - class Output { - public int Value { get; set; } - [Projectable] - internal static Output Create(Input i) => new Output { }; - } -}", + FactoryMethodToCtorSources.InsertedParameterlessCtorIsAlwaysPublic, "EFP0012", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -153,17 +93,9 @@ class Output { public Task CodeFix_ImplicitObjectCreation() => Verifier.Verify( ApplyCodeFixAsync( - @" -namespace Foo { - class OtherObj { public string Prop1 { get; set; } } - class MyObj { - public string Prop1 { get; set; } - [Projectable] - public static MyObj Create(OtherObj obj) => new() { Prop1 = obj.Prop1 }; - } -}", + FactoryMethodToCtorSources.ImplicitObjectCreation, "EFP0012", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -171,15 +103,9 @@ class MyObj { public async Task TwoCodeFixActionsAreOffered() { var actions = await GetCodeFixActionsAsync( - @" -namespace Foo { - class MyObj { - [Projectable] - public static MyObj Create() => new MyObj { }; - } -}", + FactoryMethodToCtorSources.TwoActionsSource, "EFP0012", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Equal(2, actions.Count); @@ -199,7 +125,7 @@ class MyObj { } }", "EFP0012", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Empty(actions); diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt deleted file mode 100644 index 57a096b..0000000 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists.verified.txt +++ /dev/null @@ -1,14 +0,0 @@ - -namespace Foo { - class Input { } - class Output { - public Output() - { - } - - [Projectable] - public Output(Input i) - { - } - } -} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt deleted file mode 100644 index d460104..0000000 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent.verified.txt +++ /dev/null @@ -1,11 +0,0 @@ - -namespace Foo { - class Input { } - class Output { - public Output() { } - [Projectable] - public Output(Input i) - { - } - } -} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt deleted file mode 100644 index ad2d0cc..0000000 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists.verified.txt +++ /dev/null @@ -1,13 +0,0 @@ - -namespace Foo { - class Input { public int Value { get; set; } } - class Output { - public int Value { get; set; } - public Output(string name) { } - [Projectable] - public Output(Input i) - { - Value = i.Value; - } - } -} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs index 199d003..2548edd 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs @@ -1,7 +1,4 @@ -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Text; -using VerifyXunit; +using VerifyXunit; using Xunit; namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; @@ -13,15 +10,7 @@ namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; [UsesVerify] public class FactoryMethodToCtorCodeRefProviderTests : RefactoringTestBase { - private static readonly FactoryMethodToConstructorCodeRefactoringProvider _provider = new(); - - // Locates the span of the first method identifier — the provider walks up to MethodDeclarationSyntax. - private static TextSpan FirstMethodIdentifierSpan(SyntaxNode root) => - root.DescendantNodes() - .OfType() - .First() - .Identifier - .Span; + private readonly static FactoryMethodToConstructorCodeRefactoringProvider _provider = new(); // ──────────────────────────────────────────────────────────────────────────── // Action 0 — convert factory method to constructor (document only) @@ -31,16 +20,8 @@ private static TextSpan FirstMethodIdentifierSpan(SyntaxNode root) => public Task ConvertToConstructor_SimpleFactoryMethod() => Verifier.Verify( ApplyRefactoringAsync( - @" -namespace Foo { - class OtherObj { public string Prop1 { get; set; } } - class MyObj { - public string Prop1 { get; set; } - [Projectable] - public static MyObj Create(OtherObj obj) => new MyObj { Prop1 = obj.Prop1 }; - } -}", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.SimpleStaticFactoryMethod, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -48,15 +29,8 @@ class MyObj { public Task ConvertToConstructor_PreservesProjectableOptions() => Verifier.Verify( ApplyRefactoringAsync( - @" -namespace Foo { - class OtherObj { } - class MyObj { - [Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore)] - public static MyObj Create(OtherObj obj) => new MyObj { }; - } -}", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.PreservesProjectableOptions, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -74,40 +48,25 @@ class Dest { public static Dest Map(Src src) => new Dest { A = src.A, B = src.B }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); [Fact] - public Task ConvertToConstructor_AddsParameterlessConstructor_WhenNoneExists() => + public Task ConvertToConstructor_AddsParamLessCtor_WhenNoneExists() => Verifier.Verify( ApplyRefactoringAsync( - @" -namespace Foo { - class Input { } - class Output { - [Projectable] - public static Output Create(Input i) => new Output { }; - } -}", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.AddsParameterlessConstructor, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); [Fact] - public Task ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenAlreadyPresent() => + public Task ConvertToConstructor_DoesNotAddParamLessCtor_WhenAlreadyPresent() => Verifier.Verify( ApplyRefactoringAsync( - @" -namespace Foo { - class Input { } - class Output { - public Output() { } - [Projectable] - public static Output Create(Input i) => new Output { }; - } -}", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.ParameterlessConstructorAlreadyPresent, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -117,20 +76,11 @@ public Output() { } /// must NOT insert one, which would unintentionally widen the public surface area. /// [Fact] - public Task ConvertToConstructor_DoesNotAddParameterlessConstructor_WhenOtherExplicitCtorExists() => + public Task ConvertToConstructor_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists() => Verifier.Verify( ApplyRefactoringAsync( - @" -namespace Foo { - class Input { public int Value { get; set; } } - class Output { - public int Value { get; set; } - public Output(string name) { } - [Projectable] - public static Output Create(Input i) => new Output { Value = i.Value }; - } -}", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.OtherExplicitCtorExists, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -143,16 +93,8 @@ public Output(string name) { } public Task ConvertToConstructor_InsertedParameterlessCtorIsAlwaysPublic() => Verifier.Verify( ApplyRefactoringAsync( - @" -namespace Foo { - class Input { } - class Output { - public int Value { get; set; } - [Projectable] - internal static Output Create(Input i) => new Output { }; - } -}", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.InsertedParameterlessCtorIsAlwaysPublic, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -170,7 +112,7 @@ class Output { public static Output From(Input i) => new Output { Value = i.Value }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -188,7 +130,7 @@ class MyObj { public MyObj Create() => new MyObj { }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Empty(actions); @@ -206,7 +148,7 @@ class MyObj { public Other Create() => new Other { }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Empty(actions); @@ -223,7 +165,7 @@ class MyObj { public MyObj Create(int x) => new MyObj(x) { }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Empty(actions); @@ -240,7 +182,7 @@ class MyObj { public MyObj Create() => default; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Empty(actions); @@ -257,7 +199,7 @@ class MyObj { public MyObj Create() => new MyObj(); } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Empty(actions); @@ -280,7 +222,7 @@ class MyObj { public static MyObj Create() => new Other.MyObj { }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Empty(actions); @@ -302,7 +244,7 @@ class MyObj { public static MyObj Create() => new global::Other.MyObj { }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Empty(actions); @@ -316,14 +258,8 @@ class MyObj { public async Task TwoActionsAreOffered_WithCorrectTitles() { var actions = await GetRefactoringActionsAsync( - @" -namespace Foo { - class MyObj { - [Projectable] - public static MyObj Create() => new MyObj { }; - } -}", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.TwoActionsSource, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider); Assert.Equal(2, actions.Count); @@ -365,7 +301,7 @@ void Setup() { } } }"), - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 1)); @@ -389,7 +325,7 @@ Dest Use(Src src) { } } }"), - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 1)); @@ -420,7 +356,7 @@ class Consumer { Dest[] Use(IEnumerable items) => items.Select(Dest.Map).ToArray(); } }"), - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 1)); @@ -448,7 +384,7 @@ void Register(Func factory) { } void Setup() => Register(Dest.Map); } }"), - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 1)); @@ -479,7 +415,7 @@ void Setup(IEnumerable items) { } } }"), - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 1)); @@ -506,7 +442,7 @@ class Consumer { string GetMethodName() => nameof(Dest.Map); } }"), - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 1)); @@ -538,7 +474,7 @@ class Dest { }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -557,7 +493,7 @@ class Dest { public static Dest Map(Src src) => new Dest { A = src.A }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -571,16 +507,8 @@ class Dest { public Task ConvertToConstructor_ImplicitObjectCreation() => Verifier.Verify( ApplyRefactoringAsync( - @" -namespace Foo { - class OtherObj { public string Prop1 { get; set; } } - class MyObj { - public string Prop1 { get; set; } - [Projectable] - public static MyObj Create(OtherObj obj) => new() { Prop1 = obj.Prop1 }; - } -}", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.ImplicitObjectCreation, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); @@ -602,7 +530,8 @@ class Dest { }; } }", - FirstMethodIdentifierSpan, + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, _provider, actionIndex: 0)); } + diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorSources.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorSources.cs new file mode 100644 index 0000000..904414d --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorSources.cs @@ -0,0 +1,105 @@ +using System.Diagnostics.CodeAnalysis; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; + +namespace EntityFrameworkCore.Projectables.CodeFixes.Tests; + +/// +/// Source code snippets shared between +/// and . +/// +static internal class FactoryMethodToCtorSources +{ + static internal TextSpan FirstMethodIdentifierSpan(SyntaxNode root) => + root.DescendantNodes() + .OfType() + .First() + .Identifier + .Span; + + [StringSyntax("csharp")] + internal const string SimpleStaticFactoryMethod = @" +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public string Prop1 { get; set; } + [Projectable] + public static MyObj Create(OtherObj obj) => new MyObj { Prop1 = obj.Prop1 }; + } +}"; + + [StringSyntax("csharp")] + internal const string PreservesProjectableOptions = @" +namespace Foo { + class OtherObj { } + class MyObj { + [Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore)] + public static MyObj Create(OtherObj obj) => new MyObj { }; + } +}"; + + [StringSyntax("csharp")] + internal const string AddsParameterlessConstructor = @" +namespace Foo { + class Input { } + class Output { + [Projectable] + public static Output Create(Input i) => new Output { }; + } +}"; + + [StringSyntax("csharp")] + internal const string ParameterlessConstructorAlreadyPresent = @" +namespace Foo { + class Input { } + class Output { + public Output() { } + [Projectable] + public static Output Create(Input i) => new Output { }; + } +}"; + + [StringSyntax("csharp")] + internal const string OtherExplicitCtorExists = @" +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public int Value { get; set; } + public Output(string name) { } + [Projectable] + public static Output Create(Input i) => new Output { Value = i.Value }; + } +}"; + + [StringSyntax("csharp")] + internal const string InsertedParameterlessCtorIsAlwaysPublic = @" +namespace Foo { + class Input { } + class Output { + public int Value { get; set; } + [Projectable] + internal static Output Create(Input i) => new Output { }; + } +}"; + + [StringSyntax("csharp")] + internal const string ImplicitObjectCreation = @" +namespace Foo { + class OtherObj { public string Prop1 { get; set; } } + class MyObj { + public string Prop1 { get; set; } + [Projectable] + public static MyObj Create(OtherObj obj) => new() { Prop1 = obj.Prop1 }; + } +}"; + + [StringSyntax("csharp")] + internal const string TwoActionsSource = @" +namespace Foo { + class MyObj { + [Projectable] + public static MyObj Create() => new MyObj { }; + } +}"; +} \ No newline at end of file From ae40b47ace810e318b919b0ef9b45e4641134afe Mon Sep 17 00:00:00 2001 From: "fabien.menager" Date: Tue, 17 Mar 2026 22:54:49 +0100 Subject: [PATCH 10/11] Factorize test sources --- ...ddParamLessCtor_WhenAlreadyPresent.verified.txt | 11 +++++++++++ ...ssCtor_WhenOtherExplicitCtorExists.verified.txt | 13 +++++++++++++ ...r_AddsParamLessCtor_WhenNoneExists.verified.txt | 14 ++++++++++++++ ...ddParamLessCtor_WhenAlreadyPresent.verified.txt | 11 +++++++++++ ...ssCtor_WhenOtherExplicitCtorExists.verified.txt | 13 +++++++++++++ 5 files changed, 62 insertions(+) create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParamLessCtor_WhenAlreadyPresent.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParamLessCtor_WhenNoneExists.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParamLessCtor_WhenAlreadyPresent.verified.txt create mode 100644 tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists.verified.txt diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParamLessCtor_WhenAlreadyPresent.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParamLessCtor_WhenAlreadyPresent.verified.txt new file mode 100644 index 0000000..d460104 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParamLessCtor_WhenAlreadyPresent.verified.txt @@ -0,0 +1,11 @@ + +namespace Foo { + class Input { } + class Output { + public Output() { } + [Projectable] + public Output(Input i) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists.verified.txt new file mode 100644 index 0000000..ad2d0cc --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.CodeFix_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists.verified.txt @@ -0,0 +1,13 @@ + +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public int Value { get; set; } + public Output(string name) { } + [Projectable] + public Output(Input i) + { + Value = i.Value; + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParamLessCtor_WhenNoneExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParamLessCtor_WhenNoneExists.verified.txt new file mode 100644 index 0000000..57a096b --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_AddsParamLessCtor_WhenNoneExists.verified.txt @@ -0,0 +1,14 @@ + +namespace Foo { + class Input { } + class Output { + public Output() + { + } + + [Projectable] + public Output(Input i) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParamLessCtor_WhenAlreadyPresent.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParamLessCtor_WhenAlreadyPresent.verified.txt new file mode 100644 index 0000000..d460104 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParamLessCtor_WhenAlreadyPresent.verified.txt @@ -0,0 +1,11 @@ + +namespace Foo { + class Input { } + class Output { + public Output() { } + [Projectable] + public Output(Input i) + { + } + } +} \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists.verified.txt b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists.verified.txt new file mode 100644 index 0000000..ad2d0cc --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.ConvertToConstructor_DoesNotAddParamLessCtor_WhenOtherExplicitCtorExists.verified.txt @@ -0,0 +1,13 @@ + +namespace Foo { + class Input { public int Value { get; set; } } + class Output { + public int Value { get; set; } + public Output(string name) { } + [Projectable] + public Output(Input i) + { + Value = i.Value; + } + } +} \ No newline at end of file From 4711fbc47f195f844c22c344dec01975e6bd1bdd Mon Sep 17 00:00:00 2001 From: "fabien.menager" Date: Tue, 17 Mar 2026 23:17:38 +0100 Subject: [PATCH 11/11] Guard against insupported syntax --- .../FactoryMethodTransformationHelper.cs | 6 ++- .../SyntaxHelpers.cs | 11 ++++ ...FactoryMethodToCtorCodeFixProviderTests.cs | 52 +++++++++++++++++++ ...FactoryMethodToCtorCodeRefProviderTests.cs | 50 ++++++++++++++++++ 4 files changed, 117 insertions(+), 2 deletions(-) diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs index 7a39598..f8f560f 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs @@ -347,9 +347,11 @@ private static SyntaxNode BuildRootWithConstructor( var initializer = creation.Initializer!; // Only support simple object-initializer assignments (Prop = value). If there are - // other initializer forms (e.g., collection initializers), bail out to avoid + // other initializer forms (e.g., collection initializers) or assignments whose RHS + // is a nested initializer (e.g. Items = { 1, 2 }), bail out to avoid // producing a constructor that does not preserve behavior. - if (initializer.Expressions.Any(e => e is not AssignmentExpressionSyntax)) + if (initializer.Expressions.Any( + e => e is not AssignmentExpressionSyntax { Right: not InitializerExpressionSyntax })) { return root; } diff --git a/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs b/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs index 139a2a1..5c4a82f 100644 --- a/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs +++ b/src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs @@ -54,6 +54,17 @@ static internal bool TryGetFactoryMethodPattern( return false; } + // Only pure simple-assignment initializers (Prop = value) — no bare collection + // elements (which are not AssignmentExpressionSyntax) and no nested initializer + // assignments (Items = { 1, 2 }) whose RHS is an InitializerExpressionSyntax. + // Converting such entries to statements would produce invalid C#, so we must not + // offer the refactoring at all for these patterns. + if (creation.Initializer.Expressions.Any( + e => e is not AssignmentExpressionSyntax { Right: not InitializerExpressionSyntax })) + { + return false; + } + // Only allow static factory methods, to keep the code fix simpler if (!method.Modifiers.Any(m => m.IsKind(SyntaxKind.StaticKeyword))) { diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs index 97867a4..f5a5744 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs @@ -130,4 +130,56 @@ class MyObj { Assert.Empty(actions); } + + /// + /// Items = { 1, 2 } in an object initializer is an + /// + /// whose RHS is an . + /// Converting it to a statement produces invalid C# (Items = { 1, 2 };), + /// so the code fix must not be offered for this pattern. + /// + [Fact] + public async Task NoCodeFix_WhenInitializerHasNestedCollectionInitializer() + { + var actions = await GetCodeFixActionsAsync( + @" +using System.Collections.Generic; +namespace Foo { + class MyObj { + public List Items { get; set; } + [Projectable] + public static MyObj Create() => new MyObj { Items = { 1, 2 } }; + } +}", + "EFP0012", + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + + /// + /// A mixed initializer that combines a simple assignment with a nested collection + /// initializer (Items = { 1, 2 }) must also be rejected. + /// + [Fact] + public async Task NoCodeFix_WhenMixedSimpleAndNestedCollectionInitializer() + { + var actions = await GetCodeFixActionsAsync( + @" +using System.Collections.Generic; +namespace Foo { + class MyObj { + public int Value { get; set; } + public List Items { get; set; } + [Projectable] + public static MyObj Create(int v) => new MyObj { Value = v, Items = { 1, 2 } }; + } +}", + "EFP0012", + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } } diff --git a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs index 2548edd..78ee2ad 100644 --- a/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs +++ b/tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs @@ -205,6 +205,56 @@ class MyObj { Assert.Empty(actions); } + /// + /// Items = { 1, 2 } in an object initializer is an + /// + /// whose RHS is an . + /// Converting it to a statement produces invalid C# (Items = { 1, 2 };), + /// so the refactoring must not be offered for this pattern. + /// + [Fact] + public async Task NoRefactoring_WhenInitializerHasNestedCollectionInitializer() + { + var actions = await GetRefactoringActionsAsync( + @" +using System.Collections.Generic; +namespace Foo { + class MyObj { + public List Items { get; set; } + [Projectable] + public static MyObj Create() => new MyObj { Items = { 1, 2 } }; + } +}", + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + + /// + /// A mixed initializer that combines a simple assignment with a nested collection + /// initializer (Items = { 1, 2 }) must also be rejected. + /// + [Fact] + public async Task NoRefactoring_WhenMixedSimpleAndNestedCollectionInitializer() + { + var actions = await GetRefactoringActionsAsync( + @" +using System.Collections.Generic; +namespace Foo { + class MyObj { + public int Value { get; set; } + public List Items { get; set; } + [Projectable] + public static MyObj Create(int v) => new MyObj { Value = v, Items = { 1, 2 } }; + } +}", + FactoryMethodToCtorSources.FirstMethodIdentifierSpan, + _provider); + + Assert.Empty(actions); + } + /// /// Regression: new Other.MyObj { } has a qualified type name that cannot be /// confirmed as the containing type without a semantic model. The pattern must reject