Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discover all .nix files in nil diagnostics #127

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

9999years
Copy link

Follow-up to #125.

nil diagnostics now accepts directories in addition to files as arguments. If no arguments are given, the current directory (.) is used.

The ignore crate is used to traverse the directories and discover files. This is slightly slower than find . -path '*.nix' -exec nil diagnostics {} + because it inspects .gitignore files and performs glob matching: in my tests, this branch ran nil diagnostics in nixpkgs in 6.75 seconds, compared to 6.35 seconds for the find-based solution.

@oxalica oxalica added C-feature Catagory: feature A-CLI Area: command line interface labels Aug 7, 2024
@oxalica
Copy link
Owner

oxalica commented Aug 7, 2024

I don't like to do this kind of trivial traversal ourselves, given many tools like fd already satisfies this goal. We could extend diagnostics subcommand to accept multiple paths so it would be faster with fd --exec-batch (as in #126).

If it's possible, I would prefer discover all nix files using path reference traversal which is already in the code but not enabled yet. Though it may take more time to be fully implemented.

@oxalica oxalica added the A-diagnostics Area: diagnostics / error reporting label Aug 7, 2024
@9999years
Copy link
Author

We could extend diagnostics subcommand to accept multiple paths so it would be faster with fd --exec-batch

It would still be limited by command-line argument size limits, and it would still be a lot clumsier to use than just traversing the directories natively. There's a reason (e.g.) cargo clippy doesn't make you list every Rust file in your project.

If it's possible, I would prefer discover all nix files using path reference traversal which is already in the code but not enabled yet. Though it may take more time to be fully implemented.

I'm not confident this is feasible or desirable. Paths can be constructed programmatically, so it's not easy to get a list of imported paths with static analysis. Functions like builtins.readDir (and nixpkgs' lib.packagesFromDirectoryRecursive) further complicate matters.

It's also fairly normal for users to import paths from a directory or flake manually, even if the default expressions don't invoke them. (Nixpkgs makes use of this in several places, for example.) For that reason, I'll want to lint all of the Nix code in my repo, even if it's not explicitly imported by another file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: command line interface A-diagnostics Area: diagnostics / error reporting C-feature Catagory: feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants