Nixf-tidy: static/semantic linter for nix, help me to integrate it with Github Actions (PR Reviews?)

[inclyc]

I just wrote a executable for linting nix programs (intended to be consumed in high-level “UI”)

It can produce structured outputs (JSON) of diagnostics, including locations, fixes, kinds.

libnixf: introduce `nixf-tidy`, perform analysis, lints and emit json

echo '{ a, ..., ...} : 1' | build/libnixf/nixf-tidy --pretty-print --variable-lookup

[
    {
        "args": [],
        "fixes": [
            {
                "edits": [
                    {
                        "newText": "",
                        "range": {
                            "lCur": {
                                "column": 3,
                                "line": 0,
                                "offset": 3
                            },
                            "rCur": {
                                "column": 8,
                                "line": 0,
                                "offset": 8
                            }
                        }
                    }
                ],
                "message": "remove `...`"
            }
        ],
        "kind": 22,
        "message": "extra `...` for lambda formal",
        "notes": [],
        "range": {
            "lCur": {
                "column": 3,
                "line": 0,
                "offset": 3
            },
            "rCur": {
                "column": 8,
                "line": 0,
                "offset": 8
            }
        },
        "severity": 1,
        "sname": "sema-formal-extra-ellipsis",
        "tag": []
    },
    {
        "args": [
            "a"
        ],
        "fixes": [],
        "kind": 27,
        "message": "definition `{}` is not used",
        "notes": [],
        "range": {
            "lCur": {
                "column": 2,
                "line": 0,
                "offset": 2
            },
            "rCur": {
                "column": 3,
                "line": 0,
                "offset": 3
            }
        },
        "severity": 2,
        "sname": "sema-def-not-used",
        "tag": [
            0
        ]
    }
]

I’d like to integrate this somehow into GitHub CI/CD (for example producing PR reviews based on it). Anyone wants to take this ?

You can test the command using:

nix build github:nix-community/nixd?rev=55b9fd28ac1f82345783af1006565cee768c8dd5#nixd

For example

❯ echo 'rec {  }' | result/bin/nixf-tidy --variable-lookup --pretty-print

[
    {
        "args": [],
        "fixes": [
            {
                "edits": [
                    {
                        "newText": "",
                        "range": {
                            "lCur": {
                                "column": 0,
                                "line": 0,
                                "offset": 0
                            },
                            "rCur": {
                                "column": 3,
                                "line": 0,
                                "offset": 3
                            }
                        }
                    }
                ],
                "message": "remove `rec` keyword"
            }
        ],
        "kind": 28,
        "message": "attrset is not necessary to be `rec`ursive ",
        "notes": [],
        "range": {
            "lCur": {
                "column": 0,
                "line": 0,
                "offset": 0
            },
            "rCur": {
                "column": 3,
                "line": 0,
                "offset": 3
            }
        },
        "severity": 2,
        "sname": "sema-extra-rec",
        "tag": [
            0
        ]
    }
]

Also I would like to write some patterns commonly used in Nixpkgs. (e.g. lib, stdenv occurrences), any suggestion?

[Pamplemousse]

There is already an approved RFC to cover this topic: [RFC 0166] Nix formatting, take two by piegamesde · Pull Request #166 · NixOS/rfcs · GitHub .

You could contribute to the group effort:

• by contacting the formatting team
• or help with implementation RFC 166 implementation tracking issue · Issue #153 · NixOS/nixfmt · GitHub

[inclyc]

Thanks for your information :). After reading a bunch of comments I think they were proposing an RFC for formatter not linter right?

[piegames]

Indeed, the formatting RFC has little to nothing to do with linting except that we need both. I did a topic on linting, it contains a good collection of already existing linting tools. In the end, it’s not the specific tool which matters, but the need for actually integrating this into Nixpkgs.

We need Nixpkgs linting

