Merge pull request #36 from EFNext/fix/issue-35

Support nullable and value type projectables

Author: koenbeuk

src/ExpressiveSharp.CodeFixers/ConvertProjectableToTernaryCodeFixProvider.cs

@@ -0,0 +1,281 @@
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Composition;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Formatting;
+
+namespace ExpressiveSharp.CodeFixers;
+
+/// <summary>
+/// Code fix for EXP0024: the <c>??</c> projectable pattern cannot be applied to a nullable
+/// property type. Rewrites the getter and setter to the ternary <c>_hasFoo ? field : formula</c>
+/// pattern and inserts a private <c>bool _hasFoo</c> flag field into the containing type.
+/// </summary>
+[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ConvertProjectableToTernaryCodeFixProvider))]
+[Shared]
+public sealed class ConvertProjectableToTernaryCodeFixProvider : CodeFixProvider
+{
+    public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create("EXP0024");
+
+    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;
+
+        foreach (var diagnostic in context.Diagnostics)
+        {
+            var node = root.FindNode(diagnostic.Location.SourceSpan);
+            var property = node.FirstAncestorOrSelf<PropertyDeclarationSyntax>();
+            if (property is null)
+                continue;
+
+            if (!TryExtractCoalesceParts(property, out _, out _, out _, out _))
+                continue;
+
+            context.RegisterCodeFix(
+                CodeAction.Create(
+                    title: "Convert to ternary 'has-value flag' pattern",
+                    createChangedDocument: ct => ApplyFixAsync(context.Document, property, ct),
+                    equivalenceKey: "EXP0024_ConvertToTernary"),
+                diagnostic);
+        }
+    }
+
+    private static async Task<Document> ApplyFixAsync(
+        Document document,
+        PropertyDeclarationSyntax property,
+        CancellationToken cancellationToken)
+    {
+        var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+        if (root is null)
+            return document;
+
+        if (!TryExtractCoalesceParts(property, out var getAccessor, out var setAccessor, out var backingFieldRef, out var formula))
+            return document;
+
+        var containingType = property.FirstAncestorOrSelf<TypeDeclarationSyntax>();
+        if (containingType is null)
+            return document;
+
+        var flagName = await ResolveUniqueFlagNameAsync(document, containingType, property, cancellationToken).ConfigureAwait(false);
+
+        var flagIdent = SyntaxFactory.IdentifierName(flagName);
+
+        // Build the new get accessor: `flagName ? <backingFieldRef> : (<formula>)`.
+        var ternary = SyntaxFactory.ConditionalExpression(
+            flagIdent,
+            backingFieldRef!,
+            SyntaxFactory.ParenthesizedExpression(formula!));
+
+        var newGetAccessor = getAccessor!
+            .WithBody(null)
+            .WithExpressionBody(SyntaxFactory.ArrowExpressionClause(ternary))
+            .WithSemicolonToken(SyntaxFactory.Token(SyntaxKind.SemicolonToken));
+
+        // Build the new set/init accessor body: `{ _hasFoo = true; <backingFieldRef> = value; }`.
+        var flagAssignment = SyntaxFactory.ExpressionStatement(
+            SyntaxFactory.AssignmentExpression(
+                SyntaxKind.SimpleAssignmentExpression,
+                flagIdent,
+                SyntaxFactory.LiteralExpression(SyntaxKind.TrueLiteralExpression)));
+
+        var valueAssignment = SyntaxFactory.ExpressionStatement(
+            SyntaxFactory.AssignmentExpression(
+                SyntaxKind.SimpleAssignmentExpression,
+                backingFieldRef!,
+                SyntaxFactory.IdentifierName("value")));
+
+        var newSetAccessor = setAccessor!
+            .WithExpressionBody(null)
+            .WithSemicolonToken(default)
+            .WithBody(SyntaxFactory.Block(flagAssignment, valueAssignment));
+
+        // Replace accessors inside the property's accessor list.
+        var accessorList = property.AccessorList!;
+        var newAccessorList = accessorList.ReplaceNodes(
+            new AccessorDeclarationSyntax[] { getAccessor, setAccessor },
+            (original, _) => ReferenceEquals(original, getAccessor) ? (SyntaxNode)newGetAccessor : newSetAccessor);
+
+        var newProperty = property.WithAccessorList((AccessorListSyntax)newAccessorList);
+
+        // Build the flag field declaration: `private bool _hasFoo;`.
+        var flagField = SyntaxFactory.FieldDeclaration(
+                SyntaxFactory.VariableDeclaration(
+                    SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.BoolKeyword)),
+                    SyntaxFactory.SingletonSeparatedList(SyntaxFactory.VariableDeclarator(flagName))))
+            .WithModifiers(SyntaxFactory.TokenList(SyntaxFactory.Token(SyntaxKind.PrivateKeyword)))
+            .WithAdditionalAnnotations(Formatter.Annotation);
+
+        // Insert the flag field immediately before the property declaration so related state
+        // stays visually grouped.
+        var members = containingType.Members;
+        var propertyIndex = members.IndexOf(property);
+        var newMembers = members
+            .RemoveAt(propertyIndex)
+            .Insert(propertyIndex, newProperty)
+            .Insert(propertyIndex, flagField);
+
+        var newContainingType = containingType.WithMembers(newMembers);
+
+        var newRoot = root.ReplaceNode(containingType, newContainingType);
+        return document.WithSyntaxRoot(newRoot);
+    }
+
+    /// <summary>
+    /// Picks a flag-field name that doesn't collide with an existing member on the containing
+    /// type (including members declared in other <c>partial</c> declarations). The happy path
+    /// returns <c>_has&lt;PropertyName&gt;</c>; on collision a numeric suffix is appended.
+    /// </summary>
+    private static async Task<string> ResolveUniqueFlagNameAsync(
+        Document document,
+        TypeDeclarationSyntax containingType,
+        PropertyDeclarationSyntax property,
+        CancellationToken cancellationToken)
+    {
+        var baseName = $"_has{property.Identifier.Text}";
+
+        var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+        var typeSymbol = semanticModel?.GetDeclaredSymbol(containingType, cancellationToken);
+        if (typeSymbol is null)
+            return baseName;
+
+        var existing = new HashSet<string>(typeSymbol.GetMembers().Select(m => m.Name), System.StringComparer.Ordinal);
+        if (!existing.Contains(baseName))
+            return baseName;
+
+        var suffix = 1;
+        string candidate;
+        do
+        {
+            candidate = $"{baseName}{suffix++}";
+        }
+        while (existing.Contains(candidate));
+        return candidate;
+    }
+
+    /// <summary>
+    /// Inspects the property syntax for the Coalesce-shape Projectable pattern and extracts the
+    /// get accessor, set/init accessor, the backing field reference (the left operand of
+    /// <c>??</c>), and the formula (the right operand). Returns <c>false</c> if the pattern
+    /// doesn't match — in which case the fix is not offered.
+    /// </summary>
+    private static bool TryExtractCoalesceParts(
+        PropertyDeclarationSyntax property,
+        out AccessorDeclarationSyntax? getAccessor,
+        out AccessorDeclarationSyntax? setAccessor,
+        out ExpressionSyntax? backingFieldRef,
+        out ExpressionSyntax? formula)
+    {
+        getAccessor = null;
+        setAccessor = null;
+        backingFieldRef = null;
+        formula = null;
+
+        if (property.AccessorList is null)
+            return false;
+
+        foreach (var accessor in property.AccessorList.Accessors)
+        {
+            if (accessor.IsKind(SyntaxKind.GetAccessorDeclaration))
+                getAccessor = accessor;
+            else if (accessor.IsKind(SyntaxKind.InitAccessorDeclaration) || accessor.IsKind(SyntaxKind.SetAccessorDeclaration))
+                setAccessor = accessor;
+        }
+
+        if (getAccessor is null || setAccessor is null)
+            return false;
+
+        if (!TryGetSingleExpression(getAccessor, out var getBody))
+            return false;
+
+        // getBody must be a coalesce `left ?? right`.
+        if (getBody is not BinaryExpressionSyntax { RawKind: (int)SyntaxKind.CoalesceExpression } coalesce)
+            return false;
+
+        // Left must be either the C# 14 `field` keyword (parsed as FieldExpressionSyntax) or a
+        // manually-declared backing field identifier. The generator's recognizer already
+        // validated the symbol, so we only need to match the syntax shape here.
+        if (!IsBackingFieldReference(coalesce.Left))
+            return false;
+
+        if (!TryGetSingleAssignmentValue(setAccessor, out var setAssignment))
+            return false;
+
+        if (!IsBackingFieldReference(setAssignment.Left)
+            || !BackingFieldReferencesMatch(coalesce.Left, setAssignment.Left))
+            return false;
+
+        backingFieldRef = coalesce.Left;
+        formula = UnwrapParentheses(coalesce.Right);
+        return true;
+    }
+
+    private static bool IsBackingFieldReference(ExpressionSyntax expression) =>
+        expression is IdentifierNameSyntax
+        || expression.IsKind(SyntaxKind.FieldExpression);
+
+    private static bool BackingFieldReferencesMatch(ExpressionSyntax a, ExpressionSyntax b)
+    {
+        // Both `field` keyword references are always equivalent; for identifiers compare text.
+        if (a.IsKind(SyntaxKind.FieldExpression) && b.IsKind(SyntaxKind.FieldExpression))
+            return true;
+        if (a is IdentifierNameSyntax ai && b is IdentifierNameSyntax bi)
+            return ai.Identifier.Text == bi.Identifier.Text;
+        return false;
+    }
+
+    private static bool TryGetSingleExpression(AccessorDeclarationSyntax accessor, out ExpressionSyntax expression)
+    {
+        if (accessor.ExpressionBody is not null)
+        {
+            expression = accessor.ExpressionBody.Expression;
+            return true;
+        }
+
+        if (accessor.Body is { Statements: { Count: 1 } stmts }
+            && stmts[0] is ReturnStatementSyntax { Expression: { } ret })
+        {
+            expression = ret;
+            return true;
+        }
+
+        expression = null!;
+        return false;
+    }
+
+    private static bool TryGetSingleAssignmentValue(AccessorDeclarationSyntax accessor, out AssignmentExpressionSyntax assignment)
+    {
+        if (accessor.ExpressionBody is { Expression: AssignmentExpressionSyntax a1 })
+        {
+            assignment = a1;
+            return true;
+        }
+
+        if (accessor.Body is { Statements: { Count: 1 } stmts }
+            && stmts[0] is ExpressionStatementSyntax { Expression: AssignmentExpressionSyntax a2 })
+        {
+            assignment = a2;
+            return true;
+        }
+
+        assignment = null!;
+        return false;
+    }
+
+    private static ExpressionSyntax UnwrapParentheses(ExpressionSyntax expression)
+    {
+        while (expression is ParenthesizedExpressionSyntax paren)
+            expression = paren.Expression;
+        return expression;
+    }
+}

