Skip to content

fix: use old type name in Union migration type signatures for renamed types - #81

Merged
supermario merged 2 commits into
lamdera:lamdera-nextfrom
CharlonTank:fix/migration-union-type-rename
Oct 23, 2026
Merged

supermario merged 2 commits into
lamdera:lamdera-nextfrom
CharlonTank:fix/migration-union-type-rename

Conversation

@CharlonTank

@CharlonTank CharlonTank commented Mar 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Fixes migration generator producing incorrect type signatures when a Union type is renamed between versions
  • The old type name is now correctly threaded through migrateUnionDefinition → migrateUnionDefinition_, mirroring how migrateAliasDefinition already handles alias renames

Problem

When a Union type is renamed between versions (e.g. MoralType in V51 → CompanyType in V52), the migration generator was using the new type name for the old version in the generated type signature:

migrate_LegalEntity_CompanyType : Evergreen.V51.LegalEntity.CompanyType -> Evergreen.V52.LegalEntity.CompanyType

But CompanyType doesn't exist in V51 — it was called MoralType. This causes a compilation error in the generated migration file.

Fix

The correct signature should be:

migrate_LegalEntity_CompanyType : Evergreen.V51.LegalEntity.MoralType -> Evergreen.V52.LegalEntity.CompanyType

Changes in MigrationGenerator.hs:

  1. coreTypeMigration: Pass typeNameOld (from pattern match) instead of typeName (new name) to migrateUnionDefinition
  2. migrateUnionDefinition: Renamed parameter from typeNameNew to typeNameOld, derive typeNameNew from the identifier's tipe component, and thread both to migrateUnionDefinition_
  3. migrateUnionDefinition_: Added typeNameOld parameter; use it in the type signature for the old version's type reference (line 279)
  4. migrateTypeDef: Pass typeNameOld instead of typeNameNew to migrateUnionDefinition

Reference

The Alias case (migrateAliasDefinition) already correctly passes and uses both typeNameOld and typeNameNew — this fix brings the Union case in line with that pattern.

Test plan

  • Rename a Union type between two Evergreen versions (e.g. type MoralType = ... → type CompanyType = ...)
  • Run lamdera check and verify the generated migration file uses the old type name for the old version in the type signature
  • Verify the generated migration file compiles without errors

Also in this PR: nested Dict/SeqDict pipelines

The second commit (a4605ae) fixes a separate generator bug. A key migration of a Dict/SeqDict is emitted as a raw pipeline (Dict.toList |> List.map (…) |> Dict.fromList), which is not a function once it is passed as an argument (Tuple.mapBoth, List.map, a nested Dict.map…). It is now wrapped in (\v__ -> v__ |> …). The test scenarios Migrate_Union_Renamed and Migrate_Nested_Dict cover both fixes.

#111 fixes the same pipeline problem at the source, in the Dict/SeqDict templates (composition with >>, which binds no name), and also fixes the shadowed \k of nested value migrations. With #111 merged, this wrapper no longer triggers for those templates and the Migrate_Nested_Dict snapshot needs regenerating.

@CharlonTank

Copy link
Copy Markdown
Contributor Author

I built the fixed compiler locally and tested it against my project which has a Union type rename (MoralType → CompanyType between V51 and V52).

Before the fix:

migrate_LegalEntity_CompanyType : Evergreen.V51.LegalEntity.CompanyType -> Evergreen.V52.LegalEntity.CompanyType

→ Compilation error: Evergreen.V51.LegalEntity.CompanyType doesn't exist

After the fix:

migrate_LegalEntity_CompanyType : Evergreen.V51.LegalEntity.MoralType -> Evergreen.V52.LegalEntity.CompanyType

→ Correct type signature, compiles fine

@CharlonTank

Copy link
Copy Markdown
Contributor Author

Added second fix: nested Dict/SeqDict pipeline wrapping

When a container type has a nested Dict/SeqDict parameter, the recursive migration produces a pipeline (SeqDict.toList |> ... |> SeqDict.fromList) that gets inserted directly as a function argument to Tuple.mapBoth, which is syntactically invalid.

Before: Tuple.mapBoth (migrate_Email) (SeqDict.toList |> List.map (...) |> SeqDict.fromList)
After: Tuple.mapBoth (migrate_Email) (\v__ -> v__ |> SeqDict.toList |> List.map (...) |> SeqDict.fromList)

Fix: In applyMigration, detect pipeline migrations (containing |>) and wrap them in a lambda — following the same pattern already used for anonymous records.

Both fixes tested locally — built the compiler and verified the generated migration compiles correctly.

@supermario

Copy link
Copy Markdown
Member

@CharlonTank could you please add some test cases covering these changes?

@CharlonTank

CharlonTank commented Apr 27, 2026 •

Copy link
Copy Markdown
Contributor Author

@supermario added two test scenarios in test/scenario-migration-generate/src/:

  • Migrate_Union_Renamed — covers fix number 1. Old has type MoralType, New has type CompanyType, both reachable via a Wrapper constructor in Target. Verified the test fails without the fix (Actual generates Old.CompanyType -> New.CompanyType, which doesn't exist in V_old) and passes with it.

  • Migrate_Nested_Dict — covers fix number 2. Target = Wrapper (Dict Int (Dict Int Int)) → Wrapper (Dict String (Dict String Int)), which forces both outer params to change and produces a nested pipeline as the second arg to Tuple.mapBoth. Without the fix, Actual generates Tuple.mapBoth (...) (Dict.toList |> ... |> Dict.fromList) (invalid as a function argument); with the fix it generates the (\v__ -> v__ |> ...) wrapper.

While adding these, I noticed fix number 2's "|>" \isInfixOf`check was too broad — it double-wrapped already-parenthesised lambdas likeMigrate_All's UserListTriple (((t1,t2,t3) -> (t1, t2, t3 |> migrate_X))), causing that existing test to fail. Tightened the check to skip migrations that already start with (`. All 7 tests pass now.

CharlonTank and others added 2 commits October 23, 2026 23:06
… types

When a Union type is renamed between versions (e.g. MoralType in V51 →
CompanyType in V52), the migration generator was using the NEW type name
for the OLD version in the generated type signature:

  migrate_LegalEntity_CompanyType : Evergreen.V51.LegalEntity.CompanyType -> ...

But CompanyType doesn't exist in V51 - it was called MoralType. The correct
signature should be:

  migrate_LegalEntity_CompanyType : Evergreen.V51.LegalEntity.MoralType -> ...

The fix threads typeNameOld through migrateUnionDefinition →
migrateUnionDefinition_, mirroring how migrateAliasDefinition already
correctly handles this case for type alias renames.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Migrate_Union_Renamed verifies the V_old type signature uses the old
type name when a Union is renamed across versions.

Migrate_Nested_Dict verifies that a Dict migration nested inside
another one stays a valid function argument (composed with >>, from
lamdera#111).

Rebased onto lamdera-next: the lambda-wrapping commit and its heuristic
tweak are dropped, lamdera#111 fixed the templates at the source, and the
Migrate_Nested_Dict snapshot is regenerated for its output.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@supermario
supermario force-pushed the fix/migration-union-type-rename branch from ee65f89 to 1a338da Compare October 23, 2026 12:10
@supermario
supermario merged commit f4f693c into lamdera:lamdera-next Oct 23, 2026
1 check failed
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.

2 participants