Skip to content

Commit

Permalink
report nested decl aliases in todo
Browse files Browse the repository at this point in the history
  • Loading branch information
mitchellwrosen committed Jul 3, 2024
1 parent 052fd51 commit 1857640
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 25 deletions.
94 changes: 69 additions & 25 deletions unison-cli/src/Unison/CommandLine/OutputMessages.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,7 @@ notifyUser dir = \case
<> IP.makeExample IP.aliasTerm ["<hash>", prettyName name <> ".<ConstructorName>"]
<> "to give names to each unnamed constructor, and then try the merge again."
]
-- Note [NestedDeclAliasMessage] If you change this, also change the other similar one
MergeNestedDeclAlias aliceOrBob shorterName longerName ->
pure . P.wrap $
"On"
Expand Down Expand Up @@ -2732,30 +2733,48 @@ handleTodoOutput todo
else mempty

prettyConstructorAliases <-
case todo.incoherentDeclReasons.constructorAliases of
[] -> pure mempty
aliases -> do
things <-
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)
pure $
things
& map
( \(typeName, prettyCon1, prettyCon2) ->
-- Note [ConstructorAliasMessage] If you change this, also change the other similar one
P.wrap
( "The type"
<> prettyName typeName
<> "has a constructor with multiple names. Please delete all but one name for each"
<> "constructor."
)
<> P.newline
<> P.newline
<> P.indentN 2 (P.lines [prettyCon1, prettyCon2])
)
& P.sep "\n\n"
let -- We want to filter out constructor aliases whose types are part of a "nested decl alias" problem, because
-- otherwise we'd essentially be reporting those issues twice.
--
-- That is, if we have two nested aliases like
--
-- Foo = #XYZ
-- Foo.Bar = #XYZ#0
--
-- Foo.inner.Alias = #XYZ
-- Foo.inner.Alias.Constructor = #XYZ#0
--
-- then we'd prefer to say "oh no Foo and Foo.inner.Alias are aliases" but *not* additionally say "oh no
-- Foo.Bar and Foo.inner.Alias.Constructor are aliases".
notNestedDeclAlias (typeName, _, _) =
foldr
(\(short, long) acc -> typeName /= short && typeName /= long && acc)
True
todo.incoherentDeclReasons.nestedDeclAliases
in case filter notNestedDeclAlias todo.incoherentDeclReasons.constructorAliases of
[] -> pure mempty
aliases -> do
things <-
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)
pure $
things
& map
( \(typeName, prettyCon1, prettyCon2) ->
-- Note [ConstructorAliasMessage] If you change this, also change the other similar one
P.wrap
( "The type"
<> prettyName typeName
<> "has a constructor with multiple names. Please delete all but one name for each"
<> "constructor."
)
<> P.newline
<> P.newline
<> P.indentN 2 (P.lines [prettyCon1, prettyCon2])
)
& P.sep "\n\n"

prettyMissingConstructorNames <-
case todo.incoherentDeclReasons.missingConstructorNames of
Expand All @@ -2773,14 +2792,39 @@ handleTodoOutput todo
<> P.newline
<> P.indentN 2 (P.lines types1)

prettyNestedDeclAliases <-
case todo.incoherentDeclReasons.nestedDeclAliases of
[] -> pure mempty
aliases0 -> do
aliases1 <-
for aliases0 \(short, long) -> do
n1 <- addNumberedArg (SA.Name short)
n2 <- addNumberedArg (SA.Name long)
pure (formatNum n1 <> prettyName short, formatNum n2 <> prettyName long)
-- Note [NestedDeclAliasMessage] If you change this, also change the other similar one
pure $
aliases1
& map
( \(short, long) ->
P.wrap
( "These types are aliases, but one is nested under the other. Please separate them or delete"
<> "one copy."
)
<> P.newline
<> P.newline
<> P.indentN 2 (P.lines [short, long])
)
& P.sep "\n\n"

(pure . P.sep "\n\n" . P.nonEmpty)
[ prettyDependentsOfTodo,
prettyDirectTermDependenciesWithoutNames,
prettyDirectTypeDependenciesWithoutNames,
prettyConflicts,
prettyDefnsInLib,
prettyConstructorAliases,
prettyMissingConstructorNames
prettyMissingConstructorNames,
prettyNestedDeclAliases
]

listOfDefinitions ::
Expand Down
22 changes: 22 additions & 0 deletions unison-src/transcripts/todo.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,25 @@ scratch/main> todo
```ucm:hide
scratch/main> delete.project scratch
```

# Nested decl aliases

The `todo` command complains about nested decl aliases.

```ucm:hide
scratch/main> builtins.mergeio lib.builtins
```

```unison
structural type Foo a = One a | Two a a
structural type Foo.inner.Bar a = Uno a | Dos a a
```

```ucm
scratch/main> add
scratch/main> todo
```

```ucm:hide
scratch/main> delete.project scratch
```
40 changes: 40 additions & 0 deletions unison-src/transcripts/todo.output.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,43 @@ scratch/main> todo
1. Foo
```
# Nested decl aliases

The `todo` command complains about nested decl aliases.

```unison
structural type Foo a = One a | Two a a
structural type Foo.inner.Bar a = Uno a | Dos a a
```

```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`:
structural type Foo a
structural type Foo.inner.Bar a
```
```ucm
scratch/main> add
⍟ I've added these definitions:
structural type Foo a
structural type Foo.inner.Bar a
scratch/main> todo
These types are aliases, but one is nested under the other.
Please separate them or delete one copy.
1. Foo
2. Foo.inner.Bar
```

0 comments on commit 1857640

Please sign in to comment.