src/ExpressiveSharp.Generator/Infrastructure/Diagnostics.cs

@@ -180,23 +180,23 @@ static internal class Diagnostics
     public readonly static DiagnosticDescriptor ProjectableGetAccessorPattern = new DiagnosticDescriptor(
         id: "EXP0022",
         title: "Projectable get accessor pattern",
-        messageFormat: "The get accessor of a Projectable property must be of the form '=> field ?? (<formula>)' or '=> _backingField ?? (<formula>)' where _backingField is a private nullable field on the same type. Found: {0}.",
+        messageFormat: "The get accessor of a Projectable property must be of the form '=> field ?? (<formula>)', '=> _backingField ?? (<formula>)', or '=> _hasValueFlag ? field : (<formula>)' where _backingField is a private non-static instance field on the same type and _hasValueFlag is a private non-static non-readonly instance bool field on the same type. Found: {0}.",
         category: "Design",
         DiagnosticSeverity.Error,
         isEnabledByDefault: true);
 
     public readonly static DiagnosticDescriptor ProjectableSetterMustStoreToBackingField = new DiagnosticDescriptor(
         id: "EXP0023",
         title: "Projectable setter must store to backing field",
-        messageFormat: "The init/set accessor of a Projectable property must store the incoming value into the same backing field referenced by the get accessor. Found: {0}.",
+        messageFormat: "The init/set accessor of a Projectable property must store the incoming value into the same backing field referenced by the get accessor (and, for the ternary form, also set the has-value flag to true). Found: {0}.",
         category: "Design",
         DiagnosticSeverity.Error,
         isEnabledByDefault: true);
 
     public readonly static DiagnosticDescriptor ProjectableRequiresNonNullablePropertyType = new DiagnosticDescriptor(
         id: "EXP0024",
-        title: "Projectable requires non-nullable property type",
-        messageFormat: "[Expressive(Projectable = true)] cannot be applied to a property with a nullable type ('{0}'). Nullable types prevent distinguishing 'not materialized' from 'materialized to null'.",
+        title: "Projectable coalesce pattern requires non-nullable property type",
+        messageFormat: "The '??' projectable pattern cannot be applied to a property with a nullable type ('{0}') because null cannot distinguish 'not materialized' from 'materialized to null'. Use the ternary form '=> _hasValue ? field : (<formula>)' (or '=> _hasValue ? _backingField : (<formula>)' with a manual backing field) with a private bool flag instead.",
         category: "Design",
         DiagnosticSeverity.Error,
         isEnabledByDefault: true);
