render merge conflicts preferring Alice's names

lib/unison-util-relation/src/Unison/Util/Relation.hs

@@ -88,6 +88,8 @@ module Unison.Util.Relation
     outerJoinRanMultimaps,
     union,
     unions,
+    unionDomainWith,
+    unionRangeWith,
 
     -- * Converting to other data structures
     toList,
@@ -230,6 +232,14 @@ union r s =
       range = M.unionWith S.union (range r) (range s)
     }
 
+unionDomainWith :: (Ord a, Ord b) => (a -> Set b -> Set b -> Set b) -> Relation a b -> Relation a b -> Relation a b
+unionDomainWith f xs ys =
+  fromMultimap (Map.unionWithKey f (domain xs) (domain ys))
+
+unionRangeWith :: (Ord a, Ord b) => (b -> Set a -> Set a -> Set a) -> Relation a b -> Relation a b -> Relation a b
+unionRangeWith f xs ys =
+  swap (fromMultimap (Map.unionWithKey f (range xs) (range ys)))
+
 intersection :: (Ord a, Ord b) => Relation a b -> Relation a b -> Relation a b
 intersection r s =
   Relation

unison-core/src/Unison/Names.hs

@@ -41,6 +41,7 @@ module Unison.Names
     typesNamed,
     shadowing,
     shadowing1,
+    preferring,
     namesForReference,
     namesForReferent,
     shadowTerms,
@@ -210,16 +211,31 @@ restrictReferences refs Names {..} = Names terms' types'
     terms' = R.filterRan ((`Set.member` refs) . Referent.toReference) terms
     types' = R.filterRan (`Set.member` refs) types
 
--- | Prefer names in the first argument, falling back to names in the second.
--- This can be used to shadow names in the codebase with names in a unison file for instance:
--- e.g. @shadowing scratchFileNames codebaseNames@
+-- | Construct names from a left-biased map union of the domains of the input names. That is, for each distinct name,
+-- if it refers to *any* references in the left argument, use those (ignoring the right).
+--
+-- This is appropriate for shadowing names in the codebase with names in a Unison file, for instance:
+--
+-- @shadowing scratchFileNames codebaseNames@
 shadowing :: Names -> Names -> Names
 shadowing a b =
   Names (shadowing1 a.terms b.terms) (shadowing1 a.types b.types)
 
 shadowing1 :: (Ord a, Ord b) => Relation a b -> Relation a b -> Relation a b
-shadowing1 xs ys =
-  Relation.fromMultimap (Map.unionWith (\x _ -> x) (Relation.domain xs) (Relation.domain ys))
+shadowing1 =
+  Relation.unionDomainWith (\_ x _ -> x)
+
+-- | Construct names from a left-biased map union of the ranges of the input names. That is, for each distinct
+-- reference, if it is referred to by *any* names in the left argument, use those (ignoring the right).
+--
+-- This is appropriate for biasing a PPE towards picking names in the left argument.
+preferring :: Names -> Names -> Names
+preferring xs ys =
+  Names (preferring1 xs.terms ys.terms) (preferring1 xs.types ys.types)
+  where
+    preferring1 :: (Ord a, Ord b) => Relation a b -> Relation a b -> Relation a b
+    preferring1 =
+      Relation.unionRangeWith (\_ x _ -> x)
 
 -- | TODO: get this from database. For now it's a constant.
 numHashChars :: Int

unison-merge/src/Unison/Merge/Mergeblob3.hs

@@ -19,7 +19,7 @@ import Unison.DataDeclaration qualified as DataDeclaration
 import Unison.DeclNameLookup (DeclNameLookup, expectConstructorNames)
 import Unison.DeclNameLookup qualified as DeclNameLookup
 import Unison.Merge.Mergeblob2 (Mergeblob2 (..))
-import Unison.Merge.PrettyPrintEnv (makePrettyPrintEnvs)
+import Unison.Merge.PrettyPrintEnv (makePrettyPrintEnv)
 import Unison.Merge.ThreeWay (ThreeWay)
 import Unison.Merge.ThreeWay qualified as ThreeWay
 import Unison.Merge.TwoWay (TwoWay)
