Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fasttime committed Oct 31, 2022
1 parent 319f0a5 commit 886a038
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 14 deletions.
48 changes: 36 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 @@ -422,7 +432,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 @@ -519,6 +529,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 @@ -655,7 +673,10 @@ class FlatESLint {
}

const resultRules = new Map();
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 @@ -664,7 +685,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 @@ -673,20 +694,23 @@ 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) {
if (!ruleId) {
continue;
}

/*
* 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 @@ -1024,7 +1048,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
96 changes: 94 additions & 2 deletions tests/lib/eslint/flat-eslint.js
Expand Up @@ -3752,7 +3752,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 @@ -3789,7 +3789,99 @@ 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 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);
});

it("should not throw an error if a result without `filePath` contains an ignored file warning", async () => {
const engine = new FlatESLint({
overrideConfigFile: true,
cwd: path.join(fixtureDir, "foo", "bar"),
ignorePatterns: "**"
});

const results = await engine.lintText("", { warnIgnored: true });
const rulesMeta = engine.getRulesMetaForResults(results);

assert.deepStrictEqual(rulesMeta, {});
});

it("should not throw an error if results contain linted files and one ignored file", async () => {
const engine = new FlatESLint({
overrideConfigFile: true,
cwd: getFixturePath(),
ignorePatterns: "passing*",
overrideConfig: {
rules: {
"no-undef": 2,
semi: 1
}
}
});

const results = await engine.lintFiles(["missing-semicolon.js", "passing.js", "undef.js"]);

assert(
results.some(({ messages }) => messages.some(({ message, ruleId }) => !ruleId && message.startsWith("File ignored"))),
"At least one file should be ignored but none is."
);

const rulesMeta = engine.getRulesMetaForResults(results);

assert.deepStrictEqual(rulesMeta["no-undef"], coreRules.get("no-undef").meta);
assert.deepStrictEqual(rulesMeta.semi, coreRules.get("semi").meta);
});

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

0 comments on commit 886a038

Please sign in to comment.