Merge pull request #5462 from unisonweb/cp/fix-tuple-diffs

Fix diffs involving unit/tuples
unisonweb · Nov 22, 2024 · d7a1edb · d7a1edb
2 parents 3872897 + 4495ac2
commit d7a1edb
Show file tree

Hide file tree

Showing 3 changed files with 677 additions and 19 deletions.
diff --git a/unison-share-api/src/Unison/Server/Backend/DefinitionDiff.hs b/unison-share-api/src/Unison/Server/Backend/DefinitionDiff.hs
@@ -40,18 +40,18 @@ diffSyntaxText (AnnotatedText fromST) (AnnotatedText toST) =
     -- treat these special cases as equal, then we can detect and expand them in a post-processing step.
     diffEq :: AT.Segment Syntax.Element -> AT.Segment Syntax.Element -> Bool
     diffEq (AT.Segment {segment = fromSegment, annotation = fromAnnotation}) (AT.Segment {segment = toSegment, annotation = toAnnotation}) =
-      fromSegment == toSegment ||
-        case (fromAnnotation, toAnnotation) of
+      fromSegment == toSegment
+        || case (fromAnnotation, toAnnotation) of
           (Nothing, _) -> False
           (_, Nothing) -> False
           (Just a, Just b) ->
             case a of
               -- The set of annotations we want to special-case
-              Syntax.TypeReference{} -> a == b
-              Syntax.TermReference{} -> a == b
-              Syntax.DataConstructorReference{} -> a == b
-              Syntax.AbilityConstructorReference{} -> a == b
-              Syntax.HashQualifier{} -> a == b
+              Syntax.TypeReference {} -> a == b
+              Syntax.TermReference {} -> a == b
+              Syntax.DataConstructorReference {} -> a == b
+              Syntax.AbilityConstructorReference {} -> a == b
+              Syntax.HashQualifier {} -> a == b
               _ -> False
 
     expandSpecialCases :: [Diff.Diff [AT.Segment (Syntax.Element)]] -> [SemanticSyntaxDiff]
@@ -67,18 +67,23 @@ diffSyntaxText (AnnotatedText fromST) (AnnotatedText toST) =
                 ( \next acc -> case (acc, next) of
                     (Both xs : rest, Left seg) -> Both (seg : xs) : rest
                     (_, Left seg) -> Both [seg] : acc
-                    (_, Right diff) -> diff : acc
+                    (_, Right diff) -> diff ++ acc
                 )
-    detectSpecialCase :: AT.Segment Syntax.Element -> AT.Segment Syntax.Element -> Either (AT.Segment Syntax.Element) SemanticSyntaxDiff
+    detectSpecialCase :: AT.Segment Syntax.Element -> AT.Segment Syntax.Element -> Either (AT.Segment Syntax.Element) [SemanticSyntaxDiff]
     detectSpecialCase fromSegment toSegment
       | fromSegment == toSegment = Left fromSegment
-      | AT.annotation fromSegment == AT.annotation toSegment = Right (SegmentChange (AT.segment fromSegment, AT.segment toSegment) (AT.annotation fromSegment))
+      | AT.annotation fromSegment == AT.annotation toSegment = Right [SegmentChange (AT.segment fromSegment, AT.segment toSegment) (AT.annotation fromSegment)]
       -- We only emit an annotation change if it's a change in just the hash of the element (optionally the KIND of hash reference can change too).
       | AT.segment fromSegment == AT.segment toSegment,
         Just _fromHash <- AT.annotation fromSegment >>= elementHash,
         Just _toHash <- AT.annotation toSegment >>= elementHash =
-          Right (AnnotationChange (AT.segment fromSegment) (AT.annotation fromSegment, AT.annotation toSegment))
-      | otherwise = error "diffSyntaxText: found Syntax Elements in 'both' which have nothing in common."
+          Right [AnnotationChange (AT.segment fromSegment) (AT.annotation fromSegment, AT.annotation toSegment)]
+      | otherwise =
+          -- the annotation changed, but it's not a recognized hash change.
+          -- This can happen in certain special cases, e.g. a paren changed from being a syntax element into being part
+          -- of a unit.
+          -- We just emit both as old/new segments.
+          Right [Old [fromSegment], New [toSegment]]
       where
         elementHash :: Syntax.Element -> Maybe Syntax.UnisonHash
         elementHash = \case

diff --git a/unison-src/transcripts/definition-diff-api.md b/unison-src/transcripts/definition-diff-api.md
@@ -24,6 +24,10 @@ take n s =
                          else None
     { r }  -> Some r
   handle s() with h n
+
+id x = x
+unitCase = id (x -> 1)
+
 ```
 
 ``` ucm
@@ -53,6 +57,9 @@ take n s =
   if n > 0
     then handle s () with h (n - 1)
     else None
+
+id x = x
+unitCase = id (x -> (1, ()))
 ```
 
 ``` ucm
@@ -71,6 +78,12 @@ More complex diff
 GET /api/projects/diffs/diff/terms?oldBranchRef=main&newBranchRef=new&oldTerm=take&newTerm=take
 ```
 
+Regression test for weird behavior w/r to unit and parens.
+
+``` api
+GET /api/projects/diffs/diff/terms?oldBranchRef=main&newBranchRef=new&oldTerm=unitCase&newTerm=unitCase
+```
+
 
 Diff types