[Repo Assist] Refactor: move resolveCaseIdent to GeneratorHelpers — closes #244#245
Conversation
…ication DUCasesGenerator and LensesGenerator both had the same logic for resolving a DU case identifier (qualifying with parent type name when RequireQualifiedAccess is present). Move the shared implementation to GeneratorHelpers.resolveCaseIdent and update both generators to call it. Closes #244 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors Myriad.Plugins generators to centralize DU case identifier qualification logic (handling RequireQualifiedAccess) into a shared helper, eliminating duplicated implementations across generators (closes #244).
Changes:
- Added
GeneratorHelpers.resolveCaseIdentto build a fully-qualifiedSynLongIdentfor DU cases when qualified access is required. - Updated
DUCasesGeneratorto use the shared helper and removed its local implementation. - Updated
LensesGeneratorto replace inline qualification logic with the shared helper.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Myriad.Plugins/GeneratorHelpers.fs | Introduces shared resolveCaseIdent helper used by multiple generators. |
| src/Myriad.Plugins/DUCasesGenerator.fs | Removes duplicated helper and routes DU case name resolution through GeneratorHelpers. |
| src/Myriad.Plugins/LensesGenerator.fs | Replaces inline DU case qualification logic with GeneratorHelpers.resolveCaseIdent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot Can you resolve the merge conflicts please. |
…CaseMatchClause Co-authored-by: 7sharp9 <588746+7sharp9@users.noreply.github.com>
|
🤖 This is an automated response from Repo Assist. This PR has been superseded and can be safely closed. The same fix — moving
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Resolve merge conflicts with master; rename `indent` → `ident` in `createCaseMatchClause`
🤖 This is an automated pull request from Repo Assist.
Summary
Moves the
resolveCaseIdenthelper (which builds a fully-qualified DU caseSynLongIdentwhenRequireQualifiedAccessis in effect) fromDUCasesGeneratorintoGeneratorHelpers, and replaces the equivalent inline logic inLensesGeneratorwith a call to the shared helper.Closes #244
Root Cause
DUCasesGenerator.fsdefined a named helperresolveCaseIdent, whileLensesGenerator.fsre-implemented the exact same logic inline asmatchCaseIdentParts. Any future change to qualified-access case name resolution had to be applied in two places, risking silent divergence.Fix
GeneratorHelpers.fs— addedresolveCaseIdent (requiresQualifiedAccess) (parent) (id)returning aSynLongIdentDUCasesGenerator.fs— removed the localresolveCaseIdentdefinition; updated call sites toGeneratorHelpers.resolveCaseIdentLensesGenerator.fs— replaced the inlinematchCaseIdentPartsblock with a single call toGeneratorHelpers.resolveCaseIdentTrade-offs
None — pure refactor with no behaviour change. Both generators produce identical output before and after.
Test Status
✅
dotnet build src/Myriad.Plugins/Myriad.Plugins.fsproj— succeeded, 0 warnings, 0 errors✅ Integration tests (
Myriad.IntegrationPluginTests) — all passed, 0 failed, 0 errored