* Fix LSP not showing hover information
After the bidirectional refactoring (better separating the rules between
`infer` and `check`), the LSP was experiencing regressions, not showing
hover information in some cases, in particular within statically typed
blocks.
It turns out the separation between `infer` and `check` makes less clear
which function is responsible for adding the term to the linearizer:
sometimes it's `check`, sometimes it's `infer`, and it really depends on
the call graph and where each function is called from.
It turns out the previous implementation was adding many terms in
double, and this caused the initial regression observed.
Morever, fixing is made hard by the fact that `infer` now doesn't have a
type to provide right now for the linearizer: it first need to, well,
infer it. But the linearizer currently relies on AST nodes being visited
in order, so we can't either call to it whenever we want.
The situation seemed intricate enough that it was easier to separate
"visiting the term" from "amending the type of the term". This commit
thus adds a `retype` method to the `Linearizer` trait, plus some
id-related signature changes. Now, the typechecking code can add a term
to the linearizer as soon as it visits it, and set the type later. More
discipline has been introduced so that `check` and `infer` don't repeat
the same term visits anymore.
Doing so, this commit also cleans a bit the LSP implementation which was
very hard to change getting it right: `IdGen` in particular is gone
(which was maintaining a counter by hand in parallel to the
linearization's size, which was very error-prone, and if any mismatch
occurred, anything could happen). The truth is, the refactor should be
much more involved to make the code maintainable, but this commit is
just a fix for the 1.2.0 release to get out, and the linearizer is a
legacy component that will be scraped soon when the AST-based LSP will
reach feature parity with the linearizer.
* Add Hover capability to tests
* Add regression test for hover after bidir pull request
* Formatting and code clean-up
* Apply suggestions from code review
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* Make retype calls less verbose
* Fix documentation dangling link
---------
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* cache most of the work even if rev changes
* longer string for more reliable search.
* more recognizable dummyRev
* nickel-lang-static needs env
* appease nixpkgs-fmt
* copy meta to new derivation
* add comment
* Fix a typo in the release workflow
This changes `os.matrix` into the correct `matrix.os`.
* Use `nodeRuntimes` as required by nixpkgs master, now
* Add nix settings to github-runner.nix
* Disable diffutils tests on arm64 musl
* Add dependencies on `start-runner` in `release-artifacts.yaml`
* Use `--log-format raw-with-logs` like in the CI workflow
* Use the correct cargo package for `nickel-static`
* Adjust linker flags on arm64 musl
* Add github cli and docker to `github-runner.nix`
* Combine static binary and docker image building jobs for caching
* Allow docker access for github jobs
* Use `docker buildx` to assemble the multiplatform image
Allows to `nix run github:tweag/nickel#...` to work as expected
with all packages as well as `pkgs.lib.getExe` returning proper paths.
Also switch to `getExe` with nickel packages in flake.nix
to demonstrate how nice it looks.
* display icon for nickel file in vscode explorer
* implemented with DynAccess.
* added tests.
* Update grammar.lalrpop
move implementation inside CurriedOp
---------
Co-authored-by: Ben Yang <ben@ya.ng>
We previously just reported the error and then pretended to exit
successfully. With this change the process returns `ExitCode::FAILURE`
whenever it needed to report an error, and `ExitCode::SUCCESS`
otherwise.
Fixes#1575
* Add infrastructure for on-demand ARM64 runners on AWS
With this change, ARM64 release artifacts will be built automatically by
a GitHub workflow. Since GitHub doesn't offer hosted runners running on
ARM64, we're spinning up an EC2 spot instance on demand and run the jobs
building ARM64 artifacts there.
As a fun side note, the Terraform infrastructure code is written
entirely in Nickel.
* Remove unused `update-github` script
* Address comments from code review
* Address comments from code review
* Revert "[Fix] Fix bad lexing of mutline string closing delimiter (#1435)"
This reverts commit 664c397aad.
After more investigation, it turns out the original change made it very
annoying to interpolate something between double quotes, because it
conflicts with the multiline string end delimiter:
m%" "%{hello}" "%
After the reverted change, this wouldn't parse correctly anymore. One
can escape the first quote, but it's verbose and harder to read:
m%" %{"\""}%{hello}" "%
Although, we can't see any case where an interpolation sequence with a
heading quotes would have a valid meaning in the reverted commit's
model: it's always an error (applying a string to something). Thus, if
there's a sequence `"%{` in a string, there is no other meaningful
interpretation than this is a quote followed by an interpolation
sequence.
We thus decided to revert this change, although it derives a bit from
the most simple interpretation of parsing multiline string.
* Post-revert fixup: restore some comments and renamings
* Document multiline string parsing special case
* Update core/src/parser/lexer.rs
Co-authored-by: jneem <joeneeman@gmail.com>
* Fix interpolation sequence in pretty printer
---------
Co-authored-by: jneem <joeneeman@gmail.com>
* Render documentation as markdown in LSP
* Use unindent crate to retain code indentation
* Render contracts seperate from documentation
* Remove usages of unindent
* Remove unindent from `Cargo.lock`
---------
Co-authored-by: BuildTools <DeoTimeTheGithubUser@users.noreply.github.com>
Upon the failure of a higher-order contracts, we used to include quite
generic notes showing a plain illustration of the kind of higher-order
violation that happened. However, this text was totally generic (the
example didn't relate to the source code at all), quite verbose (around
4-5 lines of notes), and to be honest, not very helpful.
We now have good mechanisms to underline the right part of the function
contract that failed, together with customized label (e.g. "broken by the
caller"), which makes look like the original generic note aren't very
valuable.
This patch ditches those notes altogether.
* Import completion
* Use `cache::normalize_path` instead of `fs::canonicalize`
* Add cached files as candidates for import autocompletion
* Rename `InputFormat::from_path_buf` to `InputFormat::from_path`
* Deduplication between server cached files and disk files
---------
Co-authored-by: deotime <89555032+DeoTimeTheGithubUser@users.noreply.github.com>
In an attempt to free the allocation of the constraints of a record row
unification variable that is being assigned to some record rows, the
code checking that row constraints are satisfied was removing the
constraints from the global state. Since rows are defined as linked
list, if the constraint wasn't violated on the first row, this function
would recursively call itself. But then the subsequent recursive calls
would try to get the constraints from the state again, to only find an
empty set constraints, as it has been removed just before. The function
thus wouldn't detect constraint violations happening in the tail of the
record rows.
This patch defines a subfunction which passes the initial constraints
along recursive calls, such that they aren't lost during recursion,
instead of trying to get them from the state again at each recursive
call.
* set arguments with --
Create term that merges command line arguments with program
* Move building the override term into `Program`
* instead of immediately evaluating argument values, use
`ImportResolver::add_string` to make them separate imports.
* Move CLI export into its own file
* Integrate record builder interface
* Make record field paths work correctly
* Evaluate annotations, too, for record normal form
* Make term interfaces a first class concept
* Stop printing terms on export
* Rename eval_for_doc to be less doc-specific
And expand the documentation at the same time.
* Augment FieldInterface with more data
Augment the field interface type - for freeform argument passing in the
CLI - with additional metadata, such as being optional or defined, taken
directly from the original field's metadata. Add a somehow heuristic
(but simple) `is_input` method, which decides if a field is an input and
should have a dedicated argument.
Minor reworking/renaming of other part of the CLI export code around
freeform arguments.
* Restore the path argument, add more documentation
* Use Field/FieldMetadata instead of custom types
In the CLI export function, instead of using a structure that mostly
mirror record fields, we directly use record fields to avoid code
duplication, although fields do carry a bit more information that what
we exactly need.
* Use query printer to render freeform args help
* Continue proper handling of overrides/inputs in CLI
* Fix eval_record_spine, add sources for CLI arguments
* Remove debug println, improve CLI freeform output
* Move around reporting code to error module
* Actually implement --override, add custom error messages
* Make overrides have a `force` priority
* Add some untested tests
* Small rewording of Ident documentation
* Post-rebase fixups + get rid of Merge trait
* Fix snapshot test runner for freeform args
* Fix CLI freeform snapshot tests
* Fix unused import warning
* Formatting
* Review pass, smallfixes and clippy fix
* Fix cargo doc warnings
* Fix 1.72 new clippy warnings
* Update snapshots after extra-args -> customize mode
* Move the Combine trait in its own mod
* Apply suggestions from code review
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* Update core/src/term/make/builder.rs
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* Remove questionable Option<> layer
* Update cli/src/export.rs
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* Update cli/src/export.rs
Co-authored-by: jneem <joeneeman@gmail.com>
* Apply suggestions from code review
Co-authored-by: jneem <joeneeman@gmail.com>
* Add warning about usage of Label::default
---------
Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
Co-authored-by: jneem <joeneeman@gmail.com>
* Add multiline string support for VSCode autoclosing pairs
* Support up to 4 % symbols
---------
Co-authored-by: BuildTools <DeoTimeTheGithubUser@users.noreply.github.com>
This fixes an off-by-one error in the handling of incomplete input.
It also makes import resolution in incomplete input more reliable --
previously it would only work when the file being opened is in the
current working directory.
* Fix some leftover instances of the old enum syntax in the manual
* Update doc/manual/syntax.md
Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
---------
Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
* Fix contract application order in let bindings and annotations
In a let binding `let foo | A | B = ...` and in an infix contract
annotation `foo | A | B` the contracts `A` and `B` were actually being
applied in reverse order. That is, first `B`, then `A`. The contract
application order for record fields was still correct, however.
The root cause was the fact that there were two places with code that
merged separate annotations into a single annotation containing multiple
contracts. With this change, both spots call into the same code and I've
added regression tests to ensure the correct behaviour.
* `Annot` -> `Combine` in comments
* Add a snapshot test for let-binding contract order
* Move `Type` pretty printer test into `core/src/pretty.rs`
* Revamp pretty printer for `TypeF::Array`
* Start overhauling and testing the Type pretty printer
* Simplify pretty printer integration test
* Expand pretty printer integration tests to include stdlib
* continue overhauling the pretty printer
* Add tests for "let" pretty printing
* Ignore empty annotations in TypeAnnotation::attach_term
* Get almost all pretty printer tests to pass, missing `pass/strings/symbolic_strings.ncl`
* Use `Term::StrChunks` when desugaring symbolic strings
* Fix missing newlines in multiline strings
* Handle record patterns in let bindings
* `fun` pretty printing
* Update snapshot tests
* Fix pretty-printer dependent test cases
* Remove TODO comment
* Improve formatting for record fields with metadata
* Remove a redundant clone and some redundant calls to `pretty`
* Consistently use `%<stuff>` for internal values
* Address comments from code review
* Derive `Eq` for `OpPos`
* format other binary operators the same way as `&&` and `||`
* Add a comment about special unary operators
* Adjust grouping for field metadata annotations
* Simplify `Array` pretty printing code
* Document the lack of `indoc!` in `pretty_multiline_strings`
* WIP
* Working extraction and reparsing
* werks
* Add a test
* Remove unused linearization param
* Cleanups and docs
* Allow quoted strings to count as a single token
* Add some docs about the initial env
* WIP
WIP
WIP
* Working unit tests checkpoint
* Pass some tests
* A slight refactor and better docs
* Update core/src/destructuring.rs
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* Update lsp/nls/src/linearization/mod.rs
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* Update lsp/nls/src/position.rs
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* Also track bindings in records
* Fix warnings
---------
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* Simplify parsing of record destructuring
* Capture pattern destructuring errors as snapshot tests
* Record the destructuring contract in a label
This makes error reporting point at the destructuring instead of at a
fictitious `Dyn`.
* Make Symbol available.
Ident is a symbol plus a position. The position isn't intended to carry
any semantics -- it's really just for diagnostics -- so it's ignored in
comparison and hashing. This is mostly useful, but sometimes
(particularly in the LSP) we do want meaningful positions. Also, a lot
of code that completely doesn't care about the positions were carrying
one around "by accident."
This PR makes Symbol publicly available, and updates many usages of
Ident to use Symbol instead. There are probably more places that
Symbol could be used.
We do not yet introduce a Symbol + meaningful position type. That will
be done in a future PR, and probably restricted to the LSP.
* two more conversions
* Update core/src/eval/operation.rs
Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
* The big rename
---------
Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
* Use the original unevaluated type and contract annotation in `nickel doc`
With this change we no longer print evaluated types or contracts in
field annotations for `nickel doc`. Ever since `nickel doc` started
evaluating terms to address #1462 contract and type annotations would be
evaluated before reaching the documentation extraction stage. This means
they would be affected by program transformations and the result would
most likely be meaningless to the user.
Incidentally, `nickel query` already correctly used the original
unevaluated type for contract annotations but not for type annotations.
If a type annotation contains a `Term` as a `TypeF::Flat` variant, it
was possible to trigger the same undesirable behaviour:
```
❯ cargo run --bin nickel -- query foo <<<'{ foo : { x | Dyn, y } = {x = 1, y = 2} | { x | Dyn, y } }'
Finished dev [unoptimized + debuginfo] target(s) in 0.12s
Running `target/debug/nickel query foo`
• type: let %182 = $dyn in { x | Dyn, y, }
Available fields
• x
• y
```
Fixes#1519
* Add a snapshot test against regressions
* Introduce nickel_lang_core::format and use it in the CLI
* Use nickel_lang_core::format for formatting in the LSP
* Update lsp/nls/README.md
* Update lsp/nls/README.md
Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
* Remove an unused import
* Remove the format feature flag for NLS
---------
Co-authored-by: Yann Hamdaoui <yann.hamdaoui@tweag.io>
* Use the pretty printer in the `Display` implementation for `Type`
We previously used a bespoke formatting algorithm for `Type`. I replaced
the analogous code for `Term`s by the pretty printer in #1262 but we
were worried about some questionable code for contract error reporting
before doing the same for types. Namely, at some point it relied on hard
coded string offsets for pointing at parts of types that were inferred
by Nickel and consequently had no `TermPos`. In #1229 we ripped out
that code and replaced it by reparsing the pretty printer output when
necessary.
Incidentally, this change also fixes some terms being truncated when
formatted. For example, previously
```
SomeUserDefinedContract "that" "takes" "many" "arguments"
```
would be printed as `SomeUserDefinedContract "that" …`. This is somewhat
useful to prevent huge screenfuls of error messages sometimes, but it
makes the `Display` implementation useless for other natural purposes.
* Revert a useless change to snapshot test inputs
* Factor out calling the pretty printer for `Display`
* Add warning for empty query path on the CLI
* Update cli/src/error.rs
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
* Fix formatting after committing reviewer's suggestion
---------
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>