fix upgrade ppe

unisonweb · Sep 3, 2024 · 20b67e6 · 20b67e6
1 parent 4e1a309
commit 20b67e6
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 63 deletions.
diff --git a/parser-typechecker/src/Unison/PrettyPrintEnvDecl/Names.hs b/parser-typechecker/src/Unison/PrettyPrintEnvDecl/Names.hs
@@ -1,11 +1,8 @@
 module Unison.PrettyPrintEnvDecl.Names
   ( makePPED,
-    makeFilePPED,
-    makeCodebasePPED,
   )
 where
 
-import Unison.Names (Names)
 import Unison.PrettyPrintEnv.Names qualified as PPE
 import Unison.PrettyPrintEnvDecl (PrettyPrintEnvDecl (PrettyPrintEnvDecl))
 
@@ -14,23 +11,3 @@ makePPED namer suffixifier =
   PrettyPrintEnvDecl
     (PPE.makePPE namer PPE.dontSuffixify)
     (PPE.makePPE namer suffixifier)
-
--- | Make a PPED suitable for names in a Unison file.
---
--- Such names have special suffixification rules: aliases may *not* be referred to by a common suffix. For example, if
--- a file contains
---
---   one.foo = 6
---   two.foo = 6
---
--- then the suffix `foo` will *not* be accepted (currently). So, this PPE uses the "suffixify by name" strategy.
-makeFilePPED :: Names -> PrettyPrintEnvDecl
-makeFilePPED names =
-  makePPED (PPE.namer names) (PPE.suffixifyByName names)
-
--- | Make a PPED suitable for names in the codebase. These names are hash qualified and suffixified by hash.
-makeCodebasePPED :: Names -> PrettyPrintEnvDecl
-makeCodebasePPED names =
-  makePPED
-    (PPE.hqNamer 10 names)
-    (PPE.suffixifyByHash names)
diff --git a/unison-cli/src/Unison/Codebase/Editor/HandleInput/Upgrade.hs b/unison-cli/src/Unison/Codebase/Editor/HandleInput/Upgrade.hs
@@ -59,7 +59,7 @@ import Unison.PrettyPrintEnv qualified as PPE
 import Unison.PrettyPrintEnv.Names qualified as PPE
 import Unison.PrettyPrintEnvDecl (PrettyPrintEnvDecl (..))
 import Unison.PrettyPrintEnvDecl qualified as PPED (addFallback)
-import Unison.PrettyPrintEnvDecl.Names qualified as PPED (makeCodebasePPED, makeFilePPED)
+import Unison.PrettyPrintEnvDecl.Names qualified as PPED (makePPED)
 import Unison.Project (ProjectBranchName)
 import Unison.Reference (TermReference, TermReferenceId, TypeReference, TypeReferenceId)
 import Unison.Reference qualified as Reference
@@ -162,15 +162,23 @@ handleUpgrade oldName newName = do
           UnisonFile.emptyUnisonFile
       pure
         ( unisonFile,
-          makeOldDepPPE
-            oldName
-            newName
-            currentDeepNamesSansOld
-            (Branch.toNames oldNamespace)
-            (Branch.toNames oldLocalNamespace)
-            (Branch.toNames newLocalNamespace)
-            `PPED.addFallback` PPED.makeFilePPED (Names.fromReferenceIds dependents)
-            `PPED.addFallback` PPED.makeCodebasePPED currentDeepNamesSansOld
+          let ppe1 =
+                makeOldDepPPE
+                  oldName
+                  newName
+                  currentDeepNamesSansOld
+                  (Branch.toNames oldNamespace)
+                  (Branch.toNames oldLocalNamespace)
+                  (Branch.toNames newLocalNamespace)
+              ppe2 =
+                PPED.makePPED
+                  (PPE.namer (Names.fromReferenceIds dependents))
+                  (PPE.suffixifyByName currentDeepNamesSansOld)
+              ppe3 =
+                PPED.makePPED
+                  (PPE.hqNamer 10 currentDeepNamesSansOld)
+                  (PPE.suffixifyByHash currentDeepNamesSansOld)
+           in ppe1 `PPED.addFallback` ppe2 `PPED.addFallback` ppe3
         )
 
   pp@(PP.ProjectPath project projectBranch _path) <- Cli.getCurrentProjectPath
@@ -300,12 +308,12 @@ makeUnisonFile abort codebase doFindCtorNames defns = do
                 overwriteConstructorNames name ed.toDataDecl <&> \ed' ->
                   uf
                     & #effectDeclarationsId
-                    %~ Map.insertWith (\_new old -> old) (Name.toVar name) (Reference.Id h i, Decl.EffectDeclaration ed')
+                      %~ Map.insertWith (\_new old -> old) (Name.toVar name) (Reference.Id h i, Decl.EffectDeclaration ed')
               Right dd ->
                 overwriteConstructorNames name dd <&> \dd' ->
                   uf
                     & #dataDeclarationsId
-                    %~ Map.insertWith (\_new old -> old) (Name.toVar name) (Reference.Id h i, dd')
+                      %~ Map.insertWith (\_new old -> old) (Name.toVar name) (Reference.Id h i, dd')
 
         -- Constructor names are bogus when pulled from the database, so we set them to what they should be here
         overwriteConstructorNames :: Name -> DataDeclaration Symbol Ann -> Transaction (DataDeclaration Symbol Ann)

diff --git a/unison-src/transcripts/fix-5323.md b/unison-src/transcripts/fix-5323.md
@@ -16,6 +16,6 @@ c = b.y + 1
 scratch/main> add
 ```
 
-```ucm:error
+```ucm
 scratch/main> upgrade old new
 ```
diff --git a/unison-src/transcripts/fix-5323.output.md b/unison-src/transcripts/fix-5323.output.md
@@ -46,32 +46,6 @@ scratch/main> add
 ``` ucm
 scratch/main> upgrade old new
 
-  I couldn't automatically upgrade old to new. However, I've
-  added the definitions that need attention to the top of
-  scratch.u.
-  
-  When you're done, you can run
-  
-    upgrade.commit
-  
-  to merge your changes back into main and delete the temporary
-  branch. Or, if you decide to cancel the upgrade instead, you
-  can run
-  
-    delete.branch /upgrade-old-to-new
-  
-  to delete the temporary branch and switch back to main.
-
-```
-``` unison:added-by-ucm scratch.u
-b.y : Nat
-b.y =
-  use Nat +
-  x + 1
+  I upgraded old to new, and removed old.
 
-c : Nat
-c =
-  use Nat +
-  y + 1
 ```
-