Skip to content

Commit

Permalink
fix: handle files with unspecified path in getRulesMetaForResults
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
fasttime committed Oct 17, 2022
1 parent 1692840 commit 56cc899
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 14 deletions.
49 changes: 37 additions & 12 deletions lib/eslint/flat-eslint.js
Expand Up @@ -161,6 +161,16 @@ function createRulesMeta(rules) {
}, {});
}

/**
* Return the absolute path of a file named `"__placeholder__.js"` in a given directory.
* This is used as a replacement for a missing file path.
* @param {string} cwd An absolute directory path.
* @returns {string} The absolute path of a file named `"__placeholder__.js"` in the given directory.
*/
function getPlaceholderPath(cwd) {
return path.join(cwd, "__placeholder__.js");
}

/** @type {WeakMap<ExtractedConfig, DeprecatedRuleInfo[]>} */
const usedDeprecatedRulesCache = new WeakMap();

Expand All @@ -177,7 +187,7 @@ function getOrFindUsedDeprecatedRules(eslint, maybeFilePath) {
} = privateMembers.get(eslint);
const filePath = path.isAbsolute(maybeFilePath)
? maybeFilePath
: path.join(cwd, "__placeholder__.js");
: getPlaceholderPath(cwd);
const config = configs.getConfig(filePath);

// Most files use the same config, so cache it.
Expand Down Expand Up @@ -438,7 +448,7 @@ function verifyText({
* `config.extractConfig(filePath)` requires an absolute path, but `linter`
* doesn't know CWD, so it gives `linter` an absolute path always.
*/
const filePathToVerify = filePath === "<text>" ? path.join(cwd, "__placeholder__.js") : filePath;
const filePathToVerify = filePath === "<text>" ? getPlaceholderPath(cwd) : filePath;
const { fixed, messages, output } = linter.verifyAndFix(
text,
configs,
Expand Down Expand Up @@ -535,6 +545,14 @@ function *iterateRuleDeprecationWarnings(configs) {
}
}

/**
* Creates an error to be thrown when an array of results passed to `getRulesMetaForResults` was not created by the current engine.
* @returns {TypeError} An error object.
*/
function createExtraneousResultsError() {
return new TypeError("Results object was not created from this ESLint instance.");
}

//-----------------------------------------------------------------------------
// Main API
//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -672,7 +690,10 @@ class FlatESLint {
return resultRules;
}

const { configs } = privateMembers.get(this);
const {
configs,
options: { cwd }
} = privateMembers.get(this);

/*
* We can only accurately return rules meta information for linting results if the
Expand All @@ -681,7 +702,7 @@ class FlatESLint {
* to let the user know we can't do anything here.
*/
if (!configs) {
throw new TypeError("Results object was not created from this ESLint instance.");
throw createExtraneousResultsError();
}

for (const result of results) {
Expand All @@ -690,16 +711,20 @@ class FlatESLint {
* Normalize filename for <text>.
*/
const filePath = result.filePath === "<text>"
? "__placeholder__.js" : result.filePath;

/*
* All of the plugin and rule information is contained within the
* calculated config for the given file.
*/
const config = configs.getConfig(filePath);
? getPlaceholderPath(cwd) : result.filePath;
const allMessages = result.messages.concat(result.suppressedMessages);

for (const { ruleId } of allMessages) {

/*
* All of the plugin and rule information is contained within the
* calculated config for the given file.
*/
const config = configs.getConfig(filePath);

if (!config) {
throw createExtraneousResultsError();
}
const rule = getRuleFromConfig(ruleId, config);

// ensure the rule exists
Expand Down Expand Up @@ -1037,7 +1062,7 @@ class FlatESLint {
const npmFormat = naming.normalizePackageName(normalizedFormatName, "eslint-formatter");

// TODO: This is pretty dirty...would be nice to clean up at some point.
formatterPath = ModuleResolver.resolve(npmFormat, path.join(cwd, "__placeholder__.js"));
formatterPath = ModuleResolver.resolve(npmFormat, getPlaceholderPath(cwd));
} catch {
formatterPath = path.resolve(__dirname, "../", "cli-engine", "formatters", `${normalizedFormatName}.js`);
}
Expand Down
57 changes: 55 additions & 2 deletions tests/lib/eslint/flat-eslint.js
Expand Up @@ -3679,7 +3679,7 @@ describe("FlatESLint", () => {

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

it("should throw an error when results were not created from this instance", async () => {
it("should throw an error when this instance did not lint any files", async () => {
const engine = new FlatESLint({
overrideConfigFile: true
});
Expand Down Expand Up @@ -3716,7 +3716,42 @@ describe("FlatESLint", () => {
"var err = doStuff();\nif (err) console.log('failed tests: ' + err);\nprocess.exit(1);\n"
}
]);
}, /Results object was not created from this ESLint instance/u);
}, {
constructor: TypeError,
message: "Results object was not created from this ESLint instance."
});
});

it("should throw an error when results were created from a different instance", async () => {
const engine1 = new FlatESLint({
overrideConfigFile: true,
cwd: path.join(fixtureDir, "foo"),
overrideConfig: {
rules: {
semi: 2
}
}
});
const engine2 = new FlatESLint({
overrideConfigFile: true,
cwd: path.join(fixtureDir, "bar"),
overrideConfig: {
rules: {
semi: 2
}
}
});

const results1 = await engine1.lintText("1", { filePath: "file.js" });
const results2 = await engine2.lintText("2", { filePath: "file.js" });

engine1.getRulesMetaForResults(results1); // should not throw an error
assert.throws(() => {
engine1.getRulesMetaForResults(results2);
}, {
constructor: TypeError,
message: "Results object was not created from this ESLint instance."
});
});

it("should return empty object when there are no linting errors", async () => {
Expand Down Expand Up @@ -3789,6 +3824,24 @@ describe("FlatESLint", () => {
nodePlugin.rules["no-new-require"].meta
);
});

it("should treat a result without `filePath` as if the file was located in `cwd`", async () => {
const engine = new FlatESLint({
overrideConfigFile: true,
cwd: path.join(fixtureDir, "foo", "bar"),
ignorePatterns: "*/**", // ignore all subdirectories of `cwd`
overrideConfig: {
rules: {
eqeqeq: "warn"
}
}
});

const results = await engine.lintText("a==b");
const rulesMeta = engine.getRulesMetaForResults(results);

assert.deepStrictEqual(rulesMeta.eqeqeq, coreRules.get("eqeqeq").meta);
});
});

describe("outputFixes()", () => {
Expand Down

0 comments on commit 56cc899

Please sign in to comment.