@@ -234,4 +234,12 @@ static internal class Diagnostics
         category: "Design",
         DiagnosticSeverity.Error,
         isEnabledByDefault: true);
+
+    public readonly static DiagnosticDescriptor ProjectableInconsistentGetSetBacking = new DiagnosticDescriptor(
+        id: "EXP0030",
+        title: "Projectable getter and setter reference inconsistent backing storage",
+        messageFormat: "The init/set accessor of a Projectable property must reference the same backing field (and has-value flag, for the ternary form) as the get accessor: {0}",
+        category: "Design",
+        DiagnosticSeverity.Error,
+        isEnabledByDefault: true);
 }

src/ExpressiveSharp.Generator/Interpretation/ExpressiveInterpreter.BodyProcessors.cs

@@ -189,14 +189,9 @@ private static bool TryApplyProjectablePropertyBody(
             return false;
         }
 
-        // EXP0024: must not be nullable. Rejects both `string?` and `int?`/`Nullable<int>`.
-        if (IsNullablePropertyType(propertySymbol.Type))
-        {
-            context.ReportDiagnostic(Diagnostic.Create(
-                Diagnostics.ProjectableRequiresNonNullablePropertyType, propertyLocation,
-                propertySymbol.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)));
-            return false;
-        }
+        // EXP0024 is now deferred until after pattern recognition: it applies only to the
+        // Coalesce shape, because the Ternary shape's has-value flag independently distinguishes
+        // "not materialized" from "materialized to null" and thus supports nullable property types.
 
         // EXP0026: incompatible with the `required` modifier.
         if (propertySymbol.IsRequired)
