Skip to content

Commit 886a038

Browse files
fasttimefasttime
authored
fix: handle files with unspecified path in getRulesMetaForResults (#16437)
* fix: handle files with unspecified path in `getRulesMetaForResults` * In `getRulesMetaForResults`, files with an unspecified path are now treated as if they were located inside `cwd`. * In `getRulesMetaForResults`, when a result referencing a rule has no config, we will explicitly throw an error with a descriptive message. * Added top-level, internal functions `getPlaceholderPath` and `createExtraneousResultsError` to avoid code repetition. * Added two new unit tests. * Renamed an existing unit test to better disambiguate it from a new one. Also changed the assertion to check both error message and constructor. Fixes #16410 * Add a new unit test, move around an existing one * Add a unit test with linted and ignored files * Assert that there is an ignored file
1 parent 319f0a5 commit 886a038

2 files changed

Lines changed: 130 additions & 14 deletions

File tree

‎lib/eslint/flat-eslint.js‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,16 @@ function createRulesMeta(rules) {
161161
}, {});
162162
}
163163

164+
/**
165+
* Return the absolute path of a file named `"__placeholder__.js"` in a given directory.
166+
* This is used as a replacement for a missing file path.
167+
* @param {string} cwd An absolute directory path.
168+
* @returns {string} The absolute path of a file named `"__placeholder__.js"` in the given directory.
169+
*/
170+
function getPlaceholderPath(cwd) {
171+
return path.join(cwd, "__placeholder__.js");
172+
}
173+
164174
/** @type {WeakMap<ExtractedConfig, DeprecatedRuleInfo[]>} */
165175
const usedDeprecatedRulesCache = new WeakMap();
166176

@@ -177,7 +187,7 @@ function getOrFindUsedDeprecatedRules(eslint, maybeFilePath) {
177187
} = privateMembers.get(eslint);
178188
const filePath = path.isAbsolute(maybeFilePath)
179189
? maybeFilePath
180-
: path.join(cwd, "__placeholder__.js");
190+
: getPlaceholderPath(cwd);
181191
const config = configs.getConfig(filePath);
182192

