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

[Fix] no-unused-modules: don't error out when running with flat config and an eslintrc isn't present #3116

Merged
merged 1 commit into from
Dec 24, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
});
Loading