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

Wrong unused binding #148

Open
viperML opened this issue Aug 19, 2024 · 6 comments
Open

Wrong unused binding #148

viperML opened this issue Aug 19, 2024 · 6 comments
Labels
C-support Catagory: support questions

Comments

@viperML
Copy link
Contributor

viperML commented Aug 19, 2024

{
  hello,
  symlinkJoin,
  lib,
} @ inputs:
symlinkJoin {
  name = "foo";
  paths = builtins.filter lib.isDerivation (builtins.attrValues inputs);
}

nil reports an unused_binding on hello, while it is not the case.

@NobbZ
Copy link

NobbZ commented Aug 19, 2024

hello is unused. The fact that inputs.hello does not exist without the explicit hello in the argset due to being callPackageed doesn't change that.

@viperML
Copy link
Contributor Author

viperML commented Aug 19, 2024

You can evaluate this expression without callPackage or builtins.functionArgs with import ./test.nix (with import <nixpkgs> {}; {inherit hello symlinkJoin lib;})

@NobbZ
Copy link

NobbZ commented Aug 19, 2024

Either way hello is not used, and you'd need ... to signify "and others", you then can not use callPackage anymore.

Point remains, hello is unused currently and nil is rightfully complaining.

@inclyc
Copy link

inclyc commented Aug 19, 2024

The problem here is: {a, unused }@arg: a, the formal1 unused might be used later by accessing arg, but the language server cannot precisely track it.

In libnixf we treat { a, unused }@arg: a differently from { a, unused }: a

See the image below:

Case 1: maybeReferenced could be referenced later from c, thus this is a "Hint". It causes the editor rendering faded text.
Case 2: b is definitely unused, thus this is a "Warning". Hinting the user to fix that.

Either way hello is not used, and you'd need ... to signify "and others", you then can not use callPackage anymore.

That's the reason why hello is used in this case. Unused things are something we can safely remove without further modifications. If it affects the usage of callPackage, then hello is actually used (but yes, it is not "directly used", by its name).

Footnotes

  1. In Nix language parser (official) it is called "formal", but not very documented.

@viperML
Copy link
Contributor Author

viperML commented Aug 19, 2024

I do agree that this might be or not an issue depending on how we define "using"...

@NobbZ
Copy link

NobbZ commented Aug 20, 2024

In {bar}@foo: foo.bar, the binding bar is unused. The value of it is not unused.

I do see, that the use of the unused binding in the pattern slightly improves the error message in case of calling the function wrong, I also see the benefits in using Vipers original example and how it enables the usage of callPackage where it wouldn't be possible otherwise.

Still, I consider the warning "unused binding" correct, as the binding itself remains unused.

I would prefer an explicit "#ignore: unused" magic comment or so, rather than implicit logic in nil (or the supporting library) that would heuristically decide whether or not a bindings value was used by the use of the encapsulating attribute set.


I do agree that this might be or not an issue depending on how we define "using"...

As seen above, its not about how we define "unused", it is about the object it describes.

@oxalica oxalica added the C-support Catagory: support questions label Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-support Catagory: support questions
Projects
None yet
Development

No branches or pull requests

4 participants