Conversation
WalkthroughAdds constraint-validation support to the newtype generator: new enum and attribute property to opt-in, model and extractor changes to track constraint flags and validation-method presence, generator and code-emission changes to produce validation checks (and diagnostics), plus tests and analyzer release metadata. Changes
Sequence DiagramsequenceDiagram
participant SourceGenerator as Code Generator
participant AttrParser as Attribute Parser
participant ModelExtractor as Model Extractor
participant CodeGen as Constructor CodeGen
participant Diagnostics as Diagnostic Emitter
SourceGenerator->>AttrParser: Parse newtype attribute (includes ConstraintOptions)
AttrParser->>ModelExtractor: Provide attribute args & aliased type
ModelExtractor->>ModelExtractor: Determine IncludeConstraints, DebugOnlyConstraints, validValidationMethod via HasValidIsValid
ModelExtractor->>SourceGenerator: Return AliasModel with constraint flags
alt IncludeConstraints = true and validValidationMethod = false
SourceGenerator->>Diagnostics: Emit NEWTYPE001 (missing validation method)
end
SourceGenerator->>CodeGen: Request constructor emission for AliasModel
alt IncludeConstraints = true
CodeGen->>CodeGen: Emit multi-line ctor, call validation (IsValid), throw if invalid (DEBUG gating if requested)
else
CodeGen->>CodeGen: Emit simple single-line value constructor
end
CodeGen->>SourceGenerator: Return generated source
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
NewType.Generator/AliasModel.cs (1)
11-64:⚠️ Potential issue | 🟡 MinorUpdate the model summary and validation flag naming.
The summary still says there are no Roslyn types, but
Location?is now part of the model. Also,validValidationMethodbreaks PascalCase consistency; considerHasValidValidationMethod(update usages accordingly).✏️ Suggested doc tweak
-/// Contains only strings, bools, plain enums, and EquatableArrays — no Roslyn symbols. +/// Contains only strings, bools, plain enums, EquatableArrays, and Location for diagnostics.
🤖 Fix all issues with AI agents
In `@NewType.Generator/AliasCodeGenerator.cs`:
- Around line 141-173: When _model.IncludeConstraints is true, forwarded
constructors emitted by AppendForwardedConstructor/for each ctor in
_model.ForwardedConstructors currently assign _value directly and bypass
validation; modify AppendForwardedConstructor so forwarded constructors route to
the validated primary constructor (the ctor taking _model.AliasedTypeFullName
and running AppendConstraintChecker) or explicitly call AppendConstraintChecker
before assigning _value, ensuring every path that sets _value (including
constructors generated for ForwardedConstructors) enforces the same IsValid
checks for _model.TypeName.
In `@NewType.Generator/AliasGenerator.cs`:
- Around line 128-142: The diagnostic is being reported after emitting the
generated file; move the validation check so that you call
context.ReportDiagnostic(...) (using MissingIsValidMethodDiagnostic,
model.Location, model.TypeName, model.AliasedTypeFullName) and return if
model.IncludeConstraints && !model.validValidationMethod before calling
context.AddSource(...) (the current call that constructs fileName and invokes
context.AddSource with SourceText.From(source, Encoding.UTF8)); alternatively,
when the validation fails simply skip the context.AddSource(...) path so no
source is emitted for that model.
In `@NewType.Generator/AliasModelExtractor.cs`:
- Around line 365-379: HasValidIsValid currently detects a matching IsValid
method but does not verify it's static, which lets an instance method slip
through and causes generated code that calls IsValid(value) to fail to compile;
update HasValidIsValid to also check that the found IMethodSymbol.IsStatic is
true when selecting members from targetType via
AliasModel.ConstraintValidationMethodSymbol (and keep the other checks: return
type boolean and single parameter type equals aliasedType) so only static
validation methods are accepted.
In `@NewType.Generator/AnalyzerReleases.Unshipped.md`:
- Around line 1-8: The release entry for NEWTYPE001 uses category `<Undetected>`
but the DiagnosticDescriptor in AliasGenerator (check the
DiagnosticDescriptor/constructor in AliasGenerator.cs where the category is set
to "Unknown") declares the category as "Unknown"; update the Unshipped release
table row for NEWTYPE001 in NewType.Generator/AnalyzerReleases.Unshipped.md to
use `Unknown` as the Category so the table reads: NEWTYPE001 | `Unknown` | Error
| AliasGenerator.
🧹 Nitpick comments (5)
NewType.Generator/AliasAttributeSource.cs (1)
38-79: ExposeConstraintOptionson the non-generic attribute for parity.The non-generic
newtypeAttributeis the fallback API; withoutConstraintOptions, constraint-enabled aliases can’t be expressed there. Consider adding the same property to keep feature parity.🧩 Suggested addition (non-generic attribute)
public NewtypeOptions Options { get; set; } +/// <summary>Controls which constraint features the generator emits.</summary> +public NewtypeConstraintOptions ConstraintOptions { get; set; } + /// <summary> /// Overrides the MethodImplOptions applied to generated members. /// Default is <see cref="global::System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining"/>.NewType.Generator/AliasGenerator.cs (1)
15-24: Consider a more descriptive diagnostic category.The category
"Unknown"at line 20 is not informative. Roslyn diagnostic categories help users filter and organize warnings/errors. Consider using a category like"NewType"or"CodeGeneration"to align with the analyzer's purpose.Suggested change
- category: "Unknown", + category: "NewType",NewType.Tests/ConstraintValidationTests.cs (2)
7-19: Add explicit assertions to verify test intent.These tests rely on "no exception thrown" as the success criterion, but this is implicit. Consider adding explicit assertions to make the test intent clear and catch potential regressions (e.g., if the constructor becomes a no-op).
Suggested improvement
[Fact] public void Direction_Valid() { //doesn't throw var dir = new Direction(new Vector2(0, 1)); + Assert.Equal(new Vector2(0, 1), dir.Value); } [Fact] public void Direction_ValidImplicit() { //doesn't throw Direction dir = new Vector2(0, 1); + Assert.Equal(new Vector2(0, 1), dir.Value); }
33-39: Add assertion to verify the operation result.Similar to the construction tests, this test should assert on the result value to verify the operation completed correctly, not just that it didn't throw.
Suggested improvement
[Fact] public void Direction_ForwardedOperation_ValidResult() { var dir1 = new Direction(new Vector2(0, 1f)); var dir2 = new Direction(new Vector2(0, 1f)); var dir3 = dir1 * dir2; + // Assert expected result of multiplication + Assert.NotNull(dir3); }NewType.Generator/AliasModelExtractor.cs (1)
60-62: Minor: trailing whitespace.Lines 61-62 have trailing whitespace. Consider removing for cleaner formatting.
Suggested fix
var useConstraints = (allOptions.ConstraintOptions & OptionsUseConstraints) != 0; - - + return new AliasModel(
| private static bool HasValidIsValid(ITypeSymbol targetType, ITypeSymbol aliasedType) | ||
| { | ||
| IMethodSymbol? isValidMethod = | ||
| targetType | ||
| .GetMembers(AliasModel.ConstraintValidationMethodSymbol) | ||
| .OfType<IMethodSymbol>() | ||
| .FirstOrDefault(m => | ||
| m.ReturnType.SpecialType == SpecialType.System_Boolean && | ||
| m.Parameters.Length == 1 && | ||
| SymbolEqualityComparer.Default.Equals( | ||
| m.Parameters[0].Type, | ||
| aliasedType)); | ||
|
|
||
| return isValidMethod != null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "IsValid" --type cs NewType.Generator/Repository: outfox/newtype
Length of output: 989
🏁 Script executed:
# Also search for how the validation method is called in generated code
rg -n "static bool IsValid" --type csRepository: outfox/newtype
Length of output: 134
🏁 Script executed:
# Look for the code generator that generates the constraint validation calls
rg -n "ConstraintValidationMethodSymbol" --type cs -A 3 -B 3Repository: outfox/newtype
Length of output: 1847
🏁 Script executed:
# Find the context around line 721 in AliasCodeGenerator.cs to see if IsValid is called in static context
sed -n '710,735p' NewType.Generator/AliasCodeGenerator.csRepository: outfox/newtype
Length of output: 1003
🏁 Script executed:
# Check if there are any other validation method checks or if static is checked elsewhere
rg -n "m\.IsStatic" --type cs NewType.Generator/Repository: outfox/newtype
Length of output: 172
🏁 Script executed:
# Look at the full HasValidIsValid method and how it's used
sed -n '365,379p' NewType.Generator/AliasModelExtractor.csRepository: outfox/newtype
Length of output: 688
🏁 Script executed:
# Look for generated code examples or tests that show how the constraint validation is used
rg -n "if (!IsValid" --type cs -B 5 -A 5Repository: outfox/newtype
Length of output: 124
🏁 Script executed:
# Check if there's any documentation about the expected IsValid signature
rg -n "bool IsValid" --type cs -B 2 -A 2Repository: outfox/newtype
Length of output: 972
Missing IsStatic check in validation method detection.
The generated code at line 721 of AliasCodeGenerator.cs calls IsValid(value) directly without instance qualification. In C#, this can only resolve to a static method on the type or an extension method. Since HasValidIsValid searches for methods on the specific type symbol, it expects a static method. A non-static IsValid method would incorrectly pass the validation check but cause the generated code to fail to compile.
The test case confirms the expected behavior: private static bool IsValid(Vector2 direction) is a static method.
Proposed fix
private static bool HasValidIsValid(ITypeSymbol targetType, ITypeSymbol aliasedType)
{
IMethodSymbol? isValidMethod =
targetType
.GetMembers(AliasModel.ConstraintValidationMethodSymbol)
.OfType<IMethodSymbol>()
.FirstOrDefault(m =>
+ m.IsStatic &&
m.ReturnType.SpecialType == SpecialType.System_Boolean &&
m.Parameters.Length == 1 &&
SymbolEqualityComparer.Default.Equals(
m.Parameters[0].Type,
aliasedType));
return isValidMethod != null;
}📝 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.
| private static bool HasValidIsValid(ITypeSymbol targetType, ITypeSymbol aliasedType) | |
| { | |
| IMethodSymbol? isValidMethod = | |
| targetType | |
| .GetMembers(AliasModel.ConstraintValidationMethodSymbol) | |
| .OfType<IMethodSymbol>() | |
| .FirstOrDefault(m => | |
| m.ReturnType.SpecialType == SpecialType.System_Boolean && | |
| m.Parameters.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals( | |
| m.Parameters[0].Type, | |
| aliasedType)); | |
| return isValidMethod != null; | |
| } | |
| private static bool HasValidIsValid(ITypeSymbol targetType, ITypeSymbol aliasedType) | |
| { | |
| IMethodSymbol? isValidMethod = | |
| targetType | |
| .GetMembers(AliasModel.ConstraintValidationMethodSymbol) | |
| .OfType<IMethodSymbol>() | |
| .FirstOrDefault(m => | |
| m.IsStatic && | |
| m.ReturnType.SpecialType == SpecialType.System_Boolean && | |
| m.Parameters.Length == 1 && | |
| SymbolEqualityComparer.Default.Equals( | |
| m.Parameters[0].Type, | |
| aliasedType)); | |
| return isValidMethod != null; | |
| } |
🤖 Prompt for AI Agents
In `@NewType.Generator/AliasModelExtractor.cs` around lines 365 - 379,
HasValidIsValid currently detects a matching IsValid method but does not verify
it's static, which lets an instance method slip through and causes generated
code that calls IsValid(value) to fail to compile; update HasValidIsValid to
also check that the found IMethodSymbol.IsStatic is true when selecting members
from targetType via AliasModel.ConstraintValidationMethodSymbol (and keep the
other checks: return type boolean and single parameter type equals aliasedType)
so only static validation methods are accepted.
tvenclovas96
force-pushed
the
constraints
branch
from
af180e1 to
7faa613
Compare
|
You are absolutely correct Mr. AI! (Mostly) |
|
Thinking if a separate attribute on the validation method would be more ergonomic/intuitive/clear what is happening 🤔 |
There was a problem hiding this comment.
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/AliasAttributeSource.cs (1)
94-120:⚠️ Potential issue | 🟡 MinorNon-generic attribute is missing
ConstraintOptionsproperty.The generic
newtypeAttribute<T>(line 78) includes the newConstraintOptionsproperty, but the non-genericnewtypeAttributeclass does not. This inconsistency means users of the non-generic attribute cannot enable constraint validation.Proposed fix
/// <summary>Controls which features the generator emits.</summary> public NewtypeOptions Options { get; set; } + /// <summary>Controls which constraint features the generator emits.</summary> + public NewtypeConstraintOptions ConstraintOptions { get; set; } + /// <summary> /// Overrides the MethodImplOptions applied to generated members.
🤖 Fix all issues with AI agents
In `@NewType.Generator/AliasCodeGenerator.cs`:
- Around line 150-163: The constructor body indentation is inconsistent: in
AliasCodeGenerator.cs when _model.IncludeConstraints is true you append the
assignment using three SingleIndent units which ignores memberIndent and
mis-indents types without namespaces; change the code that writes the body (the
AppendConstraintChecker call and the line writing "_value = value;") to build
indentation relative to memberIndent (e.g., append memberIndent then one
SingleIndent before body lines) instead of using SingleIndent repeated three
times so the body is always indented exactly one level deeper than memberIndent;
keep AppendConstraintChecker("SingleIndent", "value") usage but ensure the
subsequent "_value = value;" write uses memberIndent + SingleIndent.
- Around line 690-705: The forwarded constructor uses hardcoded SingleIndent
repetitions which don't respect the current member indentation; change both
branches to build indentation from the existing indent variable plus
SingleIndent(s) instead of appending SingleIndent three times. Specifically, in
the block that calls AppendConstraintChecker and sets _value, replace the
_sb.Append(SingleIndent).Append(SingleIndent).Append(SingleIndent)... call with
something that starts from indent and then appends the needed SingleIndent(s)
(for example _sb.Append(indent).Append(SingleIndent).Append(SingleIndent) before
writing the "_value" assignment), and in the else branch replace the single-line
AppendLine with a properly indented multi-line emission that uses indent +
SingleIndent(s) so the forwarded constructor matches the same member-indent
level; reference AppendConstraintChecker, _sb, indent, SingleIndent,
_model.TypeName and _model.AliasedTypeFullName to locate the lines to change.
- Around line 728-743: AppendConstraintChecker currently hardcodes triple
indentation by calling .Append(indent) three times; instead use the passed-in
indent as the correct "body" indent: emit the if line with a single
.Append(indent).AppendLine($"if (!IsValid({valueName}))"), and emit the throw
line with the body indent plus one more indent (e.g.
.Append(indent).Append(indent).AppendLine(...)) so nesting is correct; keep the
`#if` DEBUG guard logic and the same exception text referencing _model.TypeName
and _model.AliasedTypeMinimalName and write to _sb as before.
🧹 Nitpick comments (6)
NewType.Generator/AliasGenerator.cs (1)
15-24: Consider using a more descriptive diagnostic category.The diagnostic category is set to
"Unknown"(line 20). Using a more specific category like"NewType"or"CodeGeneration"would improve discoverability when filtering diagnostics.Proposed fix
private static readonly DiagnosticDescriptor MissingIsValidMethodDiagnostic = new DiagnosticDescriptor( id: "NEWTYPE001", title: "Missing validation method", messageFormat: $"Type '{{0}}' uses constraints but does not define a compatible validation method. Expected signature: 'bool {AliasModel.ConstraintValidationMethodSymbol}({{1}})'.", - category: "Unknown", + category: "NewType", DiagnosticSeverity.Error, isEnabledByDefault: true, description: "A constraint-enabled wrapped type must define a validation method." );NewType.Generator/AliasModel.cs (2)
47-49: Naming convention:validValidationMethodshould beValidValidationMethod.The property uses camelCase (
validValidationMethod) instead of PascalCase, which is the standard C# convention for public properties/fields.Proposed fix
bool IncludeConstraints, bool DebugOnlyConstraints, - bool validValidationMethod, + bool ValidValidationMethod,Note: This requires updating all usages in
AliasCodeGenerator.csandAliasGenerator.csas well.
19-21:Locationstores a Roslyn symbol reference.The doc comment (line 9) states "no Roslyn symbols," but
Locationis a Roslyn type. While this is necessary for diagnostic reporting and is acceptable, it's worth noting this deviation from the stated design goal. Consider updating the comment to reflect this exception.NewType.Tests/ConstraintValidationTests.cs (3)
1-3: Consider adding explicitusing System;directive.
InvalidOperationException(used in assertions at lines 31, 37, 43, 60) is in theSystemnamespace. This will compile with implicit usings enabled, but adding an explicitusing System;improves clarity and prevents issues if implicit usings are disabled.using System.Numerics; +using System; using newtype.tests; using Xunit;
7-26: Use discard pattern for "doesn't throw" tests to suppress unused variable warnings.The variables
dirin these tests are intentionally unused—their purpose is to verify construction doesn't throw. Using the discard pattern_ = ...makes this intent explicit and avoids compiler warnings.♻️ Proposed fix
[Fact] public void Direction_Valid() { //doesn't throw - var dir = new Direction(new Vector2(0, 1)); + _ = new Direction(new Vector2(0, 1)); } [Fact] public void Direction_ValidImplicit() { //doesn't throw - Direction dir = new Vector2(0, 1); + Direction _ = new Vector2(0, 1); } [Fact] public void Direction_Valid_Forwarded() { //doesn't throw - var dir = new Direction(0, 1); + _ = new Direction(0, 1); }
46-52: Discard unuseddir3variable or add an assertion.Similar to the earlier tests,
dir3is unused. Either discard it to clarify intent or add an assertion to verify the result.♻️ Proposed fix
[Fact] public void Direction_ForwardedOperation_ValidResult() { var dir1 = new Direction(new Vector2(0, 1f)); var dir2 = new Direction(new Vector2(0, 1f)); - var dir3 = dir1 * dir2; + _ = dir1 * dir2; }
| if (_model.IncludeConstraints) | ||
| { | ||
| _sb.AppendLine($"{memberIndent}public {_model.TypeName}({_model.AliasedTypeFullName} value)"); | ||
| _sb.Append(memberIndent).Append('{').AppendLine(); | ||
|
|
||
| AppendConstraintChecker(SingleIndent, "value"); | ||
| _sb.Append(SingleIndent).Append(SingleIndent).Append(SingleIndent).AppendLine("_value = value;"); | ||
|
|
||
| _sb.Append(memberIndent).Append('}').AppendLine(); | ||
| } | ||
| else | ||
| { | ||
| _sb.AppendLine($"{memberIndent}public {_model.TypeName}({_model.AliasedTypeFullName} value) => _value = value;"); | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation in generated constructor body.
The _value = value; assignment (line 156) uses three SingleIndent levels hardcoded, but memberIndent already accounts for namespace depth. When a namespace is present, memberIndent is 8 spaces, and the body should be indented one level deeper (12 spaces). However, SingleIndent * 3 always produces 12 spaces regardless of namespace presence.
For types without a namespace, the member indent is 4 spaces, so body lines should be 8 spaces, but this code emits 12.
Proposed fix
if (_model.IncludeConstraints)
{
_sb.AppendLine($"{memberIndent}public {_model.TypeName}({_model.AliasedTypeFullName} value)");
- _sb.Append(memberIndent).Append('{').AppendLine();
-
- AppendConstraintChecker(SingleIndent, "value");
- _sb.Append(SingleIndent).Append(SingleIndent).Append(SingleIndent).AppendLine("_value = value;");
-
- _sb.Append(memberIndent).Append('}').AppendLine();
+ _sb.AppendLine($"{memberIndent}{{");
+
+ var bodyIndent = memberIndent + SingleIndent;
+ AppendConstraintChecker(bodyIndent, "value");
+ _sb.AppendLine($"{bodyIndent}_value = value;");
+
+ _sb.AppendLine($"{memberIndent}}}");
}🤖 Prompt for AI Agents
In `@NewType.Generator/AliasCodeGenerator.cs` around lines 150 - 163, The
constructor body indentation is inconsistent: in AliasCodeGenerator.cs when
_model.IncludeConstraints is true you append the assignment using three
SingleIndent units which ignores memberIndent and mis-indents types without
namespaces; change the code that writes the body (the AppendConstraintChecker
call and the line writing "_value = value;") to build indentation relative to
memberIndent (e.g., append memberIndent then one SingleIndent before body lines)
instead of using SingleIndent repeated three times so the body is always
indented exactly one level deeper than memberIndent; keep
AppendConstraintChecker("SingleIndent", "value") usage but ensure the subsequent
"_value = value;" write uses memberIndent + SingleIndent.
| if (_model.IncludeConstraints) | ||
| { | ||
| const string valueName = "newValue"; | ||
| _sb.AppendLine($"{indent}public {_model.TypeName}({parameters})"); | ||
| _sb.Append(indent).Append('{').AppendLine(); | ||
| _sb.Append(indent).Append(SingleIndent).AppendLine($"var {valueName} = new {_model.AliasedTypeFullName}({arguments});"); | ||
|
|
||
| AppendConstraintChecker(SingleIndent, valueName); | ||
| _sb.Append(SingleIndent).Append(SingleIndent).Append(SingleIndent).AppendLine($"_value = {valueName};"); | ||
|
|
||
| _sb.Append(indent).Append('}').AppendLine(); | ||
| } | ||
| else | ||
| { | ||
| _sb.AppendLine($"{indent}public {_model.TypeName}({parameters}) => _value = new {_model.AliasedTypeFullName}({arguments});"); | ||
| } |
There was a problem hiding this comment.
Same indentation issue in forwarded constructor.
The forwarded constructor has the same hardcoded indentation problem. SingleIndent * 3 doesn't adapt to the actual member indent level.
Proposed fix
if (_model.IncludeConstraints)
{
const string valueName = "newValue";
_sb.AppendLine($"{indent}public {_model.TypeName}({parameters})");
- _sb.Append(indent).Append('{').AppendLine();
- _sb.Append(indent).Append(SingleIndent).AppendLine($"var {valueName} = new {_model.AliasedTypeFullName}({arguments});");
-
- AppendConstraintChecker(SingleIndent, valueName);
- _sb.Append(SingleIndent).Append(SingleIndent).Append(SingleIndent).AppendLine($"_value = {valueName};");
-
- _sb.Append(indent).Append('}').AppendLine();
+ _sb.AppendLine($"{indent}{{");
+ var bodyIndent = indent + SingleIndent;
+ _sb.AppendLine($"{bodyIndent}var {valueName} = new {_model.AliasedTypeFullName}({arguments});");
+
+ AppendConstraintChecker(bodyIndent, valueName);
+ _sb.AppendLine($"{bodyIndent}_value = {valueName};");
+
+ _sb.AppendLine($"{indent}}}");
}🤖 Prompt for AI Agents
In `@NewType.Generator/AliasCodeGenerator.cs` around lines 690 - 705, The
forwarded constructor uses hardcoded SingleIndent repetitions which don't
respect the current member indentation; change both branches to build
indentation from the existing indent variable plus SingleIndent(s) instead of
appending SingleIndent three times. Specifically, in the block that calls
AppendConstraintChecker and sets _value, replace the
_sb.Append(SingleIndent).Append(SingleIndent).Append(SingleIndent)... call with
something that starts from indent and then appends the needed SingleIndent(s)
(for example _sb.Append(indent).Append(SingleIndent).Append(SingleIndent) before
writing the "_value" assignment), and in the else branch replace the single-line
AppendLine with a properly indented multi-line emission that uses indent +
SingleIndent(s) so the forwarded constructor matches the same member-indent
level; reference AppendConstraintChecker, _sb, indent, SingleIndent,
_model.TypeName and _model.AliasedTypeFullName to locate the lines to change.
| private void AppendConstraintChecker(string indent, string valueName) | ||
| { | ||
| if (!_model.validValidationMethod) return; | ||
|
|
||
| if (_model.DebugOnlyConstraints) | ||
| _sb.AppendLine("#if DEBUG"); | ||
|
|
||
| _sb.Append(indent).Append(indent).Append(indent) | ||
| .AppendLine($"if (!IsValid({valueName}))"); | ||
| _sb.Append(indent).Append(indent).Append(indent).Append(indent) | ||
| .AppendLine($"throw new InvalidOperationException($\"Failed validation check when trying to create '{_model.TypeName}' with '{_model.AliasedTypeMinimalName}' value: {{{valueName}}}\");"); // we heard you like interpolation | ||
|
|
||
|
|
||
| if (_model.DebugOnlyConstraints) | ||
| _sb.AppendLine("#endif"); | ||
| } |
There was a problem hiding this comment.
AppendConstraintChecker uses incorrect indentation approach.
The method receives an indent parameter but then uses indent * 3 hardcoded (via three .Append(indent) calls), which doesn't align with the actual nesting level. The caller should pass the correct body indent, and this method should use it directly.
Proposed fix
- private void AppendConstraintChecker(string indent, string valueName)
+ private void AppendConstraintChecker(string bodyIndent, string valueName)
{
if (!_model.validValidationMethod) return;
if (_model.DebugOnlyConstraints)
_sb.AppendLine("#if DEBUG");
- _sb.Append(indent).Append(indent).Append(indent)
- .AppendLine($"if (!IsValid({valueName}))");
- _sb.Append(indent).Append(indent).Append(indent).Append(indent)
- .AppendLine($"throw new InvalidOperationException($\"Failed validation check when trying to create '{_model.TypeName}' with '{_model.AliasedTypeMinimalName}' value: {{{valueName}}}\");");
+ _sb.AppendLine($"{bodyIndent}if (!IsValid({valueName}))");
+ _sb.AppendLine($"{bodyIndent}{SingleIndent}throw new InvalidOperationException($\"Failed validation check when trying to create '{_model.TypeName}' with '{_model.AliasedTypeMinimalName}' value: {{{valueName}}}\");");
if (_model.DebugOnlyConstraints)
_sb.AppendLine("#endif");
}🤖 Prompt for AI Agents
In `@NewType.Generator/AliasCodeGenerator.cs` around lines 728 - 743,
AppendConstraintChecker currently hardcodes triple indentation by calling
.Append(indent) three times; instead use the passed-in indent as the correct
"body" indent: emit the if line with a single .Append(indent).AppendLine($"if
(!IsValid({valueName}))"), and emit the throw line with the body indent plus one
more indent (e.g. .Append(indent).Append(indent).AppendLine(...)) so nesting is
correct; keep the `#if` DEBUG guard logic and the same exception text referencing
_model.TypeName and _model.AliasedTypeMinimalName and write to _sb as before.
1 participant
I've noticed in my own projects that often when I create a "type alias", it is a constrained version of the wrapped type, e.g:
Usually this involves writing repeat boilerplate involving
Debug.Assertwithin the type constructors/setters, as a simple verification layer during design and testing. In relatively high-performance code, I don't want to be checking for this excessivelyThis PR adds a "valid" constraint to newtype, injecting a user-defined validation method into the constructor, helping to avoid invariants. Optionally enabled, optionally enabled in release.
Example:
Generated code:
Includes an analyzer warning if the constraint is enabled but a method is not provided
Feedback on API and naming appreciated.
(I was actually originally going the route of making my own source generator for this, but as I tried to implement it I realized the design was growing closer and closer to what newtype already was, so I decided it makes more sense to add on to it instead xd)
Summary by CodeRabbit
New Features
Tests
Diagnostics / Docs