Skip to content

Refactor: Consolidate duplicate resolveCaseIdent logic into GeneratorHelpers#260

Open
Copilot wants to merge 3 commits into
masterfrom
copilot/refactor-duplicate-resolvecaseident-logic
Open

Refactor: Consolidate duplicate resolveCaseIdent logic into GeneratorHelpers#260
Copilot wants to merge 3 commits into
masterfrom
copilot/refactor-duplicate-resolvecaseident-logic

Conversation

Copilot AI commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

DUCasesGenerator and LensesGenerator both independently implemented the same qualified DU case name resolution logic — any divergence between the two would cause inconsistent behaviour across generators.

Changes

  • GeneratorHelpers.fs — Added shared resolveCaseIdent helper that resolves a DU case identifier, optionally fully qualifying it with the parent type name when RequireQualifiedAccess is present
  • DUCasesGenerator.fs — Removed local resolveCaseIdent; updated call sites to GeneratorHelpers.resolveCaseIdent
  • LensesGenerator.fs — Replaced inline matchCaseIdentParts block with a call to GeneratorHelpers.resolveCaseIdent
// GeneratorHelpers.fs — single source of truth
let resolveCaseIdent (requiresQualifiedAccess: bool) (parent: LongIdent) (id: Ident) : SynLongIdent =
    let parts =
        if requiresQualifiedAccess then
            (parent |> List.map (fun i -> i.idText)) @ [id.idText]
        else
            [id.idText]
    SynLongIdent.Create parts
Original prompt

This section details on the original issue you should resolve

<issue_title>Duplicate Code: resolveCaseIdent Logic Duplicated Across DUCasesGenerator and LensesGenerator</issue_title>
<issue_description>Analysis of the Myriad plugin generators reveals that the logic for resolving a DU case identifier (optionally fully qualifying it with the parent type name when RequireQualifiedAccess is present) is duplicated between DUCasesGenerator.fs and LensesGenerator.fs.

Assignee: @copilot

Summary

DUCasesGenerator.fs defines a named helper function resolveCaseIdent that computes the full case name. LensesGenerator.fs re-implements the exact same logic inline as matchCaseIdentParts. These should be unified in GeneratorHelpers so both generators share a single implementation.

Duplication Details

Pattern: Qualified DU case name resolution

  • Severity: Medium
  • Occurrences: 2
  • Locations:
    • src/Myriad.Plugins/DUCasesGenerator.fs (lines 18–24) — named function resolveCaseIdent
    • src/Myriad.Plugins/LensesGenerator.fs (lines 68–72) — inline matchCaseIdentParts

Code in DUCasesGenerator.fs (lines 18–24):

let resolveCaseIdent (requiresQualifiedAccess: bool) (parent: LongIdent) (id: Fantomas.FCS.Syntax.Ident) : SynLongIdent =
    let parts =
        if requiresQualifiedAccess then
            (parent |> List.map (fun i -> i.idText)) @ [id.idText]
        else
            [id.idText]
    SynLongIdent.Create parts

Equivalent inline logic in LensesGenerator.fs (lines 68–72):

let matchCaseIdentParts =
    if requiresQualifiedAccess then
        (parent |> List.map (fun i -> i.idText)) @ [id.idText]
    else
        [id.idText]
// then used as: SynLongIdent.Create matchCaseIdentParts

Impact Analysis

  • Maintainability: Any change to qualified-access case resolution (e.g. supporting nested modules) must be applied in two places, risking divergence.
  • Bug Risk: A fix for one generator may be missed in the other, causing inconsistent behaviour between lenses and dus generators.
  • Code Bloat: Minor, but obscures the shared contract between the two generators.

Refactoring Recommendations

  1. Move resolveCaseIdent to GeneratorHelpers
    • Add resolveCaseIdent (or rename to resolveDuCaseIdent) to src/Myriad.Plugins/GeneratorHelpers.fs
    • Remove the local definition from DUCasesGenerator.fs
    • Replace the inline matchCaseIdentParts block in LensesGenerator.fs with a call to the shared helper
    • Estimated effort: ~30 minutes
    • Benefits: Single source of truth for qualified-access case resolution

Implementation Checklist

  • Review duplication findings
  • Move resolveCaseIdent to GeneratorHelpers.fs
  • Update DUCasesGenerator.fs to call the shared helper
  • Update LensesGenerator.fs to call the shared helper
  • Run integration tests to verify no functionality broken

Analysis Metadata

  • Analyzed Files: 3 (DUCasesGenerator.fs, LensesGenerator.fs, GeneratorHelpers.fs)
  • Detection Method: Semantic code analysis
  • Commit: 165a7e5
  • Analysis Date: 2026-03-07

Generated by Duplicate Code Detector

To install this workflow, run gh aw add githubnext/agentics/workflows/duplicate-code-detector.md@ee49512da7887942965ac0a0e48357106313c9dd. View source at https://github.com/githubnext/agentics/tree/ee49512da7887942965ac0a0e48357106313c9dd/workflows/duplicate-code-detector.md.

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…ication

Co-authored-by: 7sharp9 <588746+7sharp9@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor duplicate resolveCaseIdent logic in generators Refactor: Consolidate duplicate resolveCaseIdent logic into GeneratorHelpers Mar 11, 2026
@7sharp9 7sharp9 marked this pull request as ready for review March 14, 2026 16:55
Copilot AI review requested due to automatic review settings March 14, 2026 16:55

Copilot AI left a comment

Copy link
Copy Markdown

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 consolidates duplicated DU case identifier resolution logic into GeneratorHelpers so both DUCasesGenerator and LensesGenerator apply the same RequireQualifiedAccess-aware naming behavior.

Changes:

  • Added GeneratorHelpers.resolveCaseIdent to resolve (optionally qualified) DU case identifiers.
  • Updated DUCasesGenerator to call the shared helper and removed its local implementation.
  • Updated LensesGenerator to replace inline logic with a call to the shared helper.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Myriad.Plugins/GeneratorHelpers.fs Introduces shared resolveCaseIdent helper for DU case SynLongIdent construction.
src/Myriad.Plugins/DUCasesGenerator.fs Removes duplicated helper and switches call sites to GeneratorHelpers.resolveCaseIdent.
src/Myriad.Plugins/LensesGenerator.fs Replaces inline DU case qualification logic with the shared helper call.

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

@7sharp9

7sharp9 commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts

…plicate-resolvecaseident-logic

# Conflicts:
#	src/Myriad.Plugins/GeneratorHelpers.fs

Co-authored-by: 7sharp9 <588746+7sharp9@users.noreply.github.com>

Copilot AI commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts

Resolved. There was one conflict in GeneratorHelpers.fs where origin/master had added getCaseIdent and getFieldName helpers in the same region as our new resolveCaseIdent. I kept all three helpers and merged in origin/master (commit 346cf56). Build passes with 0 warnings/errors.

Copilot AI requested a review from 7sharp9 June 1, 2026 10:58
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.

Duplicate Code: resolveCaseIdent Logic Duplicated Across DUCasesGenerator and LensesGenerator

3 participants