feat(v1.7): conditional property assignment (Condition/SkipWhen) - #137
Merged
Conversation
…Into, composition
There was a problem hiding this comment.
Pull request overview
Implements v1.7 Feature 2 “conditional property assignment” by adding Condition / SkipWhen to [ForgeProperty], threading the config through the generator, emitting guarded assignments for both Forge and ForgeInto, and introducing FM0060–FM0064 diagnostics with xUnit coverage.
Changes:
- Add
Condition/SkipWhentoForgePropertyAttributeand parse/merge mappings (including[IncludeBaseForge]inheritance). - Emit conditional guards in
Forge(post-construction blocks) andForgeInto(captured statement blocks + replay insideif). - Add FM0060–FM0064 descriptors + analyzer release tracking, and introduce
ConditionalAssignmentGeneratorTests.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ForgeMap.Tests/ConditionalAssignmentGeneratorTests.cs | Adds coverage for guarded emission, diagnostics, and composition scenarios |
| src/ForgeMap.Abstractions/ForgePropertyAttribute.cs | Public API surface: adds Condition and SkipWhen named properties |
| src/ForgeMap.Generator/ForgerConfig.cs | Extends ResolvedMethodConfig with conditional mappings |
| src/ForgeMap.Generator/ForgeCodeEmitter.Configuration.cs | Parses/merges Condition/SkipWhen mappings and threads into config |
| src/ForgeMap.Generator/ForgeCodeEmitter.Conditional.cs | Adds predicate resolution + guard-building helpers for conditional assignment |
| src/ForgeMap.Generator/ForgeCodeEmitter.PropertyAssignment.cs | Triggers conditional validation during assignment generation |
| src/ForgeMap.Generator/ForgeCodeEmitter.ForgeMethod.cs | Moves conditional assignments out of initializers into guarded post-construction blocks |
| src/ForgeMap.Generator/ForgeCodeEmitter.ForgeIntoMethod.cs | Wraps per-property emission in conditional guards; wires per-property ConvertWith |
| src/ForgeMap.Generator/DiagnosticDescriptors.cs | Adds FM0060–FM0064 descriptors |
| src/ForgeMap.Generator/AnalyzerReleases.Unshipped.md | Registers new diagnostic IDs for RS2000 tracking |
| docs/SPEC-v1.7-projection-and-conditional.md | Marks Feature 2 as “Implemented” |
| docs/superpowers/plans/2026-04-21-v1.7-conditional-assignment.md | Adds implementation plan document for the feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- ForgeIntoMethod: when SelectProperty conflicts with [ForgeFrom]/[ForgeWith], emit FM0072 only and skip the FM0075 'not supported' notice (avoids dual diagnostics for the same conflict). - Configuration: gate base Condition/SkipWhen merge on IsExplicitlyConfigured so cross-kind overrides (derived [Ignore]/[ForgeFrom]/etc.) suppress the inherited conditional, preventing spurious FM0063 and unwanted guards. - Conditional: extract ResolveSourcePropertyPath that honors PropertyMatching (case-insensitive convention matching) and uses safe 'is' cast for source type instead of unconditional (INamedTypeSymbol) cast.
- Configuration: track Condition/SkipWhen as explicit derived config so an inherited mapping cannot overwrite a derived conditional, and report FM0021 when an inherited Condition/SkipWhen is masked by a derived configuration. Block Condition/SkipWhen leakage across multiple [IncludeBaseForge] bases by consulting preExistingConfigured (now also includes prior conditional keys). - Forge/ForgeInto guard emission: route the predicate-argument source path through ResolveSourcePropertyPath so case-insensitive PropertyMatching and convention-based matches generate the correct source.* expression instead of falling back to destProp.Name.
…signments When GeneratePropertyAssignment returns null (the property's emission was queued into skipNullAssignments or postConstructionCollections instead of returned as a single expression), the resolved Condition/SkipWhen was silently dropped, leaving the assignment unguarded. Snapshot queue counts before each call and use a new WrapQueuedEntriesWithConditionalGuard helper to retroactively wrap any newly-queued entries inside the predicate guard. Applies to all three Forge writers (ctor+initializer, AfterForge, plain object-init) so SkipNull / post-construction collection / projection paths all honor the user's conditional configuration.
Address Copilot review feedback on PR #137: - Remove obsolete 'upcoming task' header comment in Conditional.cs - Defer FM0064 (ConditionalAssignmentApplied) reporting from predicate resolution to the actual guard block emission, so the diagnostic reflects emitted output rather than mere configuration validity. - Wire ReportConditionalAssignmentApplied into all Forge emit sites (ctor-bound, after-forge, plain) and ForgeInto's FlushConditionalGuard.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review on PR #137: - Fix stale comment in ForgeIntoMethod.cs (FM0064 deferred from resolution) - Fix XML doc indentation on BuildConditionalGuardExpression - Extract ResolveConditionalAndPredicateArg helper to centralize the conditional-resolution + predicate-arg derivation pattern that was duplicated across the three Forge writers (ctor+initializer, obj-init+after-forge, simple object init).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per-property [ForgeProperty(ConvertWith=...)] in ForgeInto was checked after [ForgeFrom]/[ForgeWith], silently dropping ConvertWith when a resolver/forging method also matched. Forge's GeneratePropertyAssignment checks ConvertWith first. Move ForgeInto's ConvertWith block to run before ForgeFrom and ForgeWith so behavior matches across the two generation paths.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Order was: (1) convert skipNullAssignments into pre-wrapped post-construction blocks and append them, (2) iterate from postCtorSnapshot wrapping all entries — which re-wrapped the just-appended pre-wrapped blocks, producing nested duplicate 'if (guard)' statements. Wrap pre-existing entries first, then append the already-wrapped skipNull-derived blocks.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…to SelectProperty diagnostics
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…orgeInto destinations
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 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
ConditionandSkipWhennamed properties on[ForgeProperty]that wrap per-property assignments in predicate guardsForge(post-constructionif-guard) andForgeInto(existing-target) paths; composes with v1.6ConvertWithand v1.7 Feature 1SelectPropertyNotable
ForgeCodeEmitter.ForgeIntoMethod.cs: per-propertyConvertWithwas previously silently ignored onForgeInto. Now wired through and required forCondition + ConvertWithcomposition.Test plan
ConditionalAssignmentGeneratorTests.cscover happy paths, all 5 diagnostics, FM0072-suppresses-FM0063, ConvertWith composition, Ignore precedence, and ReverseForge non-propagation