Incorrect file resolution for paths higher than CWD

[aliaksandr-yermalayeu]

Env:

Windows 7 x64, NodeJs v10.15.0, eslint 5.11.1, eslint-plugin-import: 2.14.0

Repro steps:

Rule no-unresolved is turned on with case sensitive option

"rules": {
        "import/no-unresolved": ["error", { caseSensitive: true}]
 }

Project root is "B/"
CWD is "C/"

Project Structure (tree)

├─ A
   ├─ B
   │  ├─ C
   │  │  ├─ D.js

import D from 'B/C/D.js'; // should be no error (casing is preserved correctly)

import D from 'B/C/d.js'; // causes error (filename is misspelled)

import D from 'B/c/D.js'; // causes error (cwd is misspelled)

Expected result:

import D from 'b/C/D.js'; // should cause error (project root is misspelled)

Actual result:

import D from 'b/C/D.js'; // no error (project root is misspelled)

Description:

Eslint task is launched by grunt, where grunt config file is located in "C/" folder (CWD). While actual project folder is one ore more levels higher than cwd ("B/" ). So imports with such path don't cause error.

It is caused by "fileExistsWithCaseSync" function
https://github.com/benmosher/eslint-plugin-import/blob/1cd82eb27df85768fbd076e4ff6b7f36d6f652ce/utils/resolve.js#L38
So it basically doesn't go any higher than cwd to cache path.

Suggestion:
It either should not be limited at all (because it is relatively cheap operation as it caches previous paths),

import D from 'a/B/C/D.js'; // should also cause error (it is higher than project root)

or at least it should be limited to project dir (something like getBaseDir() is doing - line 153)

import D from 'b/C/D.js'; // should cause error (project root is misspelled)

[ljharb]

All commands should always be ran from the project root; why is the cwd not A?

[aliaksandr-yermalayeu]

One example is monorepo, which aggregates multiple projects under one root.

[ljharb]

In a monorepo, you'd still want to run every command from the root.

[sergei-startsev]

@ljharb I think the issue is wider than just current working directory. import/no-unresolved rule ensures that a module can be resolved on the local filesystem and a resolver used by the rule should return an absolute path if a module can be located on the filesystem. Applying caseSensitive option you expect that the rule checks if a resolved absolute path matches a filesystem path, however just a part of a resolved path is actually checked. However a resolver can return a case-insensitive absolute path. There's also a non-zero chance that an import contains an absolute path that includes cwd.

Since most of build tools are case-sensitive, a module with an equal name when case is ignored can lead to unexpected behavior and cause real bugs because different module instances are returned.

I suppose performance optimization makes sense here, but I'd rather to have an option (caseSensitiveStrict?) to check the full absolute path to prevent possible inconsistency.

[ljharb]

If there's a reproducible test case, I'm open to fixing it, but I'm still not clear on the use case.

[sergei-startsev]

@ljharb, here're a few examples reproducible with NodeJS resolver (used by default):

// Absolute paths
import Foo from `/Users/fOo/bar/file.js`
import Foo from `d:/fOo/bar/file.js`

// Relative paths, cwd is Users/foo/
import Foo from `./../fOo/bar/file.js`

[ljharb]

@sergei-startsev with each of those, what is the current vs expected error?

[sergei-startsev]

It's expected that the code above (each of imports) is identified as invalid by import/no-unresolved rule with enabled caseSensitive option since resolved paths mismatch filesystem paths. However no errors are emitted.