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

[New] no-unresolved: add caseSensitiveStrict option #1262

Merged
merged 2 commits into from Sep 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
## [Unreleased]

### Added
- [`no-unresolved`]: add `caseSensitiveStrict` option ([#1262], thanks [@sergei-startsev])
- [`no-unused-modules`]: add eslint v8 support ([#2194], thanks [@coderaiser])
- [`no-restricted-paths`]: add/restore glob pattern support ([#2219], thanks [@stropho])

Expand Down Expand Up @@ -1053,6 +1054,7 @@ for info on changes for earlier releases.
[#1294]: https://github.com/import-js/eslint-plugin-import/pull/1294
[#1290]: https://github.com/import-js/eslint-plugin-import/pull/1290
[#1277]: https://github.com/import-js/eslint-plugin-import/pull/1277
[#1262]: https://github.com/import-js/eslint-plugin-import/pull/1262
[#1257]: https://github.com/import-js/eslint-plugin-import/pull/1257
[#1253]: https://github.com/import-js/eslint-plugin-import/pull/1253
[#1248]: https://github.com/import-js/eslint-plugin-import/pull/1248
Expand Down
19 changes: 17 additions & 2 deletions docs/rules/no-unresolved.md
Expand Up @@ -76,10 +76,25 @@ By default, this rule will report paths whose case do not match the underlying f
const { default: x } = require('./foo') // reported if './foo' is actually './Foo' and caseSensitive: true
```

#### `caseSensitiveStrict`

The `caseSensitive` option does not detect case for the current working directory. The `caseSensitiveStrict` option allows checking `cwd` in resolved path. By default, the option is disabled.


```js
/*eslint import/no-unresolved: [2, { caseSensitiveStrict: true }]*/

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

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

## When Not To Use It

If you're using a module bundler other than Node or Webpack, you may end up with
a lot of false positive reports of missing dependencies.
If you're using a module bundler other than Node or Webpack, you may end up with a lot of false positive reports of missing dependencies.

## Further Reading

Expand Down
13 changes: 8 additions & 5 deletions src/rules/no-unresolved.js
Expand Up @@ -18,14 +18,17 @@ module.exports = {
schema: [
makeOptionsSchema({
caseSensitive: { type: 'boolean', default: true },
caseSensitiveStrict: { type: 'boolean', default: false },
}),
],
},

create(context) {
const options = context.options[0] || {};

function checkSourceValue(source) {
const shouldCheckCase = !CASE_SENSITIVE_FS
&& (!context.options[0] || context.options[0].caseSensitive !== false);
const caseSensitive = !CASE_SENSITIVE_FS && options.caseSensitive !== false;
const caseSensitiveStrict = !CASE_SENSITIVE_FS && options.caseSensitiveStrict;

const resolvedPath = resolve(source.value, context);

Expand All @@ -34,9 +37,9 @@ module.exports = {
source,
`Unable to resolve path to module '${source.value}'.`
);
} else if (shouldCheckCase) {
} else if (caseSensitive || caseSensitiveStrict) {
const cacheSettings = ModuleCache.getSettings(context.settings);
if (!fileExistsWithCaseSync(resolvedPath, cacheSettings)) {
if (!fileExistsWithCaseSync(resolvedPath, cacheSettings, caseSensitiveStrict)) {
context.report(
source,
`Casing of ${source.value} does not match the underlying filesystem.`
Expand All @@ -45,6 +48,6 @@ module.exports = {
}
}

return moduleVisitor(checkSourceValue, context.options[0]);
return moduleVisitor(checkSourceValue, options);
},
};
16 changes: 12 additions & 4 deletions tests/src/core/resolve.js
Expand Up @@ -3,7 +3,6 @@ import eslintPkg from 'eslint/package.json';
import semver from 'semver';

import resolve, { CASE_SENSITIVE_FS, fileExistsWithCaseSync } from 'eslint-module-utils/resolve';
import ModuleCache from 'eslint-module-utils/ModuleCache';

import * as path from 'path';
import * as fs from 'fs';
Expand Down Expand Up @@ -319,7 +318,11 @@ describe('resolve', function () {
const caseDescribe = (!CASE_SENSITIVE_FS ? describe : describe.skip);
caseDescribe('case sensitivity', function () {
let file;
const testContext = utils.testContext({ 'import/resolve': { 'extensions': ['.jsx'] } });
const testContext = utils.testContext({
'import/resolve': { 'extensions': ['.jsx'] },
'import/cache': { lifetime: 0 },
});
const testSettings = testContext.settings;
before('resolve', function () {
file = resolve(
// Note the case difference 'MyUncoolComponent' vs 'MyUnCoolComponent'
Expand All @@ -329,14 +332,19 @@ describe('resolve', function () {
expect(file, 'path to ./jsx/MyUncoolComponent').to.exist;
});
it('detects case does not match FS', function () {
expect(fileExistsWithCaseSync(file, ModuleCache.getSettings(testContext)))
expect(fileExistsWithCaseSync(file, testSettings))
.to.be.false;
});
it('detecting case does not include parent folder path (issue #720)', function () {
const f = path.join(process.cwd().toUpperCase(), './tests/files/jsx/MyUnCoolComponent.jsx');
expect(fileExistsWithCaseSync(f, ModuleCache.getSettings(testContext), true))
expect(fileExistsWithCaseSync(f, testSettings))
.to.be.true;
});
it('detecting case should include parent folder path', function () {
const f = path.join(process.cwd().toUpperCase(), './tests/files/jsx/MyUnCoolComponent.jsx');
expect(fileExistsWithCaseSync(f, testSettings, true))
.to.be.false;
});
});

describe('rename cache correctness', function () {
Expand Down
29 changes: 28 additions & 1 deletion tests/src/rules/no-unresolved.js
Expand Up @@ -15,7 +15,7 @@ function runResolverTests(resolver) {
function rest(specs) {
specs.settings = Object.assign({},
specs.settings,
{ 'import/resolver': resolver },
{ 'import/resolver': resolver, 'import/cache': { lifetime: 0 } },
);

return test(specs);
Expand Down Expand Up @@ -227,6 +227,10 @@ function runResolverTests(resolver) {
});

if (!CASE_SENSITIVE_FS) {
const relativePath = './tests/files/jsx/MyUnCoolComponent.jsx';
const cwd = process.cwd();
const mismatchedPath = path.join(cwd.toUpperCase(), relativePath).replace(/\\/g, '/');

ruleTester.run('case sensitivity', rule, {
valid: [
rest({ // test with explicit flag
Expand All @@ -247,6 +251,29 @@ function runResolverTests(resolver) {
}),
],
});

ruleTester.run('case sensitivity strict', rule, {
valid: [
// #1259 issue
rest({ // caseSensitiveStrict is disabled by default
code: `import foo from "${mismatchedPath}"`,
}),
],

invalid: [
// #1259 issue
rest({ // test with enabled caseSensitiveStrict option
code: `import foo from "${mismatchedPath}"`,
options: [{ caseSensitiveStrict: true }],
errors: [`Casing of ${mismatchedPath} does not match the underlying filesystem.`],
}),
rest({ // test with enabled caseSensitiveStrict option and disabled caseSensitive
code: `import foo from "${mismatchedPath}"`,
options: [{ caseSensitiveStrict: true, caseSensitive: false }],
errors: [`Casing of ${mismatchedPath} does not match the underlying filesystem.`],
}),
],
});
}

}
Expand Down
5 changes: 5 additions & 0 deletions utils/CHANGELOG.md
Expand Up @@ -5,6 +5,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

## Unreleased

### Added
- `fileExistsWithCaseSync`: add `strict` argument ([#1262], thanks [@sergei-startsev])

## v2.6.2 - 2021-08-08

### Fixed
Expand Down Expand Up @@ -102,6 +105,7 @@ Yanked due to critical issue with cache key resulting from #839.
[#1409]: https://github.com/import-js/eslint-plugin-import/pull/1409
[#1356]: https://github.com/import-js/eslint-plugin-import/pull/1356
[#1290]: https://github.com/import-js/eslint-plugin-import/pull/1290
[#1262]: https://github.com/import-js/eslint-plugin-import/pull/1262
[#1218]: https://github.com/import-js/eslint-plugin-import/pull/1218
[#1166]: https://github.com/import-js/eslint-plugin-import/issues/1166
[#1160]: https://github.com/import-js/eslint-plugin-import/pull/1160
Expand All @@ -119,6 +123,7 @@ Yanked due to critical issue with cache key resulting from #839.
[@kaiyoma]: https://github.com/kaiyoma
[@manuth]: https://github.com/manuth
[@pmcelhaney]: https://github.com/pmcelhaney
[@sergei-startsev]: https://github.com/sergei-startsev
[@sompylasar]: https://github.com/sompylasar
[@timkraut]: https://github.com/timkraut
[@vikr01]: https://github.com/vikr01
6 changes: 3 additions & 3 deletions utils/resolve.js
Expand Up @@ -52,13 +52,13 @@ function tryRequire(target, sourceFile) {
}

// http://stackoverflow.com/a/27382838
exports.fileExistsWithCaseSync = function fileExistsWithCaseSync(filepath, cacheSettings) {
exports.fileExistsWithCaseSync = function fileExistsWithCaseSync(filepath, cacheSettings, strict) {
// don't care if the FS is case-sensitive
if (CASE_SENSITIVE_FS) return true;

// null means it resolved to a builtin
if (filepath === null) return true;
if (filepath.toLowerCase() === process.cwd().toLowerCase()) return true;
if (filepath.toLowerCase() === process.cwd().toLowerCase() && !strict) return true;
const parsedPath = path.parse(filepath);
const dir = parsedPath.dir;

Expand All @@ -73,7 +73,7 @@ exports.fileExistsWithCaseSync = function fileExistsWithCaseSync(filepath, cache
if (filenames.indexOf(parsedPath.base) === -1) {
result = false;
} else {
result = fileExistsWithCaseSync(dir, cacheSettings);
result = fileExistsWithCaseSync(dir, cacheSettings, strict);
}
}
fileExistsCache.set(filepath, result);
Expand Down