oppiliappan / statix

lints and suggestions for the nix programming language
https://git.peppe.rs/languages/statix/about
MIT License
557 stars 21 forks source link

Multiple levels of eta-reduction as a single suggestion #6

Open danth opened 2 years ago

danth commented 2 years ago

I came across this case while testing #5:

[W07] Warning: This function expression is eta reducible
    ╭─[/tmp/test.nix:10:16]
    │
 10 │   test7 = arg: otherArg: function arg otherArg;
    ·                ───────────────┬───────────────  
    ·                               ╰───────────────── Found eta-reduction: function arg
────╯

After applying the suggestion above, I get a new suggestion to eta-reduce test7 further:

[W07] Warning: This function expression is eta reducible
    ╭─[/tmp/test.nix:10:11]
    │
 11 │   test7 = arg: function arg;
    ·           ────────┬────────  
    ·                   ╰────────── Found eta-reduction: function
────╯

Ideally, Statix would produce a single suggestion like:

[W07] Warning: This function expression is eta reducible
    ╭─[/tmp/test.nix:10:11]
    │
 10 │   test7 = arg: otherArg: function arg otherArg;
    ·           ────────────────────┬───────────────  
    ·                               ╰───────────────── Found eta-reduction: function
────╯
oppiliappan commented 2 years ago

Hmm, this is a tall ask :sweat_smile:. I think it partly requires rethinking how the whole lint framework behaves altogether. Currently lints are expected to be as "local" as possible. When the lint runner approaches the node beginning at otherArg it has to somehow know that otherArg: function arg otherArg is not supposed to be linted because it was already linted as a part of arg: otherArg: function arg otherArg. This partly helps fixes also to be "local" and easy to reason about. At the moment, I can't quite think of a way to implement this.

statix fix is able to fix the this issue in one blow however (a single run, but multiple passes over the source).

danth commented 2 years ago

Further to that, we can't simply skip linting of otherArg: function arg otherArg as it could be a more complex expression containing other suggestions.