Skip to content

Commit

Permalink
[Fix] no-unused-modules: provide more meaningful error message when…
Browse files Browse the repository at this point in the history
… no eslintrc is present

This change adjusts what we're doing if an error is thrown while attempting to enumerate lintable files in the no-used-modules rule, when users are running with flat config. Now, if FileEnumerator throws due to a lack of eslintrc and the user is running with flat config, we catch the error and rethrow with a more informative message.

I also cleaned up the original aspects of the implementation that was using the proposed eslint context functions, since that proposal was walked back and the API was never introduced.

Note: This isn't an ideal state, since this rule still relies on the legacy api to understand what needs to be ignored. Remaining tethered to the legacy config system is going to need to be solved at some point.

Fixes #3079
  • Loading branch information
michaelfaith committed Dec 24, 2024
1 parent aee018f commit e5edf49
Show file tree
Hide file tree
Showing 12 changed files with 157 additions and 116 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
- add [`enforce-node-protocol-usage`] rule and `import/node-version` setting ([#3024], thanks [@GoldStrikeArch] and [@sevenc-nanashi])
- add TypeScript types ([#3097], thanks [@G-Rath])

### Fixed
- [`no-unused-modules`]: provide more meaningful error message when no .eslintrc is present ([#3116], thanks [@michaelfaith])

### Changed
- [Docs] [`extensions`], [`order`]: improve documentation ([#3106], thanks [@Xunnamius])
- [Docs] add flat config guide for using `tseslint.config()` ([#3125], thanks [@lnuvy])
Expand Down Expand Up @@ -1165,6 +1168,7 @@ for info on changes for earlier releases.

[#3125]: https://github.com/import-js/eslint-plugin-import/pull/3125
[#3122]: https://github.com/import-js/eslint-plugin-import/pull/3122
[#3116]: https://github.com/import-js/eslint-plugin-import/pull/3116
[#3106]: https://github.com/import-js/eslint-plugin-import/pull/3106
[#3097]: https://github.com/import-js/eslint-plugin-import/pull/3097
[#3073]: https://github.com/import-js/eslint-plugin-import/pull/3073
Expand Down
22 changes: 22 additions & 0 deletions examples/v9/eslint.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import importPlugin from 'eslint-plugin-import';
import js from '@eslint/js';

export default [
js.configs.recommended,
importPlugin.flatConfigs.recommended,
{
files: ['**/*.{js,mjs,cjs}'],
languageOptions: {
ecmaVersion: 'latest',
sourceType: 'module',
},
ignores: ['eslint.config.mjs', 'node_modules/*'],
rules: {
'no-unused-vars': 'off',
'import/no-dynamic-require': 'warn',
'import/no-nodejs-modules': 'warn',
'import/no-unused-modules': ['warn', { unusedExports: true }],
'import/no-cycle': 'warn',
},
},
];
14 changes: 14 additions & 0 deletions examples/v9/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "v9",
"version": "1.0.0",
"main": "index.js",
"type": "module",
"scripts": {
"lint": "eslint src --report-unused-disable-directives"
},
"devDependencies": {
"@eslint/js": "^9.17.0",
"eslint": "^9.17.0",
"eslint-plugin-import": "file:../.."
}
}
3 changes: 3 additions & 0 deletions examples/v9/src/depth-zero.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { foo } from "./es6/depth-one-dynamic";

foo();
3 changes: 3 additions & 0 deletions examples/v9/src/es6/depth-one-dynamic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function foo() {}

export const bar = () => import("../depth-zero").then(({foo}) => foo);
6 changes: 6 additions & 0 deletions examples/v9/src/exports-unused.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const a = 13;
export const b = 18;

const defaultExport = { a, b };

export default defaultExport;
6 changes: 6 additions & 0 deletions examples/v9/src/exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const a = 13;
export const b = 18;

const defaultExport = { a, b };

export default defaultExport;
6 changes: 6 additions & 0 deletions examples/v9/src/imports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//import c from './exports';
import { a, b } from './exports';

import path from 'path';
import fs from 'node:fs';
import console from 'console';
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@
"test": "npm run tests-only",
"test-compiled": "npm run prepublish && BABEL_ENV=testCompiled mocha --compilers js:babel-register tests/src",
"test-all": "node --require babel-register ./scripts/testAll",
"test-examples": "npm run build && npm run test-example:legacy && npm run test-example:flat",
"test-examples": "npm run build && npm run test-example:legacy && npm run test-example:flat && npm run test-example:v9",
"test-example:legacy": "cd examples/legacy && npm install && npm run lint",
"test-example:flat": "cd examples/flat && npm install && npm run lint",
"test-example:v9": "cd examples/v9 && npm install && npm run lint",
"test-types": "npx --package typescript@latest tsc --noEmit index.d.ts",
"prepublishOnly": "safe-publish-latest && npm run build",
"prepublish": "not-in-publish || npm run prepublishOnly",
Expand Down Expand Up @@ -106,6 +107,7 @@
"rimraf": "^2.7.1",
"safe-publish-latest": "^2.0.0",
"sinon": "^2.4.1",
"tmp": "^0.2.1",
"typescript": "^2.8.1 || ~3.9.5 || ~4.5.2",
"typescript-eslint-parser": "^15 || ^20 || ^22"
},
Expand Down
48 changes: 0 additions & 48 deletions src/core/fsWalk.js

This file was deleted.

118 changes: 51 additions & 67 deletions src/rules/no-unused-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ import { getPhysicalFilename } from 'eslint-module-utils/contextCompat';
import { getFileExtensions } from 'eslint-module-utils/ignore';
import resolve from 'eslint-module-utils/resolve';
import visit from 'eslint-module-utils/visit';
import { dirname, join, resolve as resolvePath } from 'path';
import { dirname, join } from 'path';
import readPkgUp from 'eslint-module-utils/readPkgUp';
import values from 'object.values';
import includes from 'array-includes';
import flatMap from 'array.prototype.flatmap';

import { walkSync } from '../core/fsWalk';
import ExportMapBuilder from '../exportMap/builder';
import recursivePatternCapture from '../exportMap/patternCapture';
import docsUrl from '../docsUrl';
Expand Down Expand Up @@ -51,21 +50,62 @@ function requireFileEnumerator() {
}

/**
*
* Given a FileEnumerator class, instantiate and load the list of files.
* @param FileEnumerator the `FileEnumerator` class from `eslint`'s internal api
* @param {string} src path to the src root
* @param {string[]} extensions list of supported extensions
* @returns {{ filename: string, ignored: boolean }[]} list of files to operate on
*/
function listFilesUsingFileEnumerator(FileEnumerator, src, extensions) {
const e = new FileEnumerator({
// We need to know whether this is being run with flat config in order to
// determine how to report errors if FileEnumerator throws due to a lack of eslintrc.

const { ESLINT_USE_FLAT_CONFIG } = process.env;

// This condition is sufficient to test in v8, since the environment variable is necessary to turn on flat config
let isUsingFlatConfig = ESLINT_USE_FLAT_CONFIG && process.env.ESLINT_USE_FLAT_CONFIG !== 'false';

// In the case of using v9, we can check the `shouldUseFlatConfig` function
// If this function is present, then we assume it's v9
try {
const { shouldUseFlatConfig } = require('eslint/use-at-your-own-risk');
isUsingFlatConfig = shouldUseFlatConfig && ESLINT_USE_FLAT_CONFIG !== 'false';
} catch (_) {
// We don't want to throw here, since we only want to update the
// boolean if the function is available.
}

const enumerator = new FileEnumerator({
extensions,
});

return Array.from(
e.iterateFiles(src),
({ filePath, ignored }) => ({ filename: filePath, ignored }),
);
try {
return Array.from(
enumerator.iterateFiles(src),
({ filePath, ignored }) => ({ filename: filePath, ignored }),
);
} catch (e) {
// If we're using flat config, and FileEnumerator throws due to a lack of eslintrc,
// then we want to throw an error so that the user knows about this rule's reliance on
// the legacy config.
if (
isUsingFlatConfig
&& e.message.includes('No ESLint configuration found')
) {
throw new Error(`
Due to the exclusion of certain internal ESLint APIs when using flat config,
the import/no-unused-modules rule requires an .eslintrc file to know which
files to ignore (even when using flat config).
The .eslintrc file only needs to contain "ignorePatterns", or can be empty if
you do not want to ignore any files.
See https://github.com/import-js/eslint-plugin-import/issues/3079
for additional context.
`);
}
// If this isn't the case, then we'll just let the error bubble up
throw e;
}
}

/**
Expand Down Expand Up @@ -107,70 +147,14 @@ function listFilesWithLegacyFunctions(src, extensions) {
}
}

/**
* Given a source root and list of supported extensions, use fsWalk and the
* new `eslint` `context.session` api to build the list of files we want to operate on
* @param {string[]} srcPaths array of source paths (for flat config this should just be a singular root (e.g. cwd))
* @param {string[]} extensions list of supported extensions
* @param {{ isDirectoryIgnored: (path: string) => boolean, isFileIgnored: (path: string) => boolean }} session eslint context session object
* @returns {string[]} list of files to operate on
*/
function listFilesWithModernApi(srcPaths, extensions, session) {
/** @type {string[]} */
const files = [];

for (let i = 0; i < srcPaths.length; i++) {
const src = srcPaths[i];
// Use walkSync along with the new session api to gather the list of files
const entries = walkSync(src, {
deepFilter(entry) {
const fullEntryPath = resolvePath(src, entry.path);

// Include the directory if it's not marked as ignore by eslint
return !session.isDirectoryIgnored(fullEntryPath);
},
entryFilter(entry) {
const fullEntryPath = resolvePath(src, entry.path);

// Include the file if it's not marked as ignore by eslint and its extension is included in our list
return (
!session.isFileIgnored(fullEntryPath)
&& extensions.find((extension) => entry.path.endsWith(extension))
);
},
});

// Filter out directories and map entries to their paths
files.push(
...entries
.filter((entry) => !entry.dirent.isDirectory())
.map((entry) => entry.path),
);
}
return files;
}

/**
* Given a src pattern and list of supported extensions, return a list of files to process
* with this rule.
* @param {string} src - file, directory, or glob pattern of files to act on
* @param {string[]} extensions - list of supported file extensions
* @param {import('eslint').Rule.RuleContext} context - the eslint context object
* @returns {string[] | { filename: string, ignored: boolean }[]} the list of files that this rule will evaluate.
*/
function listFilesToProcess(src, extensions, context) {
// If the context object has the new session functions, then prefer those
// Otherwise, fallback to using the deprecated `FileEnumerator` for legacy support.
// https://github.com/eslint/eslint/issues/18087
if (
context.session
&& context.session.isFileIgnored
&& context.session.isDirectoryIgnored
) {
return listFilesWithModernApi(src, extensions, context.session);
}

// Fallback to og FileEnumerator
function listFilesToProcess(src, extensions) {
const FileEnumerator = requireFileEnumerator();

// If we got the FileEnumerator, then let's go with that
Expand Down Expand Up @@ -295,10 +279,10 @@ const isNodeModule = (path) => (/\/(node_modules)\//).test(path);
function resolveFiles(src, ignoreExports, context) {
const extensions = Array.from(getFileExtensions(context.settings));

const srcFileList = listFilesToProcess(src, extensions, context);
const srcFileList = listFilesToProcess(src, extensions);

// prepare list of ignored files
const ignoredFilesList = listFilesToProcess(ignoreExports, extensions, context);
const ignoredFilesList = listFilesToProcess(ignoreExports, extensions);

// The modern api will return a list of file paths, rather than an object
if (ignoredFilesList.length && typeof ignoredFilesList[0] === 'string') {
Expand Down
39 changes: 39 additions & 0 deletions tests/src/rules/no-unused-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@ import jsxConfig from '../../../config/react';
import typescriptConfig from '../../../config/typescript';

import { RuleTester } from '../rule-tester';
import { expect } from 'chai';
import { execSync } from 'child_process';
import fs from 'fs';
import eslintPkg from 'eslint/package.json';
import path from 'path';
import process from 'process';
import semver from 'semver';

let FlatRuleTester;
Expand All @@ -14,6 +18,7 @@ try {

// TODO: figure out why these tests fail in eslint 4 and 5
const isESLint4TODO = semver.satisfies(eslintPkg.version, '^4 || ^5');
const isESLint9 = semver.satisfies(eslintPkg.version, '>=9');

const ruleTester = new RuleTester();
const typescriptRuleTester = new RuleTester(typescriptConfig);
Expand Down Expand Up @@ -1482,3 +1487,37 @@ describe('parser ignores prefixes like BOM and hashbang', () => {
});
});
});

(isESLint9 ? describe : describe.skip)('with eslint 9+', () => {
it('provides meaningful error when eslintrc is not present', () => {
const tmp = require('tmp');

// Create temp directory outside of project root
const tempDir = tmp.dirSync({ unsafeCleanup: true });

// Copy example project to temp directory
fs.cpSync(path.join(process.cwd(), 'examples/v9'), tempDir.name, { recursive: true });

let errorMessage = '';

// Build the plugin
try {
execSync('npm run build');
} catch (_) {
/* ignore */
}

// Install the plugin and run the lint command in the temp directory
try {
execSync(`npm install -D ${process.cwd()} && npm run lint`, { cwd: tempDir.name });
} catch (error) {
errorMessage = error.stderr.toString();
}

// Verify that the error message is as expected
expect(errorMessage).to.contain('the import/no-unused-modules rule requires an .eslintrc file');

// Cleanup
tempDir.removeCallback();
}).timeout(100000);
});

0 comments on commit e5edf49

Please sign in to comment.