The amount of person-hours we currently waste on manually reviewing things which could be automated is not okay. The advantages of automated code quality checking: It reduces the amount of back and forth between author and reviewer, because a lot of fixes can be done locally already. This reduces stress and is faster due to having a lower round trip latency. It reduces the amount of nitpicky review comments, increasing the signal-to-noise ratio for actually important review comments. Higher qu…

[Artturin]

What does it lint? There’s GitHub - nix-community/nixpkgs-lint: A fast semantic linter for Nix using tree-sitter 🌳 + ❄️. [maintainers=@Artturin,@siraben] which is more flexible due to tree-sitter

[Pamplemousse]

Correct.

Linters very often embed stylistic rules so I tend to conflate the two. And the examples given in the OP seemed to be stylistic…

[inclyc]

which is more flexible due to tree-sitter

Yes I admit that tree-sitter has more intuitive grammar (and yet much more user-friendly than C++ codes).

Previously I choose tree-sitter but now it is based on handwritten recursive descent parser. Focusing on error recovery, and “native” fix-it hints. There are challenges to write linters by tree-sitter CST:

1. It is “too” raw. Tree-sitter only produces raw CST (directly from the context-free-grammar rules), but not do any semantic actions. This is important for “checking duplication” in the attrset. For example

{
  # This is correct
  x.a = 1;
  x.b = 2;
}

{
  # This is wrong!
  x.a = { c = 2;  } ;
  x.a = { c = 1; };
}

{
  x.a = { c = { a = 1; }; };
  #             ~~~~~~<-------- This attrset (c = ) will not be merged!
  x.a = { c = { b = 2; }; };
  #       ^<------------------- Duplicated attr!
}

Semantic merging attrsets with different rec modifier!

This is a set of complex transforming rules, which cannot be "query"ied easily on tree-sitter CST. nixd/libnixf/src/Sema/SemaActions.cpp at dc0164e0df37e5d06bda61b6fb2ca16089d5f838 · nix-community/nixd · GitHub

2. Because of (1), it’s hard to perform variable liveness analysis based on tree-sitter produced CST, in nix language, variable lookups are performed in “merged” attrsets.

3. As it is CST produced by generated parser, thus may not suitable for better diagnostics and fixes.

{
   # in context-free grammars, "1" will be considered as a function call
  # in libnixf, this will not be parsed as (1 b) + fail, = , 2, but semantically parsed as "1" only
  a = 1 # Diagnostic about missing ";"
  b = 2 # Diagnostic about missing ";"
}

[inclyc]

@piegames

I think that a linter for Nixpkgs should meet the following requirements:

• Being able to run locally, with fast iteration times
• Provide good error messages and feedback about what should be done
• A powerful architecture that allows to express various kinds of rules. Static analysis is a must, evaluation-based linting would be a welcome addition
• Commit message linting
• Easily extensible; People should be able to add new lints for things that come up frequently
• Granular configuration, especially for the transition phase
• Tooling integration, e.g. with GitHub warnings or IDE extensions

libnixf already addresses some points here:

Being able to run locally, with fast iteration times

Yes.

Provide good error messages and feedback about what should be done

Yes. Fixes are available even for missing */ for closing comments.

A powerful architecture that allows to express various kinds of rules.

Not so much, currently all static lintings are written in C++.

Static analysis is a must, evaluation-based linting would be a welcome addition

libnixf could transform it’s AST to nix::Expr * expression, thus it can evaluate the code.

Commit message linting

Not implemented, but I suppose it should based on eval results / AST-diff ?

Easily extensible; People should be able to add new lints for things that come up frequently

Not so much. Please write rules / feature requests and I would like to impl it, in C++.

Granular configuration, especially for the transition phase

Just run nixf-tidy on git-diff only.

Tooling integration, e.g. with GitHub warnings or IDE extensions

Yes. libnixf is the frontend of nixd. That is, rules implemented in libnixf works out of box for nixd users. As a language server integrated to your editors (e.g. vscode/nvim/…) .