183193
// Most files use the same config, so cache it.
@@ -422,7 +432,7 @@ function verifyText({
422432
* `config.extractConfig(filePath)` requires an absolute path, but `linter`
423433
* doesn't know CWD, so it gives `linter` an absolute path always.
424434
*/
425-
const filePathToVerify = filePath === "<text>" ? path.join(cwd, "__placeholder__.js") : filePath;
435+
const filePathToVerify = filePath === "<text>" ? getPlaceholderPath(cwd) : filePath;
426436
const { fixed, messages, output } = linter.verifyAndFix(
427437
text,
428438
configs,
@@ -519,6 +529,14 @@ function *iterateRuleDeprecationWarnings(configs) {
519529
}
520530
}
521531

532+
/**
533+
* Creates an error to be thrown when an array of results passed to `getRulesMetaForResults` was not created by the current engine.
534+
* @returns {TypeError} An error object.
535+
*/
536+
function createExtraneousResultsError() {
537+
return new TypeError("Results object was not created from this ESLint instance.");
538+
}
539+
522540
//-----------------------------------------------------------------------------
523541
// Main API
524542
//-----------------------------------------------------------------------------
@@ -655,7 +673,10 @@ class FlatESLint {
655673
}
656674

657675
const resultRules = new Map();
658-
const { configs } = privateMembers.get(this);
676+
const {
677+
configs,
678+
options: { cwd }
679+
} = privateMembers.get(this);
659680

660681
/*
661682
* We can only accurately return rules meta information for linting results if the
@@ -664,7 +685,7 @@ class FlatESLint {
664685
* to let the user know we can't do anything here.
665686
*/
666687
if (!configs) {
667-
throw new TypeError("Results object was not created from this ESLint instance.");
688+
throw createExtraneousResultsError();
668689
}
669690

670691
for (const result of results) {
@@ -673,20 +694,23 @@ class FlatESLint {
673694
* Normalize filename for <text>.
674695
*/
675696
const filePath = result.filePath === "<text>"
676-
? "__placeholder__.js" : result.filePath;
677-
678-
/*
679-
* All of the plugin and rule information is contained within the
680-
* calculated config for the given file.
681-
*/
682-
const config = configs.getConfig(filePath);
697+
? getPlaceholderPath(cwd) : result.filePath;
683698
const allMessages = result.messages.concat(result.suppressedMessages);
684699

685700
for (const { ruleId } of allMessages) {
686701
if (!ruleId) {
687702
continue;
688703
}
689704

705+
/*
706+
* All of the plugin and rule information is contained within the
707+
* calculated config for the given file.
708+
*/
709+
const config = configs.getConfig(filePath);
710+
711+
if (!config) {
712+
throw createExtraneousResultsError();
713+
}
690714
const rule = getRuleFromConfig(ruleId, config);
691715

692716
// ensure the rule exists
@@ -1024,7 +1048,7 @@ class FlatESLint {
10241048
const npmFormat = naming.normalizePackageName(normalizedFormatName, "eslint-formatter");
10251049

10261050
// TODO: This is pretty dirty...would be nice to clean up at some point.
1027-
formatterPath = ModuleResolver.resolve(npmFormat, path.join(cwd, "__placeholder__.js"));
1051+
formatterPath = ModuleResolver.resolve(npmFormat, getPlaceholderPath(cwd));
10281052
} catch {
10291053
formatterPath = path.resolve(__dirname, "../", "cli-engine", "formatters", `${normalizedFormatName}.js`);
10301054
}

‎tests/lib/eslint/flat-eslint.js‎

Lines changed: 94 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3752,7 +3752,7 @@ describe("FlatESLint", () => {
37523752

37533753
describe("getRulesMetaForResults()", () => {
37543754

3755-
it("should throw an error when results were not created from this instance", async () => {
3755+
it("should throw an error when this instance did not lint any files", async () => {
37563756
const engine = new FlatESLint({
37573757
overrideConfigFile: true
37583758
});
@@ -3789,7 +3789,99 @@ describe("FlatESLint", () => {
37893789
"var err = doStuff();\nif (err) console.log('failed tests: ' + err);\nprocess.exit(1);\n"
37903790
}
37913791
]);
3792-
}, /Results object was not created from this ESLint instance/u);
3792+
}, {
3793+
constructor: TypeError,
3794+
message: "Results object was not created from this ESLint instance."
3795+
});
3796+
});
3797+
3798+
it("should throw an error when results were created from a different instance", async () => {
3799+
const engine1 = new FlatESLint({
3800+
overrideConfigFile: true,
3801+
cwd: path.join(fixtureDir, "foo"),
3802+
overrideConfig: {
3803+
rules: {
3804+
semi: 2
3805+
}
3806+
}
3807+
});
3808+
const engine2 = new FlatESLint({
3809+
overrideConfigFile: true,
3810+
cwd: path.join(fixtureDir, "bar"),
3811+
overrideConfig: {
3812+
rules: {
3813+
semi: 2
3814+
}
3815+
}
3816+
});
3817+
3818+
const results1 = await engine1.lintText("1", { filePath: "file.js" });
3819+
const results2 = await engine2.lintText("2", { filePath: "file.js" });
3820+
3821+
engine1.getRulesMetaForResults(results1); // should not throw an error
3822+
assert.throws(() => {
3823+
engine1.getRulesMetaForResults(results2);
3824+
}, {
3825+
constructor: TypeError,
3826+
message: "Results object was not created from this ESLint instance."
3827+
});
3828+
});
3829+
3830+
it("should treat a result without `filePath` as if the file was located in `cwd`", async () => {
3831+
const engine = new FlatESLint({
3832+
overrideConfigFile: true,
3833+
cwd: path.join(fixtureDir, "foo", "bar"),
3834+
ignorePatterns: "*/**", // ignore all subdirectories of `cwd`
3835+
overrideConfig: {
3836+
rules: {
3837+
eqeqeq: "warn"
3838+
}
3839+
}
3840+
});
3841+
3842+
const results = await engine.lintText("a==b");
3843+
const rulesMeta = engine.getRulesMetaForResults(results);
3844+
3845+
assert.deepStrictEqual(rulesMeta.eqeqeq, coreRules.get("eqeqeq").meta);
3846+
});
3847+
3848+
it("should not throw an error if a result without `filePath` contains an ignored file warning", async () => {
3849+
const engine = new FlatESLint({
3850+
overrideConfigFile: true,
3851+
cwd: path.join(fixtureDir, "foo", "bar"),
3852+
ignorePatterns: "**"
3853+
});
3854+
3855+
const results = await engine.lintText("", { warnIgnored: true });
3856+
const rulesMeta = engine.getRulesMetaForResults(results);
3857+
3858+
assert.deepStrictEqual(rulesMeta, {});
3859+
});
3860+
3861+
it("should not throw an error if results contain linted files and one ignored file", async () => {
3862+
const engine = new FlatESLint({
3863+
overrideConfigFile: true,
3864+
cwd: getFixturePath(),
3865+
ignorePatterns: "passing*",
3866+
overrideConfig: {
3867+
rules: {
3868+
"no-undef": 2,
3869+
semi: 1
3870+
}
3871+
}
3872+
});
3873+
3874+
const results = await engine.lintFiles(["missing-semicolon.js", "passing.js", "undef.js"]);
3875+
3876+
assert(
3877+
results.some(({ messages }) => messages.some(({ message, ruleId }) => !ruleId && message.startsWith("File ignored"))),
3878+
"At least one file should be ignored but none is."
3879+
);
3880+
3881+
const rulesMeta = engine.getRulesMetaForResults(results);
3882+
3883+
assert.deepStrictEqual(rulesMeta["no-undef"], coreRules.get("no-undef").meta);
3884+
assert.deepStrictEqual(rulesMeta.semi, coreRules.get("semi").meta);
37933885
});
37943886

37953887
it("should return empty object when there are no linting errors", async () => {

0 commit comments

Comments
 (0)