Bug with shadowed variable not being resolved correctly

[dummdidumm]

Analyzing this code

const path = 123;
let binding;
for (const path of ["asd"]) {
        binding = require(path);
}
exports.default = binding;

fails with the error

Failed to resolve dependency 123:
The "path" argument must be of type string. Received type number (123)

It seems that path is resolved to 123 and not to "asd", and it seems that assigning to binding is what triggeres it, because if that is removed, it works.

The real world case where this happens is when analyzing aws-crt

[styfle]

Thanks for reporting!

Would you like to submit a PR with the fix?

[dummdidumm]

I looked into it and I'm not sure how to proceed. The problem is the very outdated dependency rollup-pluginutils which now lives under @rollup/pluginutils. Using version 4 (version 5 requires node 14 at the minimum, which is probably too high for this package) of it correctly recognizes the for loop as a new scope. Using that prevents the analyze-code from crashing and therefore the warning from being emitted, but the dependency that is required in the above code snippet isn't added to the list of files. I'm not sure if this is expected, or a different bug.

[dummdidumm]

With #316 merged, is there anything left to do here? The dependency "add" isn't added, and I'm not sure if this is expected behavior or a missing case in static analysis.

[styfle]

version 5 requires node 14 at the minimum, which is probably too high for this package

We can a target node 14 now that 12 is EOL https://github.com/vercel/nft/pull/318

[iamabhshk]

To fix this code, you should avoid using the name path for both the loop variable and the constant variable at the beginning. Here's an updated version of the code:

const pathValue = 123;

let binding;
for (const p of ["asd"]) {
    binding = require(p);
}

exports.default = binding;