rnix-parser icon indicating copy to clipboard operation
rnix-parser copied to clipboard

Broken paths are silently skipped

Open tazjin opened this issue 3 years ago • 4 comments

Certain path literals are parse errors, most prominently paths with a trailing slash. Compare C++ Nix and Tvix (with rnix-parser):

# C++ Nix
nix-repl> ../../
error: path '../../' has a trailing slash

# rnix-parser (through tvix)
tvix-repl> ../../
error[E015]: failed to parse Nix code:
 --> [tvix-repl]:1:1
  |
1 | ../../
  | ^^^^^^ code ended unexpectedly while the parser still expected more

These are correctly turned into errors in both cases, however, rnix-parser seems to do something strange if the broken path occurs somewhere in the middle of an AST:

# rnix-parser, via tvix, with `--display-ast`
tvix-repl> import ../../
Ident(Ident([email protected]))
=> <<primop>> :: lambda

tvix-repl> [ 1 2 ../../ 3 ]
List(List([email protected]))
=> [ 1 2 3 ] :: list

Compare with C++ Nix:

nix-repl> import ../../
error: path '../../' has a trailing slash

nix-repl> [ 1 2 ../../ 3 ] 
error: path '../../' has a trailing slash

I haven't manually investigated the actual structure produced by rnix-parser in this case yet; it must be in there somewhere (side note: is there a built-in way for some more pretty-printed representation of the AST, either in the crate (that I've missed) or that someone wrote?)

tazjin avatar Oct 08 '22 23:10 tazjin

You can use {:#?} to get a more detailed pretty-printed representation

oberblastmeister avatar Oct 13 '22 14:10 oberblastmeister

The ast is

Root(
    [email protected]
      [email protected]
        [email protected] "import"
      [email protected] " "
      [email protected] "../../"
      [email protected] "\n"
    ,
)

This is probably due to https://github.com/nix-community/rnix-parser/blob/f8056c4811b6f5196af64db14b89651e892ad98a/src/kinds.rs#L133 We just bump error tokens as trivia so they appear in the tree, but don't report them as errors.

oberblastmeister avatar Oct 13 '22 16:10 oberblastmeister

We just bump error tokens as trivia so they appear in the tree, but don't report them as errors.

Hmm, not sure I understand the implication of this. We'd have to do a pass over the entire tree and detect any error nodes to get "all" errors? Are there any things other than broken paths that exhibit this behaviour?

Right now in the Tvix compiler, we have an expectation that the root expression is "sane" when passing it over as long as rnix didn't report any errors.

tazjin avatar Oct 13 '22 21:10 tazjin

Yeah we should probably be reporting these errors.

oberblastmeister avatar Oct 13 '22 21:10 oberblastmeister