nix-linter icon indicating copy to clipboard operation
nix-linter copied to clipboard

False positive "Move `let` block..." lint

Open lovesegfault opened this issue 4 years ago • 4 comments

The following code is minimized from something I had in a nixpkgs overlay:

pkgs: _:
let
  common = if pkgs.hostPlatform.system == "x86_64-linux" then pkgs.foo else pkgs.bar;
in
{
  foo = import ./foo.nix { inherit common; };
  bar = import ./bar.nix { inherit common; };
  baz = import ./baz.nix { inherit common; };
}

It triggers this warning:

Move `let` block outside function definition at foo.nix:1:7-10:1

Which I believe to be a false-positive as moving the let block outside the function definition will put pkgs out of scope and break the if ... then ... else clause.

lovesegfault avatar Oct 06 '20 22:10 lovesegfault

What if nix-linter meant the following:

pkgs:
let
  common = if pkgs.hostPlatform.system == "x86_64-linux" then pkgs.foo else pkgs.bar;
in
_: {
  foo = import ./foo.nix { inherit common; };
  bar = import ./bar.nix { inherit common; };
  baz = import ./baz.nix { inherit common; };
}

That would technically be correct but also stupid.

EDIT: This would mean that this is not a false positive. But instead another issue. I agree with the OP though in that the linter should not recommend moving the let block outside of the second function definition.

seppeljordan avatar Dec 12 '20 12:12 seppeljordan

What if nix-linter meant the following:

It does mean that, so it is working as intended. I'm not sure how one would make the distinction between this "stupid" case and the normal, more useful case (even though this check is not too important even if it does correctly trigger). Maybe it should just be disabled by default to be honest

Synthetica9 avatar Dec 14 '20 20:12 Synthetica9

Oh, indeed, this is way trickier than I expected at first.

@Synthetica9 perhaps just making the diagnostics/hint clearer would be sufficient?

lovesegfault avatar Dec 14 '20 21:12 lovesegfault

So how to disable this check?

Neither can I find an option that would disable a check at all, nor would I know which of the checks causes this error.

NobbZ avatar Jan 01 '21 22:01 NobbZ