tweag / nickel

Better configuration for less
https://nickel-lang.org/
MIT License
2.43k stars 93 forks source link

Allow fully validating and checking formatting of nickel files without evaluation #2028

Open SRv6d opened 2 months ago

SRv6d commented 2 months ago

Is your feature request related to a problem? Please describe. There is currently no way to validate that a nickel configuration file is free of type and syntax errors without fully evaluating it. The same goes for formatting, it is not possible to only check if a file is formatted correctly, without actually performing the formatting.

Describe the solution you'd like A validate subcommand that ensures a nickel file is free of type or syntax errors, essentially an eval without the actual full evaluation and output. To check formatting, something like nickel format --check config.ncl would be great.

Describe alternatives you've considered I currently use nickel eval config.ncl > /dev/null to check for errors. Since nickel format returns the same exit code whether it did something or not, the only way I can come up with to validate formatting is to actually run nickel format config.ncl and checking if the file was changed.

jneem commented 2 months ago

There's a nickel typecheck subcommand -- does that not do what you want? I agree it would be nice to have nickel format --check.

SRv6d commented 2 months ago

I'm aware of nickel typecheck, but that doesn't catch syntax errors that are caught by an eval.

jneem commented 2 months ago

Can you give an example of the sort of error you'd like to catch but isn't caught by nickel typecheck? There will always be some errors that it doesn't catch (for example, errors caused by contract application, which are only triggered during eval).

SRv6d commented 2 months ago

As an example:

{
    example = [
      {
        foo = "bar",
      }
      {
        bar = "baz",
      },
    ],
}

nickel typecheck does not care about the missing trailing comma on line 5.

nickel eval however:

error: not a function
  ┌─ example.ncl:3:7
  │    
3 │ ╭ ╭       {
4 │ │ │         foo = "bar",
5 │ │ │       }
  │ ╰─│───────^ this term is applied, but it is not a function
6 │   │       {
7 │   │         bar = "baz",
8 │   │       },
  │   ╰───────' applied here
yannham commented 2 months ago

Well, this isn't a syntax error per se - the syntax allows to apply pretty much any value to any other value. In a fully statically typed language, that would be caught by the typechecker. This is obviously not the case for the parts that are dynamically typed in Nickel.

However, I reckon that this precise kind of error should be easy to detect statically. As typechecking already walks the AST for various purposes, maybe we should also check that the head (the left part) of an application isn't something that has no chance of ever making sense (typically any literal - record, array, number, string, boolean, etc.). Of course, once you do something more elaborate such as ({foo = 1} & {bar = 2}) {baz = 3}, all bets are off: to catch those kind of errors in all generality, you precisely need typechecking; in such a case, I would say just use a type annotation.

SRv6d commented 2 months ago

However you call it, I would like to be able to validate configuration in such a way that I know it won't cause any errors during eval/export.

jneem commented 2 months ago

If you want to catch all possible eval/export errors, I think you just have to do a full eval/export. There are evaluation-time errors (like evaluation-time contract failures) that don't get caught by any other phase.

yannham commented 1 month ago

As a side note, the strict mode proposed in #2049 could help detect those kind of errors as well (I mean, static typing in general). Otherwise, as Joe says, there's a whole class of validation errors that you just can't check statically in general. The bad application stuff is still something that might be worth checking maybe during the walk mode of the typechecker; I'm keeping this issue open for the latter to be implemented.