Skip to content

Commit

Permalink
Breaking: lint overrides files (fixes #10828, refs eslint/rfcs#20)
Browse files Browse the repository at this point in the history
  • Loading branch information
mysticatea committed Dec 17, 2019
1 parent 257f3d6 commit 79785b4
Show file tree
Hide file tree
Showing 10 changed files with 391 additions and 40 deletions.
2 changes: 1 addition & 1 deletion conf/default-cli-options.js
Expand Up @@ -12,7 +12,7 @@ module.exports = {
useEslintrc: true,
envs: [],
globals: [],
extensions: [".js"],
extensions: null,
ignore: true,
ignorePath: void 0,
cache: false,
Expand Down
1 change: 1 addition & 0 deletions lib/cli-engine/cascading-config-array-factory.js
Expand Up @@ -105,6 +105,7 @@ function createBaseConfigArray({
*/
if (rulePaths && rulePaths.length > 0) {
baseConfigArray.push({
type: "config",
name: "--rulesdir",
filePath: "",
plugins: {
Expand Down
16 changes: 7 additions & 9 deletions lib/cli-engine/cli-engine.js
Expand Up @@ -57,7 +57,7 @@ const validFixTypes = new Set(["problem", "suggestion", "layout"]);
* @property {string} configFile The configuration file to use.
* @property {string} cwd The value to use for the current working directory.
* @property {string[]} envs An array of environments to load.
* @property {string[]} extensions An array of file extensions to check.
* @property {string[]|null} extensions An array of file extensions to check.
* @property {boolean|Function} fix Execute in autofix mode. If a function, should return a boolean.
* @property {string[]} fixTypes Array of rule types to apply fixes for.
* @property {string[]} globals An array of global variables to declare.
Expand Down Expand Up @@ -201,7 +201,7 @@ function calculateStatsPerRun(results) {
* @param {boolean} config.fix If `true` then it does fix.
* @param {boolean} config.allowInlineConfig If `true` then it uses directive comments.
* @param {boolean} config.reportUnusedDisableDirectives If `true` then it reports unused `eslint-disable` comments.
* @param {RegExp} config.extensionRegExp The `RegExp` object that tests if a file path has the allowed file extensions.
* @param {FileEnumerator} config.fileEnumerator The file enumerator to check if a path is a target or not.
* @param {Linter} config.linter The linter instance to verify.
* @returns {LintResult} The result of linting.
* @private
Expand All @@ -214,7 +214,7 @@ function verifyText({
fix,
allowInlineConfig,
reportUnusedDisableDirectives,
extensionRegExp,
fileEnumerator,
linter
}) {
const filePath = providedFilePath || "<text>";
Expand All @@ -238,13 +238,11 @@ function verifyText({

/**
* Check if the linter should adopt a given code block or not.
* Currently, the linter adopts code blocks if the name matches `--ext` option.
* In the future, `overrides` in the configuration would affect the adoption (https://github.com/eslint/rfcs/pull/20).
* @param {string} blockFilename The virtual filename of a code block.
* @returns {boolean} `true` if the linter should adopt the code block.
*/
filterCodeBlock(blockFilename) {
return extensionRegExp.test(blockFilename);
return fileEnumerator.isTargetPath(blockFilename);
}
}
);
Expand Down Expand Up @@ -703,7 +701,7 @@ class CLIEngine {
return patterns.filter(Boolean);
}

const extensions = options.extensions.map(ext => ext.replace(/^\./u, ""));
const extensions = (options.extensions || [".js"]).map(ext => ext.replace(/^\./u, ""));
const dirSuffix = `/**/*.{${extensions.join(",")}}`;

return patterns.filter(Boolean).map(pathname => {
Expand Down Expand Up @@ -802,7 +800,7 @@ class CLIEngine {
fix,
allowInlineConfig,
reportUnusedDisableDirectives,
extensionRegExp: fileEnumerator.extensionRegExp,
fileEnumerator,
linter
});

Expand Down Expand Up @@ -890,7 +888,7 @@ class CLIEngine {
fix,
allowInlineConfig,
reportUnusedDisableDirectives,
extensionRegExp: fileEnumerator.extensionRegExp,
fileEnumerator,
linter
}));
}
Expand Down
5 changes: 4 additions & 1 deletion lib/cli-engine/config-array-factory.js
Expand Up @@ -540,7 +540,7 @@ class ConfigArrayFactory {
*/
*_normalizeESLintIgnoreData(ignorePatterns, filePath, name) {
const elements = this._normalizeObjectConfigData(
{ ignorePatterns },
{ type: "ignore", ignorePatterns },
filePath,
name
);
Expand Down Expand Up @@ -642,6 +642,7 @@ class ConfigArrayFactory {
root,
rules,
settings,
type = "config",
overrides: overrideList = []
},
filePath,
Expand Down Expand Up @@ -673,6 +674,7 @@ class ConfigArrayFactory {
yield {

// Debug information.
type,
name,
filePath,

Expand Down Expand Up @@ -1022,6 +1024,7 @@ class ConfigArrayFactory {
if (processorId.startsWith(".")) {
yield* this._normalizeObjectConfigData(
{
type: "implicit-processor",
files: [`*${processorId}`],
processor: `${pluginId}/${processorId}`
},
Expand Down
19 changes: 19 additions & 0 deletions lib/cli-engine/config-array/config-array.js
Expand Up @@ -454,6 +454,25 @@ class ConfigArray extends Array {

return cache.get(cacheKey);
}

/**
* Check if a given path is an additional lint target.
* @param {string} filePath The absolute path to the target file.
* @returns {boolean} `true` if the file is an additional lint target.
*/
isAdditionalTargetPath(filePath) {
for (const { criteria, type } of this) {
if (
type === "config" &&
criteria &&
!criteria.endsWithWildcard &&
criteria.test(filePath)
) {
return true;
}
}
return false;
}
}

const exportObject = {
Expand Down
44 changes: 36 additions & 8 deletions lib/cli-engine/config-array/override-tester.js
Expand Up @@ -96,14 +96,22 @@ class OverrideTester {
static create(files, excludedFiles, basePath) {
const includePatterns = normalizePatterns(files);
const excludePatterns = normalizePatterns(excludedFiles);
const allPatterns = includePatterns.concat(excludePatterns);
let endsWithWildcard = false;

if (allPatterns.length === 0) {
if (includePatterns.length === 0) {
return null;
}

// Rejects absolute paths or relative paths to parents.
for (const pattern of allPatterns) {
for (const pattern of includePatterns) {
if (path.isAbsolute(pattern) || pattern.includes("..")) {
throw new Error(`Invalid override pattern (expected relative path not containing '..'): ${pattern}`);
}
if (pattern.endsWith("*")) {
endsWithWildcard = true;
}
}
for (const pattern of excludePatterns) {
if (path.isAbsolute(pattern) || pattern.includes("..")) {
throw new Error(`Invalid override pattern (expected relative path not containing '..'): ${pattern}`);
}
Expand All @@ -112,7 +120,11 @@ class OverrideTester {
const includes = toMatcher(includePatterns);
const excludes = toMatcher(excludePatterns);

return new OverrideTester([{ includes, excludes }], basePath);
return new OverrideTester(
[{ includes, excludes }],
basePath,
endsWithWildcard
);
}

/**
Expand All @@ -125,28 +137,44 @@ class OverrideTester {
*/
static and(a, b) {
if (!b) {
return a && new OverrideTester(a.patterns, a.basePath);
return a && new OverrideTester(
a.patterns,
a.basePath,
a.endsWithWildcard
);
}
if (!a) {
return new OverrideTester(b.patterns, b.basePath);
return new OverrideTester(
b.patterns,
b.basePath,
b.endsWithWildcard
);
}

assert.strictEqual(a.basePath, b.basePath);
return new OverrideTester(a.patterns.concat(b.patterns), a.basePath);
return new OverrideTester(
a.patterns.concat(b.patterns),
a.basePath,
a.endsWithWildcard || b.endsWithWildcard
);
}

/**
* Initialize this instance.
* @param {Pattern[]} patterns The matchers.
* @param {string} basePath The base path.
* @param {boolean} endsWithWildcard If `true` then a pattern ends with `*`.
*/
constructor(patterns, basePath) {
constructor(patterns, basePath, endsWithWildcard = false) {

/** @type {Pattern[]} */
this.patterns = patterns;

/** @type {string} */
this.basePath = basePath;

/** @type {boolean} */
this.endsWithWildcard = endsWithWildcard;
}

/**
Expand Down
79 changes: 58 additions & 21 deletions lib/cli-engine/file-enumerator.js
Expand Up @@ -88,7 +88,7 @@ const IGNORED = 2;
* @typedef {Object} FileEnumeratorInternalSlots
* @property {CascadingConfigArrayFactory} configArrayFactory The factory for config arrays.
* @property {string} cwd The base directory to start lookup.
* @property {RegExp} extensionRegExp The RegExp to test if a string ends with specific file extensions.
* @property {RegExp|null} extensionRegExp The RegExp to test if a string ends with specific file extensions.
* @property {boolean} globInputPaths Set to false to skip glob resolution of input file paths to lint (default: true). If false, each input file paths is assumed to be a non-glob path to an existing file.
* @property {boolean} ignoreFlag The flag to check ignored files.
* @property {(filePath:string, dot:boolean) => boolean} defaultIgnores The default predicate function to ignore files.
Expand Down Expand Up @@ -142,6 +142,27 @@ function readdirSafeSync(directoryPath) {
}
}

/**
* Create a `RegExp` object to detect extensions.
* @param {string[] | null} extensions The extensions to create.
* @returns {RegExp | null} The created `RegExp` object or null.
*/
function createExtensionRegExp(extensions) {
if (extensions) {
const normalizedExts = extensions.map(ext => escapeRegExp(
ext.startsWith(".")
? ext.slice(1)
: ext
));

return new RegExp(
`.\\.(?:${normalizedExts.join("|")})$`,
"u"
);
}
return null;
}

/**
* The error type when no files match a glob.
*/
Expand Down Expand Up @@ -188,36 +209,51 @@ class FileEnumerator {
constructor({
cwd = process.cwd(),
configArrayFactory = new CascadingConfigArrayFactory({ cwd }),
extensions = [".js"],
extensions = null,
globInputPaths = true,
ignore = true
} = {}) {
internalSlotsMap.set(this, {
configArrayFactory,
cwd,
defaultIgnores: IgnorePattern.createDefaultIgnore(cwd),
extensionRegExp: new RegExp(
`.\\.(?:${extensions
.map(ext => escapeRegExp(
ext.startsWith(".")
? ext.slice(1)
: ext
))
.join("|")
})$`,
"u"
),
extensionRegExp: createExtensionRegExp(extensions),
globInputPaths,
ignoreFlag: ignore
});
}

/**
* The `RegExp` object that tests if a file path has the allowed file extensions.
* @type {RegExp}
* Check if a given file is target or not.
* @param {string} filePath The path to a candidate file.
* @param {ConfigArray} [providedConfig] Optional. The configuration for the file.
* @returns {boolean} `true` if the file is a target.
*/
get extensionRegExp() {
return internalSlotsMap.get(this).extensionRegExp;
isTargetPath(filePath, providedConfig) {
const {
configArrayFactory,
extensionRegExp
} = internalSlotsMap.get(this);

// If `--ext` option is present, use it.
if (extensionRegExp) {
return extensionRegExp.test(filePath);
}

// `.js` file is target by default.
if (filePath.endsWith(".js")) {
return true;
}

// use `overrides[].files` to check additional targets.
const config =
providedConfig ||
configArrayFactory.getConfigArrayForFile(
filePath,
{ ignoreNotFoundError: true }
);

return config.isAdditionalTargetPath(filePath);
}

/**
Expand Down Expand Up @@ -376,7 +412,7 @@ class FileEnumerator {
*/
*_iterateFilesRecursive(directoryPath, options) {
debug(`Enter the directory: ${directoryPath}`);
const { configArrayFactory, extensionRegExp } = internalSlotsMap.get(this);
const { configArrayFactory } = internalSlotsMap.get(this);

/** @type {ConfigArray|null} */
let config = null;
Expand All @@ -400,17 +436,18 @@ class FileEnumerator {
{ ignoreNotFoundError: true }
);
}
const ignored = this._isIgnoredFile(filePath, { ...options, config });
const flag = ignored ? IGNORED_SILENTLY : NONE;
const matched = options.selector

// Started with a glob pattern; choose by the pattern.
? options.selector.match(filePath)

// Started with a directory path; choose by file extensions.
: extensionRegExp.test(filePath);
: this.isTargetPath(filePath, config);

if (matched) {
const ignored = this._isIgnoredFile(filePath, { ...options, config });
const flag = ignored ? IGNORED_SILENTLY : NONE;

debug(`Yield: ${filename}${ignored ? " but ignored" : ""}`);
yield {
config: configArrayFactory.getConfigArrayForFile(filePath),
Expand Down

0 comments on commit 79785b4

Please sign in to comment.