@@ -255,13 +250,23 @@ private static bool TryApplyProjectablePropertyBody(
 
         if (!ProjectablePatternRecognizer.TryRecognizeGetPattern(
                 propertySymbol, getOperation, context, getAccessor.GetLocation(),
-                out var backingField, out var formulaOperation)
-            || backingField is null
-            || formulaOperation is null)
+                out var getResult))
         {
             return false;
         }
 
+        // EXP0024: the Coalesce shape requires a non-nullable property type, because `null` in the
+        // backing field is the "not materialized" sentinel. The Ternary shape does not have this
+        // constraint — its has-value flag carries the sentinel independently.
+        if (getResult.Shape == ProjectablePatternRecognizer.ProjectableGetShape.Coalesce
+            && IsNullablePropertyType(propertySymbol.Type))
+        {
+            context.ReportDiagnostic(Diagnostic.Create(
+                Diagnostics.ProjectableRequiresNonNullablePropertyType, propertyLocation,
+                propertySymbol.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)));
+            return false;
+        }
+
         // ── Step 4: Validate the set/init accessor pattern ──────────────────
 
         var setOperation = semanticModel.GetOperation(setAccessor);
@@ -274,18 +279,19 @@ private static bool TryApplyProjectablePropertyBody(
         }
 
         if (!ProjectablePatternRecognizer.ValidateSetterPattern(
-                setOperation, backingField, context, setAccessor.GetLocation()))
+                setOperation, getResult, context, setAccessor.GetLocation()))
         {
             return false;
         }
 
         // ── Step 5: Feed the formula through the regular emission pipeline ──
 
-        // The formula's Syntax node is the right operand of the `??` coalesce. Passing it to
-        // EmitExpressionTreeForProperty produces a LambdaExpression factory whose registry key
-        // is the property's getter handle — exactly what ExpressiveReplacer.VisitMember looks up
-        // at runtime.
-        var formulaSyntax = formulaOperation.Syntax;
+        // The formula's Syntax node is the "not materialized" branch of the get accessor (the
+        // right operand of `??` for Coalesce, or the else-branch of the ternary for Ternary).
+        // Passing it to EmitExpressionTreeForProperty produces a LambdaExpression factory whose
+        // registry key is the property's getter handle — exactly what ExpressiveReplacer.VisitMember
+        // looks up at runtime.
+        var formulaSyntax = getResult.Formula.Syntax;
         if (formulaSyntax is null)
         {
             context.ReportDiagnostic(Diagnostic.Create(