Missing detection of __findFile usages.

[voronind-com]

Describe the bug
Right now nixd reports __findFile as unused even when there are <> references.

Expected behavior
Using <path> references marks __findFile arg as used.

Screenshots

[inclyc]

I think __findFile is internal to C++ nix implementation thus we should not expose this explicitly. CC @roberth @Mic92

[voronind-com]

I think __findFile is internal to C++ nix implementation thus we should not expose this explicitly. CC @roberth @Mic92

Why not? Is it gonna hurt users with other implementations? Because now it hurts people with cppnix, which are the majority.

Also this might only mean that nixd could use another configuration parameter being the nix implementation used.

People without cppnix would not use this thing in their codebase anyway. And this thing is quite popular.

[inclyc]

Please note that the language reference only states that

A lookup path is an identifier with an optional path suffix that resolves to a path value if the identifier matches a search path entry in builtins.nixPath. The algorithm for lookup path resolution is described in the documentation on builtins.findFile.

But it never said <> expressions will be expanded to __findFile. It just said: it is guaranteed that "the algorithm" is the same as builtins.findFile.

So I don't think it is correct to treat this syntax as a user of __findFile.

Why not? Is it gonna hurt users with other implementations? Because now it hurts people with cppnix, which are the majority.

It is not a consideration about implementations, but about what is "exposed" from a language's specification.

[voronind-com]

Do you mean this is about usability vs correctness?

[Mic92]

This is the first time, I hear of __findFile. Why do user have to specify it explicitly and how is it useful?

[inclyc]

This is the first time, I hear of __findFile. Why do user have to specify it explicitly and how is it useful?

Actually nix does allow overriding many base "internal implementation". For example,

a < b

will be expanded into

__lessThan a b

And in fact users can redefine __lessThan, e.g.

❯ nix repl
Nix 2.24.10
Type :? for help.
nix-repl> __lessThan = x: y: x + y

nix-repl> 1 < 2
3

[Mic92]

Oh. We shouldn't users encourage to do that I suppose. It's ok for debugging tools to show these internals, but endusers should not write their code in this way. If it would help to resolve the issue, maybe it could be turned into a warning that an internal symbol is used?

[inclyc]

Oh. We shouldn't users encourage to do that I suppose. It's ok for debugging tools to show these internals, but endusers should not write their code in this way. If it would help to resolve the issue, maybe it could be turned into a warning that an internal symbol is used?

Agreed. Hi @voronind-com, would you like to accept this solution? i.e. let nixd mark warnings at internal symbols usages.

[voronind-com]

Oh. We shouldn't users encourage to do that I suppose. It's ok for debugging tools to show these internals, but endusers should not write their code in this way. If it would help to resolve the issue, maybe it could be turned into a warning that an internal symbol is used?

Agreed. Hi @voronind-com, would you like to accept this solution? i.e. let nixd mark warnings at internal symbols usages.

I'm unsure. The __findFile is commonly used as an easy reference from the git root, i.e. <packages> translates to the /packages dir.

I still vote that __findFile should be considered used (as it technically is) when there are <>. As well as __lessThan when < is used etc.

Should warnings be added on top of that or not is another question.

[Mic92]

Well probably fair. I am just not sure how hard to special case it is nice.