* Fix `dhall-json`'s support for preserving alternative names
... as caught by @sjakobi in the course of #1077
The code has to be updated to reflect the new normal forms for
union literals
* Add tests
... as suggested by @sjakobi
* Improve type-on-hover behaviour over binders
Dhall's parser does not generate `Src` annotations for the names in
binders, which means that it's not quite as straight-forward to check
whether the user meant to hover over it and see its type as one would
hope. As a result, the current "naive" implementation sometimes behaves
slightly unintuitively and returns the type of the entire expression,
rather than the type of the bound variable. E.g.
let x = 2 in "text"
~
would result in the type `Text` being displayed, rather than `Natural`
as one would expect. Since the language server also marks the expression
whose type is being displayed, the behaviour isn't quite that bad in
practice (we aren't falsely led to believe that `x` has type `Text`).
This change recovers the `Src` information describing the bound variable
names and handles those cases in a way that should be more in line with
the expectations of the user.
* Fix exprAt not handling multi-lets correctly
* Fix merge left-overs
* Normalize types before displaying them to the user
Previously, the type displayed when hovering over the arrow in a lambda
(or forall) binder would not always be normal, e.g.
\(n : {Left = Natural, Right = Natural}.Left) -> n + n
~
would result in
forall (n : {Left = Natural, Right = Natural}.Left) -> Natural
being displayed.
The (expected) normal type would be:
forall (n : Natural) -> Natural
This fix normalises types before displaying them to the user (fixes both
the type-on-hover and annotate-let behaviour).
* Move normalisation from typeAt' to typeAt
This way we only have to worry about it one place, rather than in each
clause of `typeAt'`.
Related to https://github.com/dhall-lang/dhall-haskell/issues/1058
This is an attempt to test drive the Mergify GitHub app. Unfortunately, the
only way to know for sure if the app works is to merge this configuration
into `master`, but if it does not work out then I will follow up with a pull
request to revert this change.
This gets rid of the separate `santiseRange` utility function that we
used to exclude trailing whitespace from source code ranges. As a result
of this change, we no longer have to carry around the original source
code everywhere to be able to santise ranges after the fact.
Fixes#973
The formatter was behaving inconsistently for multi-line strings
depending on whether or not they were at the top level or nested as
a subexpression. For example, the same multi-line string would be
rendered in multi-line form if it was at the top level and rendered
compactly if nested within another expression.
The root cause was that the pretty-printing logic was missing a top-level
`Pretty.group` call (which is the function responsible for enabling
compact representations if they fit), which this change fixes.
While benchmarking the example from #769 I saw that a significant
amount of time was spent benchmarking record literals. When I looked
at the code more closely I saw that the first key in the record literal
was being type-checked twice (once to figure out the record's associated
type-checking constant and once as part of the `process` loop).
This change fixes that, which speeds up interpretation of the large
example by 9%:
Before:
```
time 18.13 s (18.11 s .. 18.16 s)
1.000 R² (1.000 R² .. 1.000 R²)
mean 18.09 s (18.07 s .. 18.11 s)
std dev 21.92 ms (10.66 ms .. 29.76 ms)
variance introduced by outliers: 19% (moderately inflated)
```
After:
```
time 16.53 s (16.49 s .. 16.60 s)
1.000 R² (1.000 R² .. 1.000 R²)
mean 16.59 s (16.56 s .. 16.64 s)
std dev 43.65 ms (6.227 ms .. 56.35 ms)
variance introduced by outliers: 19% (moderately inflated)
```
Related to: https://github.com/dhall-lang/dhall-haskell/issues/1039
We'll probably never see indices that exceed the space of an `Int` and
the interpreter would probably not be able to handle 9223372036854775807
nested binders anyway.
* Rewriting Dhall.LSP.Backend.Dhall: Implement new API
The old "backend" consisted of a random collection of ways to invoke
Dhall:
- runDhall :: FilePath -> Text -> IO (Expr Src X)
- runDhallSafe :: FilePath -> Text -> IO (Maybe (Expr Src X))
- loadDhallExprSafe :: FilePath -> Text -> IO (Maybe (Expr Src X))
The new backend exposes a slightly more though-out API. This also lays
the foundation for performance improvements in the dhall lsp server via
caching.
* Reorder code in Dhall.LSP.Backend.Dhall
* Remove unused constructor
* Rewrite and document Backend.Formatting
* Refactor Dhall.LSP.Backend.Linting
* Refactor Dhall.LSP.Backend.ToJSON
* Adapt Diagnostics backend to the new Dhall API
* Remove old Dhall backend API
* Implement caching; revamp LSP frontend
This commit implements caching of Dhall expressions: we only need to
fetch, typecheck and normalise each import once per session, unless they
change! This means that `dhall-lsp-server` is now viable for non-trivial
Dhall projects, for example probing around in `dhall-nethack` everything
feels near-instantaneous once the imports have been resolved.
This implementation currently has a bug: we don't invalidate imports
transitively, i.e. if A.dhall loads B.dhall and B.dhall changes we do
not discard the cached version of A.dhall. This should be reasonably
easy to fix given some time with Dhall's import graph. Furthermore,
there is some cleaning up left to do:
- Fix warnings
- Reorganise things in a less ad-hoc way
- Make the code a bit prettier
* Fix caching of errors
* Use `bimap` instead of `first` and `second`
* Re-export `Dhall.lint` rather than aliasing
Rids us of some boilderplate
* Use MVar instead of TVar for server state
The main benefit is that we get to use `modifyMVar_` which does updating
of the shared state for us (and gracefully handles any uncaught
exceptions).
* Don't invalidate hashed imports
Fixes a misinterpretation on my end of the correct behaviour regarding
the caching of imports. Quoting @Gabriel439:
> A hashed import is valid indefinitely once it is successfully
> resolved, even when the underlying import later becomes broken. That's
> why missing sha256:… works so long as the cache has that import cached
> (and this behavior is part of the standard).
* Cleanup Dhall.LSP.Backend.Dhall a little bit
* Add note about fixing cache invalidation
* Use TemplateHaskell to generate state lenses
* Make types of `typeAt` and `annotateLet` more expressive
Both assume the input to be well-typed; by using `WellTyped` rather than
`Expr Src X` as the type of their input we can make this explicit.
This change exposed a bug (also fixed in this commit) in the
type-on-hover functionality: we run `typeAt` only if the input was
well-typed _the last time we checked it_ (which was at the last save);
this means that if the code changed without being written to disk we
would happily try to normalise (in `typeAt`) non-well-typed code...
* Fix type of typecheck
Typecheck returned the well-typed _type_ of a given expression, while I
was assuming it would certify the input to be well-typed. Silly indeed.
* Remove `checkDhall` from Dhall.Backend.Diagnostics
Removes the left-over stub from the change to the new Dhall backend.
* Update comments and remove TODO note
* Remove superfluous parentheses
* Simplify MonadState code via lens combinators
* Use `guard` instead of matching on True
* Remove more superfluous parentheses
Since we only want to ensure that the benchmarks continue to
work, we use the following benchmark arguments to run them
as quickly as possible:
- `--quick` ensures that gauge runs only a single sample.
- `--min-duration=0` sets the number of iterations per sample to 1.
- `--include-first-iter` causes gauge to accept that one iteration
instead of discarding it and running a second one.
This also removes the dhall-command benchmark:
This was a non-standard benchmark that failed when run
without input from stdin. To replace this profiling tool,
I have added instructions for profiling the main executables
to the README.
This reduces the runtime of the `deep-nested-large-record` benchmark by about 50%.
Note that previously, contrary to its name and documentation, this function traversed a Dhall.Map in insertion order due to its use of Dhall.Map.toList! With this change, the traversal is changed to ascending key order.
Also:
- Fix the deep-nested-large-record benchmark
- Remove the map-operations benchmark: This benchmark actually reports a ~20% loss of performance for the unorderedTraverseWithKey_ change. Since we eventually care about dhall's performance we it's better not to be mislead by a questionable micro-benchmark.
This separates the source from the line numbers using vertical bars
instead of colons. The main reason for this is to more clearly
delimit the two and to also prevent the old colon from being confused
as a type annotation.
Fixes https://github.com/dhall-lang/dhall-haskell/issues/1025
There were two error constructors related to invalid field types:
* `InvalidField`: for field types whose types were invalid type-checking
constants
* `InvalidFieldType`: for all other invalid field types
As @sjakobi noted, there are no invalid field types for the former
category, so we can remove that category of error entirely.
Note that `InvalidField` was still being used in a few places, but each
of those uses should actually have been using `InvalidFieldType`
instead, which this change also fixes.
This improves the error message when users attempt to apply `merge` to only
one argument, as suggested by:
https://github.com/dhall-lang/dhall-lang/issues/106#issuecomment-503523358
For example:
```
$ dhall <<< 'merge { Foo = True }'
Error: Invalid input
(stdin):2:1:
|
2 | <empty line>
| ^
unexpected end of input
expecting '.', second argument to ❰merge❱, or whitespace
```
After #989 and #993 the use of the `yaml` package is isolated in the lib modules `Dhall.Yaml` and `Dhall.YamlToDhall` so it is easier to add support for compilers that cant use the `yaml`package like eta or ghcjs.
With this one we would add support for [eta](https://eta-lang.org/), a fork of ghc that compiles to jvm bytecode.
Main changes:
* Add conditional to cabal file and cpp conditions to the main modules for yaml to use a specific module for eta, `Dhall.Yaml.Eta` that replaces calls to the `yaml`package to ffi calls to the java lib [jackson](https://github.com/FasterXML/jackson), one of the most popular ones in the java world.
* Add the java files that contains the ffi calls
* Mark `buildable: False` the subpackages that cant be built by eta for now
One effect of use the cabal file for cabal and etlas (the build tool for eta) is stack and cabal builds show those warnings:
```
Warning: Cabal file warning in D:\ws\eta\dhall\dhall-haskell\dhall-json\dhall-js
on.cabal@ 69:9:
Unknown field: "maven-depends"
Warning: Cabal file warning in D:\ws\eta\dhall\dhall-haskell\dhall-json\dhall-js
on.cabal@ 74:9:
Unknown field: "java-sources"
```
I've not found a way to avoid them other than use another file for etlas build (`etlas.dhall`). It would suppose duplicate most of the logic, though.
* Type on Hover 1/2: Backend
Adds a function typeAt that, given an annotated expression and a text
position finds the type of the subexpression at that position.
* Type on Hover 2/2: Frontend
Exposes the new type-on-hover functionality as part of the hover
handler.
* Simplify explainDiagnosis
As suggested by @Gabriel439
* Simplify `inside`
Adressing @Gabriel439's comment.
* Simplify typeAt' by assuming well-typedness
* Simply srcAt
Use choice operator `<|>` instead of case distinction
`dhall diff` slightly misbehaves when diffing the following expression with
itself:
```dhall
λ(f : List Bool -> Bool) → f ([] : List Bool)
```
... producing the following diff:
```
λ(… : …
→ …)
→ …@…
- [ … ] : List …
+ [ … ] : List …
```
This is because there are two places in the `Dhall.Diff` module responsible
for comparing lists:
* Once in `diffApplicationExpression`, which compares two lists with at least
one type annotation between them
* Once in `diffPrimitiveExpression`, which compares two lists if neither one has
a type annotation
Those cases exhaustively cover all possible pairs of lists, but there was a
third (incorrect) fallback case that prematurely gave up and displayed them as
different. This fallback would trigger when applying a function to an empty
list, since the diffing algorithm wouldn't have a chance to return back to the
top-level `diffExpression` and try to compare the lists correctly in
`diffApplicationExpression`.
After this change, the diff now doesn't include a spurious difference:
```
λ(… : …
→ …)
→ …@…
…
```
The `--file` option was essentially broken since it passed in the
file path instead of the directory, meaning that transitive imports
would not be computed correctly (they'd be off by one path component).
At the moment the VSCode plugin contains a hacked-together prototype of
a dhall-to-json (and -to-yaml) preview feature. We should ultimately
move that kind of functionality to dhall-lsp-server to 1) minimise the
amount of VSCode specific code and 2) avoid having to work with and fix
bugs in typescript code ;)
This commit takes us most of the way there; what is missing is the
ability to create files (via a WorkspaceEditRequest), which is not yet
implemented by haskell-lsp-types. I will wait with exposing the
corresponding command (and removing the existing preview code) in
vscode-dhall-lsp-server until this is fixed upstream.
* Find unused bindings inside nested lets
The removeUnusedBindings rule only matches the first bound variable in a
nested let block (of the form "let ... let ... in ..."). This means that
so far the linter missed cases like
let a = 0 let b = 0 in a.
This simple fix unfolds all let blocks (syntactically this means
inserting `in's everywhere) before applying the linting rules; the
LetInLet rule folds everything back together in the end. Applied to the
example from above we now return the correct result,
let a = 0 in a.
* Rewrite `lint` in a more explicit style; add comments
* Don't export implementation details
* Fix `freeIn` to correctly handle de Briujn indices
Previously we had "x@0" `freeIn` "x@1" == True (but "x@1" `freeIn` "x@0"
== False). The fix is to use subst instead of shift to change
occurrences of the given variable.
* Fix `removeUnusedBindings` to update de Bruijn indices
Whenever we remove an unused binding we need to update any references to
variables outside the let block, so that
let a = 0 let a = 0 in a@1
gets correctly rewritten into
let a = 0 in a (a = a@0)
instead of
let a = 0 let a = 0 in a@1
~> let a = 0 in a@1
~> a@1
* Refactor Dhall.LSP.Backend.Formatting
Exposes `formatExpr :: Text -> Expr a Import -> Text` that can be reused
in the lintAndFormat command.
* Implement linting backend in Dhall.LSP.Backend.Linting
Exposes `suggest :: Eq a => Expr Src a -> [Suggestion]` and
`lintAndFormatDocument :: Text -> Either ParseError Text`.
* Fix unusedBindings
* Implement linter diagnostics
* Improve linting diagnostic ranges
In VSCode we now mark the "let" following a superfluous "in".
let a = 0 in let b = a in b
~
Previously we got
let a = 0 in let b = a in b
~
which was slightly confusing
* Implement executeCommand handler for "Lint and Format"
This exposes the command "dhall.server.lint", which should be called
from the vscode plugin to lint and format the current dhall file. Needs
a correspondingly patched vscode-dhall-lsp-server (branch lint).