@@ -30,6 +30,7 @@ import Unison.Name (Name)
 import Unison.Names (Names (..))
 import Unison.Parser.Ann (Ann)
 import Unison.Prelude
+import Unison.PrettyPrintEnvDecl (PrettyPrintEnvDecl)
 import Unison.Reference (Reference' (..), TermReferenceId, TypeReference, TypeReferenceId)
 import Unison.Referent (Referent)
 import Unison.Referent qualified as Referent
@@ -256,13 +257,12 @@ renderConflictsAndDependents ::
   )
 renderConflictsAndDependents declNameLookups hydratedDefns conflicts dependents names libdepsNames =
   unzip $
-    ( \declNameLookup (conflicts, dependents) ppe ->
+    ( \declNameLookup (conflicts, dependents) ->
         let render = renderDefnsForUnisonFile declNameLookup ppe . over (#terms . mapped) snd
          in (render conflicts, render dependents)
     )
       <$> declNameLookups
       <*> hydratedConflictsAndDependents
-      <*> makePrettyPrintEnvs names libdepsNames
   where
     hydratedConflictsAndDependents ::
       TwoWay
@@ -279,6 +279,10 @@ renderConflictsAndDependents declNameLookups hydratedDefns conflicts dependents
         <*> conflicts
         <*> dependents
 
+    ppe :: PrettyPrintEnvDecl
+    ppe =
+      makePrettyPrintEnv names libdepsNames
+
 defnsToNames :: Defns (BiMultimap Referent Name) (BiMultimap TypeReference Name) -> Names
 defnsToNames defns =
   Names

unison-merge/src/Unison/Merge/PrettyPrintEnv.hs

@@ -1,18 +1,21 @@
 module Unison.Merge.PrettyPrintEnv
-  ( makePrettyPrintEnvs,
+  ( makePrettyPrintEnv,
   )
 where
 
-import Unison.Merge.TwoWay (TwoWay)
+import Unison.Merge.TwoWay (TwoWay (..))
 import Unison.Names (Names)
+import Unison.Names qualified as Names
 import Unison.Prelude
 import Unison.PrettyPrintEnv.Names qualified as PPE
 import Unison.PrettyPrintEnvDecl (PrettyPrintEnvDecl)
 import Unison.PrettyPrintEnvDecl.Names qualified as PPED
 
--- Make PPE for Alice that contains all of Alice's names, but suffixified against her names + Bob's names
-makePrettyPrintEnvs :: TwoWay Names -> Names -> TwoWay PrettyPrintEnvDecl
-makePrettyPrintEnvs names2 libdepsNames =
-  names2 <&> \names -> PPED.makePPED (PPE.namer (names <> libdepsNames)) suffixifier
+-- Create a PPE that uses Alice's names whenever possible, falling back to Bob's names only when Alice doesn't have any.
+-- This results in a file that "looks familiar" to Alice (the one merging in Bob's changes), and avoids superfluous
+-- textual conflicts that would arise from preferring Bob's names for Bob's code (where his names differ).
+makePrettyPrintEnv :: TwoWay Names -> Names -> PrettyPrintEnvDecl
+makePrettyPrintEnv names libdepsNames =
+  PPED.makePPED (PPE.namer (Names.preferring names.alice names.bob <> libdepsNames)) suffixifier
   where
-    suffixifier = PPE.suffixifyByName (fold names2 <> libdepsNames)
+    suffixifier = PPE.suffixifyByName (fold names <> libdepsNames)

unison-src/transcripts/merge.md

@@ -1747,3 +1747,51 @@ scratch/alice> names Bar
 ``` ucm :hide
 scratch/main> project.delete scratch
 ```
+
+### Using Alice's names for Bob's things
+
+Previously, we'd render Alice's stuff with her names and Bob's stuff with his. But because Alice is doing the merge,
+we now use her names whenever possible. In this example, Alice calls something `foo` and Bob calls it `bar`. When
+rendering conflicts, in Bob's term that references (what he calls) `bar`, we render `foo` instead.
+
+``` ucm :hide
+scratch/main> builtins.mergeio lib.builtins
+```
+
+``` unison
+hello = 17
+```
+
+``` ucm
+scratch/main> add
+scratch/main> branch alice
+```
+
+``` unison
+hello = 18 + foo
+foo = 100
+```
+
+``` ucm
+scratch/alice> update
+scratch/main> branch bob
+```
+
+``` unison
+hello = 19 + bar
+bar = 100
+```
+
+``` ucm
+scratch/bob> update
+```
+
+Note Bob's `hello` references `foo` (Alice's name), not `bar` (Bob's name).
+
+``` ucm :error
+scratch/alice> merge /bob
+```
+
+``` ucm :hide
+scratch/main> project.delete scratch
+```

unison-src/transcripts/merge.output.md

@@ -2425,3 +2425,149 @@ scratch/alice> names Bar
   Names: Bar
 
 ```
+### Using Alice's names for Bob's things
+
+Previously, we'd render Alice's stuff with her names and Bob's stuff with his. But because Alice is doing the merge,
+we now use her names whenever possible. In this example, Alice calls something `foo` and Bob calls it `bar`. When
+rendering conflicts, in Bob's term that references (what he calls) `bar`, we render `foo` instead.
+
+``` unison
+hello = 17
+```
+
+``` ucm
+
+  Loading changes detected in scratch.u.
+
+  I found and typechecked these definitions in scratch.u. If you
+  do an `add` or `update`, here's how your codebase would
+  change:
+  
+    ⍟ These new definitions are ok to `add`:
+    
+      hello : Nat
+
+```
+``` ucm
+scratch/main> add
+
+  ⍟ I've added these definitions:
+  
+    hello : Nat
+
+scratch/main> branch alice
+
+  Done. I've created the alice branch based off of main.
+  
+  Tip: To merge your work back into the main branch, first
+       `switch /main` then `merge /alice`.
+
+```
+``` unison
+hello = 18 + foo
+foo = 100
+```
+
+``` ucm
+
+  Loading changes detected in scratch.u.
+
+  I found and typechecked these definitions in scratch.u. If you
+  do an `add` or `update`, here's how your codebase would
+  change:
+  
+    ⍟ These new definitions are ok to `add`:
+    
+      foo : Nat
+    
+    ⍟ These names already exist. You can `update` them to your
+      new definition:
+    
+      hello : Nat
+
+```
+``` ucm
+scratch/alice> update
+
+  Okay, I'm searching the branch for code that needs to be
+  updated...
+
+  Done.
+
+scratch/main> branch bob
+
+  Done. I've created the bob branch based off of main.
+  
+  Tip: To merge your work back into the main branch, first
+       `switch /main` then `merge /bob`.
+
+```
+``` unison
+hello = 19 + bar
+bar = 100
+```
+
+``` ucm
+
+  Loading changes detected in scratch.u.
+
+  I found and typechecked these definitions in scratch.u. If you
+  do an `add` or `update`, here's how your codebase would
+  change:
+  
+    ⍟ These new definitions are ok to `add`:
+    
+      bar : Nat
+    
+    ⍟ These names already exist. You can `update` them to your
+      new definition:
+    
+      hello : Nat
+
+```
+``` ucm
+scratch/bob> update
+
+  Okay, I'm searching the branch for code that needs to be
+  updated...
+
+  Done.
+
+```
+Note Bob's `hello` references `foo` (Alice's name), not `bar` (Bob's name).
+
+``` ucm
+scratch/alice> merge /bob
+
+  I couldn't automatically merge scratch/bob into scratch/alice.
+  However, I've added the definitions that need attention to the
+  top of scratch.u.
+  
+  When you're done, you can run
+  
+    merge.commit
+  
+  to merge your changes back into alice and delete the temporary
+  branch. Or, if you decide to cancel the merge instead, you can
+  run
+  
+    delete.branch /merge-bob-into-alice
+  
+  to delete the temporary branch and switch back to alice.
+
+```
+``` unison :added-by-ucm scratch.u
+-- scratch/alice
+hello : Nat
+hello =
+  use Nat +
+  18 + foo
+
+-- scratch/bob
+hello : Nat
+hello =
+  use Nat +
+  19 + foo
+
+```
+