feat(v1.7): [ExtractProperty] and [WrapProperty] for entity↔primitive mapping (#127) - #138
Merged
Conversation
…perty/WrapProperty
…ractProperty/WrapProperty
…thod and skip in reverse loop
…, strategies, and composition
There was a problem hiding this comment.
Pull request overview
Adds ForgeMap v1.7 “Feature 3” support for single-member entity↔primitive mappings via new method-level attributes, with generator emission + diagnostics + tests.
Changes:
- Introduces
[ExtractProperty]and[WrapProperty]attributes inForgeMap.Abstractions. - Extends the generator to detect these attributes, emit method bodies, and register new diagnostics (FM0065–FM0071, FM0074).
- Adds a comprehensive xUnit test suite and updates the v1.7 spec status table.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ForgeMap.Tests/ExtractWrapPropertyGeneratorTests.cs | Adds coverage for extract/wrap emission, coercions, diagnostics, and composition behaviors. |
| src/ForgeMap.Generator/ForgeCodeEmitter.cs | Wires attribute symbol lookup, dispatch to extract/wrap emitter, and skips reverse generation for these methods. |
| src/ForgeMap.Generator/ForgeCodeEmitter.ExtractWrap.cs | Implements Extract/Wrap validation, emission logic, constructor selection, required-member checks, and coercions. |
| src/ForgeMap.Generator/ForgeCodeEmitter.AttributeDetection.cs | Adds attribute detection + argument extraction helpers. |
| src/ForgeMap.Generator/DiagnosticDescriptors.cs | Registers the new FM0065–FM0071 + FM0074 descriptors. |
| src/ForgeMap.Generator/AnalyzerReleases.Unshipped.md | Adds the new diagnostic IDs for RS2000 tracking. |
| src/ForgeMap.Abstractions/WrapPropertyAttribute.cs | Adds the public WrapPropertyAttribute. |
| src/ForgeMap.Abstractions/ExtractPropertyAttribute.cs | Adds the public ExtractPropertyAttribute. |
| docs/superpowers/plans/2026-04-21-v1.7-extract-wrap-property.md | Adds the implementation plan document for Feature 3. |
| docs/SPEC-v1.7-projection-and-conditional.md | Marks Features 1 and 3 as Implemented in the status table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…TimeOffset wrap coercion
- Extract: require getter itself to be public (rejects `public T P { private get; ... }`)
- Wrap: require setter itself to be public (rejects `public T P { get; private set; }`)
- Wrap: implement DateTime → DateTimeOffset coercion previously only documented
…xtract semantics new DateTimeOffset(dt) interprets DateTimeKind.Unspecified as local time, breaking the round-trip with extract's .UtcDateTime normalization. SpecifyKind(dt, Utc) makes the conversion offset-invariant and symmetric.
…+ drop dead code - Extract the DateTime→DateTimeOffset logic into TryGenerateDateTimeToDateTimeOffsetCoercion mirror of TryGenerateDateTimeOffsetToDateTimeCoercion handling all four nullable shapes - Remove unused namedReadable inventory in GenerateWrapPropertyBody
…ed-member lookup - Extract source property and wrap settable destination property both used GetMembers(name) which only returned directly declared members, so a derived type was rejected - EnumerateUnsatisfiedRequiredMembers now walks BaseType chain too (with new-shadow dedup) so inherited required members are correctly detected
…, test naming Addresses Copilot PR #138 review: - TryCoerceForExtract/Wrap: emit .GetValueOrDefault() for Nullable<T> -> T (CanAssign allows the conversion, but raw access wouldn't compile). - GenerateExtractPropertyBody: unwrap Nullable<T> source for member lookup, use source!.Value.Prop after the null guard. - EnumerateUnsatisfiedRequiredMembers: also yield IFieldSymbol since C# 11 required applies to fields too. - Rename Extract_VoidReturn_EmitsFM0070 / Wrap_VoidReturn_EmitsFM0070 to *_NoBodyEmitted_DefersToCS8795 — the assertions verify the spec-compliant CS8795 behavior, not an FM0070 emission.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…biguity - TryCoerceForExtract / TryCoerceForWrap now route string<->enum through the shared IsStringToEnumPair / IsEnumToStringPair / GenerateStringToEnumParseExpression helpers, so _config.StringToEnum (Parse/TryParse/None/StrictParse) and Nullable<Enum> are honored — matching PropertyAssignment/Projection behavior. Enum->string lifts via null-conditional ToString for nullable sources. - FindWrapConstructor returns AmbiguityReported so GenerateWrapPropertyBody short-circuits after FM0013, preventing duplicate FM0068/FM0071 diagnostics on the same root cause. - WrapPropertyAttribute XML doc clarifies the initializer strategy also requires a public parameterless constructor.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
C# only treats `required` members as satisfied when the constructor is annotated with [SetsRequiredMembers] — matching a parameter name does not satisfy the check (CS9035 still fires). The previous logic suppressed required members whose names matched ctor parameters and unconditionally suppressed the wrapped member, which let WrapProperty pick the ctor strategy and emit code that wouldn't compile. Now: when a ctor is supplied AND it's not [SetsRequiredMembers], report ALL required members (including the wrapped one). The initializer path still legitimately satisfies the wrapped member via the object initializer. Adds regression test asserting FM0071 fires for a ctor whose parameter matches a required member but lacks [SetsRequiredMembers].
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Abstract types cannot be instantiated via 'new T(...)' or 'new T { ... }',
so neither wrap strategy can produce compilable code. Previously the
generator would proceed and emit 'new AbstractType(...)' which fails to
compile downstream. Add an early check that surfaces FM0068 instead.
Adds regression test Wrap_AbstractDestination_EmitsFM0068_NotUncompilableCode.
…params optional Local Copilot CLI Iter 4 findings E and F: - F: When [ForgeConstructor(T1, ...)] explicitly selects a parameterized ctor, do not silently switch to the initializer strategy just because an init/set property of the same name exists. The empty form [ForgeConstructor()] still defers to init when viable (matches Iter 1 contract). - E: For an explicitly-selected ctor, all non-matched parameters must be optional/defaulted; otherwise emitting new T(named: source) would skip required parameters and fail to compile. Surface FM0068 instead.
…alls back to init Local Copilot CLI Iter 5 finding H: when [ForgeConstructor(T1, ...)] picked a ctor whose required-member set was unsatisfied, useInit was set via the 'else if (preferInit) useInit = initStrategyViable' branch, silently switching to the initializer strategy and ignoring the user's explicit ctor opt-in. Pin useInit=false unconditionally for explicit parameterized selections so FM0071/FM0068 surfaces instead.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Local Copilot CLI Iter 1 finding A: initStrategyViable was decided purely on existence of a settable named property + parameterless ctor. If the property's type cannot accept the wrap source but a same-named ctor parameter can, the generator silently committed to init and emitted FM0069 instead of falling back to the compatible ctor. Include TryCoerceForWrap(source -> property) in initStrategyViable so the ctor path gets a fair chance, and extend the FM0069/FM0068 fallback diagnostic to cover init-property type mismatch.
… path
Local Copilot CLI Iter 1 finding A: initStrategyViable computed
unsatisfiedRequiredOnInit without considering whether the public parameterless
constructor itself was annotated [SetsRequiredMembers]. A type whose
parameterless ctor has [SetsRequiredMembers] has already initialized every
required member, so 'new T { Scope = source }' is allowed even if other
required members exist. Mirror the ctor path's HasSetsRequiredMembers trust on
the init path.
…ble return Local Copilot CLI Iter 2 finding C: TryCoerceForExtract emits .GetValueOrDefault() to keep Nullable<T> source -> T return code compiling, but null silently becomes default(T). Mirror PropertyAssignment's behavior by reporting FM0007 (NullableToNonNullableMapping) at the call site so users get a warning instead of a silent data loss.
…M0007 destination Local Copilot CLI Iter 3 findings F + G: - F: FM0074 (value-type return audit under ReturnNull) was firing for any IsValueType return, including Nullable<T>. Returning int? holds null fine — no 'null collapses to default' problem. Skip the warning when the return type is Nullable<T>. Same fix applied to wrap path. - G: FM0007 used method.ContainingType.Name as the destination 'type' (e.g. 'M.ExtractValue'), which is misleading because the destination is the return value, not a property on the containing class. Use the return type's name instead so the message reads '<src>.<prop>' mapped to '<retType>.<method>'.
…eConstructor()] through init precedence
Local Copilot CLI Iter 4:
- I (FM0068 false positive on set-only init properties): the wrap init lookup
used GetMappableProperties, which filters on getter presence. `new T { Prop = src }`
compiles for set-only properties, so the lookup is now an inheritance walk that
accepts public set/init properties regardless of getter.
- J (empty [ForgeConstructor()] always emits FM0068): the empty-types branch
unconditionally tried to find a matching ctor parameter and emitted FM0068 when
none existed. The parameterless ctor selection means "use init"; verify the
parameterless public ctor exists (FM0047 otherwise) and return without a ctor
pick so the caller's init/FM0071/FM0069/FM0068 precedence emits the correct
diagnostic.
Tests:
- Wrap_SetOnlyProperty_InitStrategySucceeds
- Wrap_ExplicitForgeConstructorEmpty_UnsatisfiedRequired_EmitsFM0071_NotFM0068
- Wrap_ExplicitForgeConstructorEmpty_TypeIncompatibleInit_EmitsFM0069_NotFM0068
All 392 tests pass on net8/net9/net10.
…structor()] [ForgeConstructor()] (empty types) explicitly opts into the parameterless ctor / init strategy. The FM0069 fallback was scanning ALL public ctors for a same-named incompatible parameter, so an unrelated parameterized ctor could trigger a bogus FM0069 instead of the real init-path failure. Detect the empty-types case via hasExplicitEmptyForgeConstructor and skip the all-ctors scan when set; the init-property mismatch path still runs. Test: Wrap_ExplicitForgeConstructorEmpty_UnrelatedCtorWithSameNameMismatch_DoesNotEmitFM0069 393/393 tests pass on net8/net9/net10.
… scan The FM0069 fallback scanned every public ctor for a same-named-but-incompatible parameter, including ctors with extra REQUIRED parameters that could never serve as single-arg wrap candidates. Citing such a ctor produced a misleading "type incompatible" message when the real issue was that no viable wrap strategy existed. Filter out ctors with additional non-defaulted/non-optional parameters before emitting FM0069, matching the viability rule used by FindWrapConstructor. Test: Wrap_NonViableCtorWithSameNameMismatch_DoesNotEmitFM0069_EmitsFM0068 394/394 tests pass on net8/net9/net10.
…ence extract FM0007 was only emitted for the Nullable<T>→T value-type case in [ExtractProperty]. The analogous reference-type case (string? property → string return) compiled but generated CS8603 in the user's generated code with no ForgeMap diagnostic explaining it. PropertyAssignment already warns on this shape — extract now matches. Test: Extract_NullableReferencePropertyToNonNullableReturn_EmitsFM0007 395/395 tests pass on net8/net9/net10.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Align WrapProperty's destination instantiation guard with the rest of
the generator: an interface return type cannot be instantiated via
'new I(...)' or 'new I { ... }', so neither wrap strategy can produce
compilable code. Surface as FM0068 (PropertyNotFound on the destination)
rather than emitting an uncompilable 'new IInterface(...)'.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ty names
[ExtractProperty(null)] / [WrapProperty("")] silently bailed out, leaving
the partial method unimplemented and surfacing only as CS8795 (which is
confusing — the user has no signal that the attribute argument is the
real cause). Emit FM0066/FM0068 with a <null>/<empty> placeholder name
so the configuration error is actionable.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 tasks
superyyrrzz
added a commit
that referenced
this pull request
Apr 23, 2026
* chore: prepare v1.7.0 release Bump VersionPrefix to 1.7.0, move FM0055-FM0075 from AnalyzerReleases.Unshipped.md to a new Release 1.7.0 section in AnalyzerReleases.Shipped.md, and add a v1.7.0 changelog entry covering SelectProperty (#136), conditional assignment (#137), and [ExtractProperty]/[WrapProperty] (#138). * docs: clarify [WrapProperty] strategies in v1.7.0 changelog Reflect that the generator selects between object-initializer and constructor-based wrap strategies based on destination member shape and ConstructorPreference, not solely the initializer form.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements v1.7 Feature 3 — two new method-level attributes that let users declare single-property entity↔primitive mappings as partial methods whose bodies the generator emits:
[ExtractProperty(name)]— entity → primitive (e.g.,ClientScope → string)[WrapProperty(name)]— primitive → entity (e.g.,string → ClientScope), with strategy selection between object initializer and constructor,[SetsRequiredMembers]trust, and a wrap-specific FM0013 tie-breakComposes with collection auto-wire so
List<Entity> → List<Primitive>flows through automatically.Changes
ExtractPropertyAttribute,WrapPropertyAttributeinForgeMap.AbstractionsForgeCodeEmitter.ExtractWrap.cspartial; dispatch hook inGenerateMethodbefore[ConvertWith]; reverse-loop skipDateTimeOffset → DateTime→string ↔ enumPreferParameterlessoverride,[SetsRequiredMembers]trust, and compositionTest plan
dotnet build -c Release— 0 errors, 0 warningsdotnet test -c Release— 377/377 passing on net8.0/net9.0/net10.0 (354 existing + 23 new)DiagnosticDescriptors.csandAnalyzerReleases.Unshipped.mdKnown minor gap
FM0070 fires for wrong-arity (verified) but not for void-return Extract/Wrap partials, because the partial-method enumeration at
ForgeCodeEmitter.cs:240filters void partials before they reach the FM0070 site. Users still get a CS8795 (partial method not implemented) from the C# compiler, so the void case is rejected — just with a different error. Worth a follow-up to relax the filter; not blocking.Out of scope (per spec)
[ExtractProperty("Owner.Name")])ForgeInto/[UseExistingValue](always create fresh values)[BeforeForge]/[AfterForge]on extract/wrap methods (existing FM0018-style diagnostic surfaces)Closes #127