Skip to content

Conversation

@thygrrr
Copy link
Contributor

@thygrrr thygrrr commented Feb 1, 2026

Summary by CodeRabbit

  • New Features

    • Support for both generic and non-generic attribute forms ([newtype] and [newtype(typeof(T))]) and consistent constructor/operator forwarding in generated aliases.
    • Generation driven by a unified model for more predictable, consistent output and filenames.
  • Documentation

    • Expanded README and NuGet docs with additional examples demonstrating constructor/operator forwarding and domain-typed examples.
  • Tests

    • Extensive new test suite covering generated output, incremental behavior, formatting, escaping, operator/constructor edge cases, and constructor signature nuances.

✏️ Tip: You can customize this high-level summary in your review settings.

@thygrrr thygrrr requested a review from Copilot February 1, 2026 00:04
@coderabbitai
Copy link

coderabbitai bot commented Feb 1, 2026

Walkthrough

Reworks the generator to a model-driven architecture: adds AliasModel and AliasModelExtractor, replaces symbol-based generation in AliasCodeGenerator, updates AliasGenerator incremental pipelines to consume models, adds EquatableArray<T>, expands tests, and updates docs and packaging references.

Changes

Cohort / File(s) Summary
Core Model Infrastructure
NewType.Generator/AliasModel.cs, NewType.Generator/AliasModelExtractor.cs, NewType.Generator/EquatableArray.cs
Adds AliasModel (immutable record structs) and an extractor that converts Roslyn symbols into model data; introduces EquatableArray<T> for value-equality over immutable arrays and all model-related value types.
Generator Pipeline & Codegen
NewType.Generator/AliasGenerator.cs, NewType.Generator/AliasCodeGenerator.cs
Refactors incremental pipeline to use ForAttributeWithMetadataName for generic and non-generic attribute forms; generator now extracts AliasModel and passes it into AliasCodeGenerator (constructor signature changed). Replaces symbol-heavy codegen with model-driven generation.
Tests & Test Helpers
NewType.Tests/GeneratorTests/*, NewType.Tests/NewType.Tests.csproj
Adds comprehensive generator tests (output, incrementality, constructor signatures, default formatting, string escaping, operator symmetry, write-only properties) and GeneratorTestHelper. Test project gains Roslyn/runtime reference packages and project references to the generator for in-memory testing.
Docs & Packaging
README.md, NewType.Generator/NUGET.md
Updates README and NUGET.md examples and wording to highlight constructor and operator forwarding, expands usage examples and generated-file viewing snippets.
Small/Formatting
other minor files under NewType.Generator and tests
Removes legacy helpers/records tied to symbol approach, adjusts usings/formatting, and updates file naming to use model.TypeDisplayString for emitted files.

Sequence Diagram

sequenceDiagram
    participant Source as Source Code
    participant Syntax as Syntax Provider
    participant Extractor as AliasModelExtractor
    participant Model as AliasModel
    participant Generator as AliasCodeGenerator
    participant Output as Generated File

    Source->>Syntax: discover [newtype] attributes
    Syntax->>Extractor: provide GeneratorAttributeSyntaxContext
    Extractor->>Model: build AliasModel (metadata, members, flags)
    Model->>Generator: supply model for code generation
    Generator->>Generator: render .g.cs text from model
    Generator->>Output: emit generated file (hint/name)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Poem

🐇
I nibble syntax, soft and spry,
Pull models from the midnight sky.
With careful hops I stitch each part,
And push out code from rabbit heart.
Tests clap—carrots for my art.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.75% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Incremental generation' is vague and doesn't clearly convey the main architectural changes, which involve replacing symbol-based code generation with a model-driven approach across multiple generator components. Consider a more specific title that describes the primary architectural change, such as 'Refactor AliasCodeGenerator to use AliasModel for incremental generation' or 'Implement model-driven code generation pipeline for newtype aliases'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch incremental-generation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements incremental generation for the newtype source generator by refactoring it to use ForAttributeWithMetadataName and extracting all Roslyn symbol data into equatable value types (AliasModel and related structs). This architectural change enables proper caching and improves build performance by avoiding unnecessary regeneration when unrelated code changes occur.

Changes:

  • Refactored generator to use ForAttributeWithMetadataName for efficient attribute-based incremental filtering
  • Introduced AliasModel and supporting value types to store extracted symbol data without retaining Roslyn symbols
  • Created EquatableArray<T> wrapper for value-based equality of immutable arrays
  • Added comprehensive test infrastructure with generator tests, incrementality tests, and README example validation
  • Updated documentation examples to demonstrate more realistic usage patterns

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 27 comments.

Show a summary per file
File Description
NewType.Generator/AliasGenerator.cs Refactored to use ForAttributeWithMetadataName with two pipelines (generic and non-generic attributes)
NewType.Generator/AliasModelExtractor.cs New extractor that converts Roslyn symbols to equatable value types during transform phase
NewType.Generator/AliasModel.cs New data models representing all extracted symbol information as value types
NewType.Generator/EquatableArray.cs New wrapper providing value-based equality for immutable arrays
NewType.Generator/AliasCodeGenerator.cs Updated to generate code from AliasModel instead of directly from Roslyn symbols
NewType.Tests/NewType.Tests.csproj Added dependencies for Roslyn APIs and reference assemblies to support generator testing
NewType.Tests/GeneratorTests/GeneratorTestHelper.cs New test helper for running and verifying generator behavior
NewType.Tests/GeneratorTests/ReadmeExampleTests.cs New tests validating README examples compile correctly
NewType.Tests/GeneratorTests/IncrementalityTests.cs New tests verifying incremental generation behavior
NewType.Tests/GeneratorTests/GeneratorOutputTests.cs New tests validating generated code structure and content
README.md Updated examples with more complete usage demonstrations including TableId, PizzasEaten, and Fullness types
NewType.Generator/NUGET.md Synchronized with README changes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +47 to +58
public override int GetHashCode()
{
var a = Array;
var hash = 17;
for (var i = 0; i < a.Length; i++)
{
unchecked
{
hash = hash * 31 + a[i].GetHashCode();
}
}
return hash;
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hash code calculation uses a simple multiplicative hash with constants 17 and 31. While this is functional, for empty arrays this always returns 17 regardless of whether the array is default or explicitly empty. Consider returning a consistent hash code for truly empty arrays, such as 0, to match typical collection behavior.

Copilot uses AI. Check for mistakes.
Comment on lines 357 to 358
if (value is decimal m)
return m.ToString() + "m";
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The decimal.ToString() call on line 358 does not specify a culture, which could produce culture-specific decimal separators (e.g., comma instead of period in some locales). This would generate invalid C# code. Use ToString(CultureInfo.InvariantCulture) to ensure the decimal literal always uses a period as the decimal separator.

Copilot uses AI. Check for mistakes.
Comment on lines 343 to 346
return $"\"{s.Replace("\\", "\\\\").Replace("\"", "\\\"")}\"";

if (value is char c)
return $"'{c}'";
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The string escape handling on line 343 does not cover all necessary escape sequences. While it handles backslashes and quotes, it doesn't escape newlines, tabs, carriage returns, and other control characters that would produce invalid string literals. Consider using a more comprehensive escaping mechanism or a library function for proper C# string literal escaping.

Suggested change
return $"\"{s.Replace("\\", "\\\\").Replace("\"", "\\\"")}\"";
if (value is char c)
return $"'{c}'";
return SymbolDisplay.FormatLiteral(s, quote: true);
if (value is char c)
return SymbolDisplay.FormatLiteral(c, quote: true);

Copilot uses AI. Check for mistakes.
<!-- Roslyn APIs for in-memory compilation in generator tests -->
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.11.0"/>

<!-- Reference assemblies for in-memory compilation (includes System.Numerics etc.) -->
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test project uses Basic.Reference.Assemblies.Net80 for .NET 8.0 reference assemblies, but the project targets net10.0 (line 4 of the file). This mismatch could lead to subtle compatibility issues or missing APIs during test execution. Consider using the appropriate reference assemblies package that matches the target framework, or document why Net80 assemblies are intentionally used for net10.0 testing.

Suggested change
<!-- Reference assemblies for in-memory compilation (includes System.Numerics etc.) -->
<!-- Reference assemblies for in-memory compilation (includes System.Numerics etc.) -->
<!-- NOTE: Using Net80 reference assemblies intentionally for net10.0 tests.
Update this to a matching reference-assemblies package once available,
or if tests require APIs added after .NET 8.0. -->

Copilot uses AI. Check for mistakes.
# `newtype` *(Distinct Type Aliases for C#)*

<p align="center">
<img src="logo.svg" alt="logo, a stylized N with a red and Blue half" width="30%">
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent capitalization: "Blue" should be lowercase "blue" to match "red" in the same sentence.

Suggested change
<img src="logo.svg" alt="logo, a stylized N with a red and Blue half" width="30%">
<img src="logo.svg" alt="logo, a stylized N with a red and blue half" width="30%">

Copilot uses AI. Check for mistakes.
Comment on lines +319 to +325
foreach (var member in typeSymbol.GetMembers())
{
if (member is IMethodSymbol {MethodKind: MethodKind.Constructor, IsImplicitlyDeclared: false} ctor)
{
signatures.Add(GetConstructorSignature(ctor));
}
}
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot uses AI. Check for mistakes.
Comment on lines +367 to +371
foreach (var op in binaryOperators)
{
if (op.Name == "op_Equality")
return true;
}
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot uses AI. Check for mistakes.
Comment on lines +45 to +54
foreach (var attributeData in context.Attributes)
{
var (compilation, aliases) = source;
foreach (var alias in aliases)
var attributeClass = attributeData.AttributeClass;
if (attributeClass is {IsGenericType: true} &&
attributeClass.TypeArguments.Length == 1)
{
GenerateAliasCode(spc, compilation, alias);
var aliasedType = attributeClass.TypeArguments[0];
return AliasModelExtractor.Extract(context, aliasedType);
}
});
}

private static bool IsCandidateType(SyntaxNode node)
{
if (node is StructDeclarationSyntax {AttributeLists.Count: > 0} structDecl)
return structDecl.Modifiers.Any(SyntaxKind.PartialKeyword);

if (node is ClassDeclarationSyntax {AttributeLists.Count: > 0} classDecl)
return classDecl.Modifiers.Any(SyntaxKind.PartialKeyword);

if (node is RecordDeclarationSyntax {AttributeLists.Count: > 0} recordDecl)
return recordDecl.Modifiers.Any(SyntaxKind.PartialKeyword);

return false;
}
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

Copilot uses AI. Check for mistakes.
else if (SymbolEqualityComparer.Default.Equals(leftType, _alias.AliasedType) &&
!SymbolEqualityComparer.Default.Equals(rightType, _alias.AliasedType))
// Operator with aliased type on left only
else if (op.LeftIsAliasedType && !op.RightIsAliasedType)
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Condition is always false because of access to property LeftIsAliasedType.

Copilot uses AI. Check for mistakes.
return d.ToString("R") + "d";

if (value is decimal m)
return m.ToString() + "m";
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant call to 'ToString' on a String object.

Copilot uses AI. Check for mistakes.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
NewType.Generator/AliasCodeGenerator.cs (1)

160-174: ⚠️ Potential issue | 🟠 Major

Make alias-to-aliased conversion null-safe for class aliases.

For class aliases, value._value will throw when the alias is null. Generate a nullable parameter and handle null explicitly, consistent with existing patterns in AppendEqualityMembers() and other methods that already use _model.IsClass ? "?" : "" for nullable parameters.

Proposed fix
-        _sb.AppendLine($"{indent}public static implicit operator {_model.AliasedTypeFullName}({_model.TypeName} value) => value._value;");
+        if (_model.IsClass)
+        {
+            _sb.AppendLine($"{indent}public static implicit operator {_model.AliasedTypeFullName}({_model.TypeName}? value) => value is null ? default! : value._value;");
+        }
+        else
+        {
+            _sb.AppendLine($"{indent}public static implicit operator {_model.AliasedTypeFullName}({_model.TypeName} value) => value._value;");
+        }
🤖 Fix all issues with AI agents
In `@NewType.Generator/AliasCodeGenerator.cs`:
- Around line 192-207: The generator currently emits Alias op T but not the
symmetric T op Alias when op.LeftIsAliasedType && op.RightIsAliasedType; update
the block that builds operator overloads (the section using opSymbol,
returnTypeStr, _model.TypeName and _model.AliasedTypeFullName) to also emit the
symmetric operator: generate a MethodImpl-aggressive-inlining operator overload
with signature returning returnTypeStr and taking (_model.AliasedTypeFullName
left, _model.TypeName right) and implement it to use left {opSymbol}
right._value (mirroring the existing Alias op T emission), including the same
spacing/blank line formatting as the other generated overloads.

In `@NewType.Generator/AliasModelExtractor.cs`:
- Around line 136-205: The extractor should skip write-only properties: in
ExtractStaticMembers (when handling IPropertySymbol in ExtractStaticMembers)
only include props with a getter (prop.GetMethod != null) because
StaticMemberInfo lacks HasGetter; likewise in ExtractInstanceProperties only add
properties that have a getter (prop.GetMethod != null) instead of relying solely
on setting HasGetter — update the property filtering in both methods
(ExtractStaticMembers and ExtractInstanceProperties) to exclude write-only
properties so generation won't produce invalid get bodies.
- Around line 335-360: In FormatDefaultValue(IParameterSymbol param) ensure
numeric default formatting uses invariant culture: update the float case (value
is float f) to use f.ToString("R", CultureInfo.InvariantCulture) + "f", the
double case (value is double d) to use d.ToString("R",
CultureInfo.InvariantCulture) + "d", and the decimal case (value is decimal m)
to use m.ToString(CultureInfo.InvariantCulture) + "m"; also add the
System.Globalization using so CultureInfo is available.
- Around line 329-333: GetConstructorSignature currently builds a signature from
parameter types only, so constructors that differ by ref/out/in are treated as
duplicates; update GetConstructorSignature to include each parameter's RefKind
(Ref, Out, In, or None) alongside the parameter Type in the joined signature
(use ctor.Parameters and p.RefKind to determine the modifier) so duplicate
detection distinguishes ref/out/in variations.

In `@NewType.Generator/NUGET.md`:
- Line 15: The heading "Basic: typed IDs and counts" is currently an H4 (####)
directly after an H2, causing a level jump; change that heading to H3 (###) so
it follows the previous H2 incrementally and satisfies MD001, i.e., replace the
four-hash prefix before "Basic: typed IDs and counts" with three hashes.

In `@README.md`:
- Around line 21-51: The headings under "Usage" are at h4 and should be h3 to
satisfy MD001; change the "#### Basic: typed IDs and counts" and "#### Typical:
quantities backed by the same data type but distinct domain semantics" headings
to use h3 (###) so heading levels do not skip and remain consistent with the
surrounding "Usage" section.

Comment on lines 192 to 207
// Generate operator with both sides as our type (if both params are the aliased type)
if (SymbolEqualityComparer.Default.Equals(leftType, _alias.AliasedType) &&
SymbolEqualityComparer.Default.Equals(rightType, _alias.AliasedType))
if (op.LeftIsAliasedType && op.RightIsAliasedType)
{
var returnTypeStr = SymbolEqualityComparer.Default.Equals(returnType, _alias.AliasedType)
? _typeName
: returnType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
var returnTypeStr = op.ReturnIsAliasedType
? _model.TypeName
: op.ReturnTypeFullName;

_sb.AppendLine($"{indent}[MethodImpl(MethodImplOptions.AggressiveInlining)]");
_sb.AppendLine($"{indent}public static {returnTypeStr} operator {opSymbol}({_typeName} left, {_typeName} right) => left._value {opSymbol} right._value;");
_sb.AppendLine($"{indent}public static {returnTypeStr} operator {opSymbol}({_model.TypeName} left, {_model.TypeName} right) => left._value {opSymbol} right._value;");
_sb.AppendLine();

// Also generate alias op T for cross-type interop
// e.g. Position + Velocity works via Position.+(Position, Vector3) with Velocity→Vector3
_sb.AppendLine($"{indent}[MethodImpl(MethodImplOptions.AggressiveInlining)]");
_sb.AppendLine($"{indent}public static {returnTypeStr} operator {opSymbol}({_typeName} left, {_aliasedTypeFullName} right) => left._value {opSymbol} right;");
_sb.AppendLine($"{indent}public static {returnTypeStr} operator {opSymbol}({_model.TypeName} left, {_model.AliasedTypeFullName} right) => left._value {opSymbol} right;");
_sb.AppendLine();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "AliasCodeGenerator.cs" | head -5

Repository: outfox/newtype

Length of output: 99


🏁 Script executed:

wc -l ./NewType.Generator/AliasCodeGenerator.cs

Repository: outfox/newtype

Length of output: 103


🏁 Script executed:

sed -n '180,220p' ./NewType.Generator/AliasCodeGenerator.cs | cat -n

Repository: outfox/newtype

Length of output: 2509


🏁 Script executed:

sed -n '180,250p' ./NewType.Generator/AliasCodeGenerator.cs | cat -n

Repository: outfox/newtype

Length of output: 4106


🏁 Script executed:

sed -n '200,210p' ./NewType.Generator/AliasCodeGenerator.cs | cat -n

Repository: outfox/newtype

Length of output: 887


🏁 Script executed:

# Let's understand the structure better - look at the surrounding code context
sed -n '175,215p' ./NewType.Generator/AliasCodeGenerator.cs | cat -n

Repository: outfox/newtype

Length of output: 2249


🏁 Script executed:

# Let's see the "right only" case to understand the pattern
sed -n '215,245p' ./NewType.Generator/AliasCodeGenerator.cs | cat -n

Repository: outfox/newtype

Length of output: 1997


Generate T op Alias operator for symmetry when both parameters of the original operator are T.

When the aliased type T defines op_* (T, T), only Alias op T is currently emitted. The symmetric T op Alias variant should also be generated to avoid implicit conversion fallback scenarios that would return T instead of maintaining the Alias wrapper type.

Proposed fix
                 _sb.AppendLine($"{indent}[MethodImpl(MethodImplOptions.AggressiveInlining)]");
                 _sb.AppendLine($"{indent}public static {returnTypeStr} operator {opSymbol}({_model.TypeName} left, {_model.AliasedTypeFullName} right) => left._value {opSymbol} right;");
                 _sb.AppendLine();
+
+                _sb.AppendLine($"{indent}[MethodImpl(MethodImplOptions.AggressiveInlining)]");
+                _sb.AppendLine($"{indent}public static {returnTypeStr} operator {opSymbol}({_model.AliasedTypeFullName} left, {_model.TypeName} right) => left {opSymbol} right._value;");
+                _sb.AppendLine();
🤖 Prompt for AI Agents
In `@NewType.Generator/AliasCodeGenerator.cs` around lines 192 - 207, The
generator currently emits Alias op T but not the symmetric T op Alias when
op.LeftIsAliasedType && op.RightIsAliasedType; update the block that builds
operator overloads (the section using opSymbol, returnTypeStr, _model.TypeName
and _model.AliasedTypeFullName) to also emit the symmetric operator: generate a
MethodImpl-aggressive-inlining operator overload with signature returning
returnTypeStr and taking (_model.AliasedTypeFullName left, _model.TypeName
right) and implement it to use left {opSymbol} right._value (mirroring the
existing Alias op T emission), including the same spacing/blank line formatting
as the other generated overloads.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
NewType.Generator/AliasCodeGenerator.cs (1)

160-174: ⚠️ Potential issue | 🟠 Major

Null-safe alias → underlying implicit conversion for class aliases.
For class aliases, passing null into the implicit conversion currently dereferences _value and throws a NullReferenceException. Since class aliases are nullable reference types, the conversion operator must accept a nullable alias parameter and guard against null.

🛠️ Suggested fix
         // Implicit from alias to aliased type
         _sb.AppendLine($"{indent}/// <summary>Implicitly converts from {_model.TypeName} to {_model.AliasedTypeMinimalName}.</summary>");
         _sb.AppendLine($"{indent}[MethodImpl(MethodImplOptions.AggressiveInlining)]");
-        _sb.AppendLine($"{indent}public static implicit operator {_model.AliasedTypeFullName}({_model.TypeName} value) => value._value;");
+        var paramType = _model.IsClass ? $"{_model.TypeName}?" : _model.TypeName;
+        var valueExpr = _model.IsClass ? "value is null ? default : value._value" : "value._value";
+        _sb.AppendLine($"{indent}public static implicit operator {_model.AliasedTypeFullName}({paramType} value) => {valueExpr};");
🤖 Fix all issues with AI agents
In `@NewType.Generator/AliasModelExtractor.cs`:
- Around line 212-256: In ExtractInstanceMethods, skip generic instance methods
so we don't generate invalid forwarders: inside the member loop (where you
already filter by IMethodSymbol, IsStatic, DeclaredAccessibility, MethodKind,
CanBeReferencedByName, and name checks) add a check to continue when the method
has generic type parameters (e.g., method.TypeParameters.Length > 0 or
equivalent) so generic methods are not added to the builder; reference
InstanceMethodInfo/MethodParameterInfo to ensure no further changes to those
types are required.

In `@NewType.Generator/NUGET.md`:
- Line 164: Edit the markdown heading in NUGET.md so the heading reads only "##
Viewing Generated Code" and remove the trailing extraneous text "< If you only
knew now what you didn't know then. >"; locate the line containing the "##
Viewing Generated Code" heading and delete the leftover debug/joke text so the
heading is clean and professional.

In `@NewType.Tests/NewType.Tests.csproj`:
- Around line 20-25: Update the Microsoft.CodeAnalysis.CSharp package reference
in the test project from Version="4.11.0" to Version="5.0.0": locate the
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.11.0"/>
entry in the NewType.Tests.csproj and change the Version attribute to "5.0.0" so
tests and generator instantiation use the updated Roslyn runtime; leave the
Basic.Reference.Assemblies.Net100 reference unchanged.
🧹 Nitpick comments (2)
NewType.Tests/GeneratorTests/ConstructorSignatureTests.cs (1)

24-31: Consider asserting both constructors are forwarded.

The test verifies that compilation succeeds and the value constructor exists, but doesn't explicitly verify that the ref string constructor is also forwarded. Adding an assertion for the ref constructor would make the test more comprehensive and better match the test name.

💡 Suggested enhancement
         Assert.Contains("public ParserAlias(string input)", text);
+        // Also verify the ref constructor is forwarded
+        Assert.Contains("public ParserAlias(ref string input)", text);
NewType.Tests/GeneratorTests/DefaultValueFormattingTests.cs (1)

5-79: LGTM, but consider testing under non-invariant culture.

The tests verify correct output format, but don't actually test the invariant culture behavior by running under a culture that uses comma as decimal separator (e.g., de-DE). Consider enhancing at least one test to explicitly set CultureInfo.CurrentCulture to a non-invariant culture before running the generator.

💡 Example enhancement for stronger culture-independence verification
[Fact]
public void Decimal_Default_Value_Uses_Invariant_Culture_Even_Under_German_Locale()
{
    var originalCulture = CultureInfo.CurrentCulture;
    try
    {
        CultureInfo.CurrentCulture = new CultureInfo("de-DE"); // Uses comma as decimal separator
        
        const string source = """
            using newtype;
            public class Currency
            {
                public decimal Amount { get; }
                public Currency(decimal amount = 1.5m) => Amount = amount;
            }
            [newtype<Currency>]
            public readonly partial struct Price;
            """;

        var result = GeneratorTestHelper.RunGenerator(source);
        var text = result.Results[0].GeneratedSources
            .Single(s => s.HintName.EndsWith("Price.g.cs"))
            .SourceText.ToString();

        Assert.Contains("1.5m", text);
        Assert.DoesNotContain("1,5m", text); // Should not use German format
    }
    finally
    {
        CultureInfo.CurrentCulture = originalCulture;
    }
}

Comment on lines +212 to +256
private static EquatableArray<InstanceMethodInfo> ExtractInstanceMethods(ITypeSymbol aliasedType)
{
var builder = ImmutableArray.CreateBuilder<InstanceMethodInfo>();

foreach (var member in aliasedType.GetMembers())
{
if (member is not IMethodSymbol method)
continue;
if (method.IsStatic || method.DeclaredAccessibility != Accessibility.Public)
continue;
if (method.MethodKind != MethodKind.Ordinary)
continue;
if (!method.CanBeReferencedByName)
continue;
if (method.Name.StartsWith("get_") || method.Name.StartsWith("set_"))
continue;
if (method.Name is "GetHashCode" or "Equals" or "ToString" or "CompareTo")
continue;

var skipReturnWrapping = aliasedType.IsValueType &&
aliasedType.SpecialType != SpecialType.None;

var paramBuilder = ImmutableArray.CreateBuilder<MethodParameterInfo>();
foreach (var p in method.Parameters)
{
paramBuilder.Add(new MethodParameterInfo(
Name: p.Name,
TypeFullName: p.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat),
RefKind: p.RefKind,
IsAliasedType: SymbolEqualityComparer.Default.Equals(p.Type, aliasedType)
));
}

builder.Add(new InstanceMethodInfo(
Name: method.Name,
ReturnTypeFullName: method.ReturnType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat),
ReturnsVoid: method.ReturnsVoid,
ReturnIsAliasedType: SymbolEqualityComparer.Default.Equals(method.ReturnType, aliasedType),
SkipReturnWrapping: skipReturnWrapping,
Parameters: new EquatableArray<MethodParameterInfo>(paramBuilder.ToImmutable())
));
}

return new EquatableArray<InstanceMethodInfo>(builder.ToImmutable());
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, examine the current code in AliasModelExtractor.cs
cd NewType.Generator && wc -l AliasModelExtractor.cs

Repository: outfox/newtype

Length of output: 84


🏁 Script executed:

# Search for forwarder generation logic and how it handles method parameters/returns
rg -n "forwarder\|Forwarder" --type=cs -B2 -A2

Repository: outfox/newtype

Length of output: 40


🏁 Script executed:

# Check the InstanceMethodInfo and MethodParameterInfo structures
rg -n "record.*InstanceMethodInfo\|class.*InstanceMethodInfo\|record.*MethodParameterInfo\|class.*MethodParameterInfo" --type=cs -B2 -A10

Repository: outfox/newtype

Length of output: 40


🏁 Script executed:

# Look for any existing generic method handling or tests
rg -n "IsGenericMethod\|Generic.*Method" --type=cs -B2 -A2

Repository: outfox/newtype

Length of output: 40


🏁 Script executed:

# Check if there are unit tests for the extractor
fd -e ".cs$" -path "*test*" -o -path "*Test*" | head -20

Repository: outfox/newtype

Length of output: 226


🏁 Script executed:

# Find all relevant CS files
find . -name "*.cs" -type f | grep -E "(Alias|Forwarder|Method)" | head -20

Repository: outfox/newtype

Length of output: 334


🏁 Script executed:

# Read the AliasModelExtractor.cs file
cat -n NewType.Generator/AliasModelExtractor.cs | sed -n '200,270p'

Repository: outfox/newtype

Length of output: 3758


🏁 Script executed:

# Search for Forwarder generation or model structures
grep -rn "Forwarder\|forwarder" . --include="*.cs" | head -20

Repository: outfox/newtype

Length of output: 343


🏁 Script executed:

# Search for InstanceMethodInfo definition
grep -rn "InstanceMethodInfo\|MethodParameterInfo" . --include="*.cs" | grep -E "record|class" | head -20

Repository: outfox/newtype

Length of output: 236


🏁 Script executed:

# Look for any tests
find . -name "*Test*.cs" -o -name "*test*.cs" | head -20

Repository: outfox/newtype

Length of output: 967


🏁 Script executed:

# Check InstanceMethodInfo and MethodParameterInfo definitions
cat -n ./NewType.Generator/AliasModel.cs | sed -n '80,110p'

Repository: outfox/newtype

Length of output: 1127


🏁 Script executed:

# View the method forwarder generation code
cat -n ./NewType.Generator/AliasCodeGenerator.cs | sed -n '539,600p'

Repository: outfox/newtype

Length of output: 3128


🏁 Script executed:

# Check if there are any tests for generic instance methods
grep -n "Generic\|generic\|<.*>" ./NewType.Tests/InstanceMemberTests.cs | head -30

Repository: outfox/newtype

Length of output: 40


🏁 Script executed:

# Search for any Matrix or Quaternion alias examples that might use generic methods
cat -n ./NewType.Tests/MatrixAliasTests.cs | head -50

Repository: outfox/newtype

Length of output: 1444


🏁 Script executed:

# Check if TypeParameterCount or related generic properties are referenced anywhere
grep -rn "TypeParameterCount\|IsGenericMethod\|TypeArguments\|TypeParameters" . --include="*.cs"

Repository: outfox/newtype

Length of output: 264


🏁 Script executed:

# Check if IsGenericMethod is used or referenced in the codebase
grep -rn "IsGenericMethod" . --include="*.cs"

Repository: outfox/newtype

Length of output: 40


🏁 Script executed:

# Check what methods Matrix4x4 and Quaternion have (from System.Numerics)
cat -n ./NewType.Tests/MatrixAliasTests.cs ./NewType.Tests/QuaternionAliasTests.cs | grep -E "public|generic|Generic|<" | head -30

Repository: outfox/newtype

Length of output: 818


🏁 Script executed:

# Look for any examples or comments about generic handling
grep -rn "generic\|Generic" . --include="*.cs" -B2 -A2 | head -50

Repository: outfox/newtype

Length of output: 4656


🏁 Script executed:

# Check InstanceMemberTests.cs to see what it tests
wc -l ./NewType.Tests/InstanceMemberTests.cs
cat -n ./NewType.Tests/InstanceMemberTests.cs | head -60

Repository: outfox/newtype

Length of output: 1761


🏁 Script executed:

# Check if there are any actual uses of IMethodSymbol properties to understand the API
grep -rn "method\." ./NewType.Generator/AliasModelExtractor.cs | head -20

Repository: outfox/newtype

Length of output: 1865


🏁 Script executed:

# Verify by searching for IsGeneric* in broader context to see what properties are available
grep -rn "method\.Is" ./NewType.Generator --include="*.cs"

Repository: outfox/newtype

Length of output: 193


🏁 Script executed:

# Check if Vector3 has any generic methods that could be aliased
cat ./NewType.Tests/InstanceMemberTests.cs

Repository: outfox/newtype

Length of output: 1306


🏁 Script executed:

# Search in external documentation - check if IsGenericMethod is valid for IMethodSymbol
# by looking at how other method properties are used in the generator
grep -rn "method\.MethodKind\|method\.Parameters\|method\.ReturnType\|method\.ReturnsVoid" ./NewType.Generator/AliasModelExtractor.cs

Repository: outfox/newtype

Length of output: 1420


Skip generic instance methods to avoid invalid forwarders.

Forwarder generation doesn't emit generic type parameters. If an aliased type declares a generic method like T Method<T>(), the generator would produce public UnderlyingType Method() — missing the generic signature and uncompilable. Since InstanceMethodInfo has no fields to store generic type parameters, skipping generic methods is the correct approach.

Suggested fix
             if (method.MethodKind != MethodKind.Ordinary)
                 continue;
+            if (method.IsGenericMethod)
+                continue;
🤖 Prompt for AI Agents
In `@NewType.Generator/AliasModelExtractor.cs` around lines 212 - 256, In
ExtractInstanceMethods, skip generic instance methods so we don't generate
invalid forwarders: inside the member loop (where you already filter by
IMethodSymbol, IsStatic, DeclaredAccessibility, MethodKind,
CanBeReferencedByName, and name checks) add a check to continue when the method
has generic type parameters (e.g., method.TypeParameters.Length > 0 or
equivalent) so generic methods are not added to the builder; reference
InstanceMethodInfo/MethodParameterInfo to ensure no further changes to those
types are required.

```

## Viewing Generated Code
## Viewing Generated Code< If you only knew now what you didn't know then. >
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove extraneous text from heading.

The heading contains what appears to be leftover debug text or a joke: < If you only knew now what you didn't know then. >. This should be removed for professional documentation.

📝 Proposed fix
-## Viewing Generated Code< If you only knew now what you didn't know then. >
+## Viewing Generated Code
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## Viewing Generated Code< If you only knew now what you didn't know then. >
## Viewing Generated Code
🤖 Prompt for AI Agents
In `@NewType.Generator/NUGET.md` at line 164, Edit the markdown heading in
NUGET.md so the heading reads only "## Viewing Generated Code" and remove the
trailing extraneous text "< If you only knew now what you didn't know then. >";
locate the line containing the "## Viewing Generated Code" heading and delete
the leftover debug/joke text so the heading is clean and professional.

Comment on lines +20 to +25

<!-- Roslyn APIs for in-memory compilation in generator tests -->
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.11.0"/>

<!-- Reference assemblies for in-memory compilation (includes System.Numerics etc.) -->
<PackageReference Include="Basic.Reference.Assemblies.Net100" Version="1.8.4"/>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Microsoft.CodeAnalysis.CSharp latest version

💡 Result:

The latest stable NuGet version of Microsoft.CodeAnalysis.CSharp is 5.0.0 (last updated November 18, 2025). [1]

Install:

  • dotnet add package Microsoft.CodeAnalysis.CSharp --version 5.0.0 [1]

Sources:
[1] NuGet Gallery – Microsoft.CodeAnalysis.CSharp (current version 5.0.0) (nuget.org)

Citations:


🌐 Web query:

Basic.Reference.Assemblies.Net100 latest version

💡 Result:

The latest NuGet version of Basic.Reference.Assemblies.Net100 is 1.8.4 (last updated Nov 26, 2025). [1]

Install:

dotnet add package Basic.Reference.Assemblies.Net100 --version 1.8.4

Sources: [1]


Update Microsoft.CodeAnalysis.CSharp to version 5.0.0

The dual project reference pattern correctly enables both build-time integration testing and direct generator instantiation in unit tests. However, Microsoft.CodeAnalysis.CSharp 4.11.0 is outdated; upgrade to the latest version 5.0.0 (released November 2025) for bug fixes, performance improvements, and compatibility with recent language features. Basic.Reference.Assemblies.Net100 1.8.4 is already current.

🤖 Prompt for AI Agents
In `@NewType.Tests/NewType.Tests.csproj` around lines 20 - 25, Update the
Microsoft.CodeAnalysis.CSharp package reference in the test project from
Version="4.11.0" to Version="5.0.0": locate the <PackageReference
Include="Microsoft.CodeAnalysis.CSharp" Version="4.11.0"/> entry in the
NewType.Tests.csproj and change the Version attribute to "5.0.0" so tests and
generator instantiation use the updated Roslyn runtime; leave the
Basic.Reference.Assemblies.Net100 reference unchanged.

@thygrrr thygrrr merged commit 4754b01 into main Feb 1, 2026
2 checks passed
@thygrrr thygrrr deleted the incremental-generation branch February 1, 2026 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants