From 052fd5194817140b8525d1e6f4c145b1afec937f Mon Sep 17 00:00:00 2001
From: Mitchell Rosen <mitchellwrosen@gmail.com>
Date: Wed, 3 Jul 2024 11:38:33 -0400
Subject: [PATCH] report missing constructor names in `todo`

---
 .../src/Unison/CommandLine/OutputMessages.hs  | 32 +++++++++++----
 unison-src/transcripts/todo.md                | 22 +++++++++++
 unison-src/transcripts/todo.output.md         | 39 +++++++++++++++++++
 3 files changed, 86 insertions(+), 7 deletions(-)

diff --git a/unison-cli/src/Unison/CommandLine/OutputMessages.hs b/unison-cli/src/Unison/CommandLine/OutputMessages.hs
index 470d6f88a..0095974c1 100644
--- a/unison-cli/src/Unison/CommandLine/OutputMessages.hs
+++ b/unison-cli/src/Unison/CommandLine/OutputMessages.hs
@@ -1402,11 +1402,11 @@ notifyUser dir = \case
               <> "the same on both branches, or making neither of them a builtin, and then try the merge again."
           )
       ]
+  -- Note [ConstructorAliasMessage] If you change this, also change the other similar one
   MergeConstructorAlias aliceOrBob typeName conName1 conName2 ->
     pure . P.lines $
       [ P.wrap "Sorry, I wasn't able to perform the merge:",
         "",
-        -- Note [ConstructorAliasMessage] If you change this, also change the other similar one
         P.wrap $
           "On"
             <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",")
@@ -1418,6 +1418,7 @@ notifyUser dir = \case
         "",
         P.wrap "Please delete all but one name for each constructor, and then try merging again."
       ]
+  -- Note [DefnsInLibMessage] If you change this, also change the other similar one
   MergeDefnsInLib aliceOrBob ->
     pure . P.lines $
       [ P.wrap "Sorry, I wasn't able to perform the merge:",
@@ -1425,12 +1426,12 @@ notifyUser dir = \case
         P.wrap $
           "On"
             <> P.group (prettyMergeSourceOrTarget aliceOrBob <> ",")
-            -- Note [DefnsInLibMessage] If you change this, also change the other similar one
             <> "there's a type or term at the top level of the `lib` namespace, where I only expect to find"
             <> "subnamespaces representing library dependencies.",
         "",
         P.wrap "Please move or remove it and then try merging again."
       ]
+  -- Note [MissingConstructorNameMessage] If you change this, also change the other similar one
   MergeMissingConstructorName aliceOrBob name ->
     pure . P.lines $
       [ P.wrap "Sorry, I wasn't able to perform the merge:",
@@ -2731,11 +2732,11 @@ handleTodoOutput todo
               else mempty
 
       prettyConstructorAliases <-
-        if null todo.incoherentDeclReasons.constructorAliases
-          then pure mempty
-          else do
+        case todo.incoherentDeclReasons.constructorAliases of
+          [] -> pure mempty
+          aliases -> do
             things <-
-              for todo.incoherentDeclReasons.constructorAliases \(typeName, conName1, conName2) -> do
+              for aliases \(typeName, conName1, conName2) -> do
                 n1 <- addNumberedArg (SA.Name conName1)
                 n2 <- addNumberedArg (SA.Name conName2)
                 pure (typeName, formatNum n1 <> prettyName conName1, formatNum n2 <> prettyName conName2)
@@ -2756,13 +2757,30 @@ handleTodoOutput todo
                   )
                 & P.sep "\n\n"
 
+      prettyMissingConstructorNames <-
+        case todo.incoherentDeclReasons.missingConstructorNames of
+          [] -> pure mempty
+          types0 -> do
+            types1 <-
+              for types0 \typ -> do
+                n <- addNumberedArg (SA.Name typ)
+                pure (formatNum n <> prettyName typ)
+            -- Note [MissingConstructorNameMessage] If you change this, also change the other similar one
+            pure $
+              P.wrap
+                "These types have some constructors with missing names:"
+                <> P.newline
+                <> P.newline
+                <> P.indentN 2 (P.lines types1)
+
       (pure . P.sep "\n\n" . P.nonEmpty)
         [ prettyDependentsOfTodo,
           prettyDirectTermDependenciesWithoutNames,
           prettyDirectTypeDependenciesWithoutNames,
           prettyConflicts,
           prettyDefnsInLib,
-          prettyConstructorAliases
+          prettyConstructorAliases,
+          prettyMissingConstructorNames
         ]
 
 listOfDefinitions ::
diff --git a/unison-src/transcripts/todo.md b/unison-src/transcripts/todo.md
index 5b4a40dec..20ed37146 100644
--- a/unison-src/transcripts/todo.md
+++ b/unison-src/transcripts/todo.md
@@ -120,3 +120,25 @@ scratch/main> todo
 ```ucm:hide
 scratch/main> delete.project scratch
 ```
+
+# Missing constructor names
+
+The `todo` command complains about missing constructor names.
+
+```ucm:hide
+scratch/main> builtins.mergeio lib.builtins
+```
+
+```unison
+type Foo = Bar
+```
+
+```ucm
+scratch/main> add
+scratch/main> delete.term Foo.Bar
+scratch/main> todo
+```
+
+```ucm:hide
+scratch/main> delete.project scratch
+```
diff --git a/unison-src/transcripts/todo.output.md b/unison-src/transcripts/todo.output.md
index 0de57bd2c..0e8e23332 100644
--- a/unison-src/transcripts/todo.output.md
+++ b/unison-src/transcripts/todo.output.md
@@ -222,3 +222,42 @@ scratch/main> todo
     2. Foo.Two
 
 ```
+# Missing constructor names
+
+The `todo` command complains about missing constructor names.
+
+```unison
+type Foo = Bar
+```
+
+```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`:
+    
+      type Foo
+
+```
+```ucm
+scratch/main> add
+
+  ⍟ I've added these definitions:
+  
+    type Foo
+
+scratch/main> delete.term Foo.Bar
+
+  Done.
+
+scratch/main> todo
+
+  These types have some constructors with missing names:
+  
+    1. Foo
+
+```