diff --git a/conf/config-schema.js b/conf/config-schema.js index 36f3c27de60..d89bf0f58c5 100644 --- a/conf/config-schema.js +++ b/conf/config-schema.js @@ -20,6 +20,7 @@ const baseConfigProperties = { processor: { type: "string" }, rules: { type: "object" }, settings: { type: "object" }, + noInlineConfig: { type: "boolean" }, ecmaFeatures: { type: "object" } // deprecated; logs a warning when used }; diff --git a/lib/cli-engine/config-array-factory.js b/lib/cli-engine/config-array-factory.js index 95430c358df..0b2ed07b6a0 100644 --- a/lib/cli-engine/config-array-factory.js +++ b/lib/cli-engine/config-array-factory.js @@ -526,6 +526,7 @@ class ConfigArrayFactory { env, extends: extend, globals, + noInlineConfig, parser: parserName, parserOptions, plugins: pluginList, @@ -567,6 +568,7 @@ class ConfigArrayFactory { criteria: null, env, globals, + noInlineConfig, parser, parserOptions, plugins, diff --git a/lib/cli-engine/config-array/config-array.js b/lib/cli-engine/config-array/config-array.js index 5c7aaa33400..0859868d824 100644 --- a/lib/cli-engine/config-array/config-array.js +++ b/lib/cli-engine/config-array/config-array.js @@ -54,6 +54,7 @@ const { ExtractedConfig } = require("./extracted-config"); * @property {InstanceType|null} criteria The tester for the `files` and `excludedFiles` of this config element. * @property {Record|undefined} env The environment settings. * @property {Record|undefined} globals The global variable settings. + * @property {boolean|undefined} noInlineConfig The flag that disables directive comments. * @property {DependentParser|undefined} parser The parser loader. * @property {Object|undefined} parserOptions The parser options. * @property {Record|undefined} plugins The plugin loaders. @@ -250,6 +251,12 @@ function createConfig(instance, indices) { config.processor = element.processor; } + // Adopt the noInlineConfig which was found at first. + if (config.noInlineConfig === void 0 && element.noInlineConfig !== void 0) { + config.noInlineConfig = element.noInlineConfig; + config.configNameOfNoInlineConfig = element.name; + } + // Merge others. mergeWithoutOverwrite(config.env, element.env); mergeWithoutOverwrite(config.globals, element.globals); diff --git a/lib/cli-engine/config-array/extracted-config.js b/lib/cli-engine/config-array/extracted-config.js index 377cc0fa915..53208c16e46 100644 --- a/lib/cli-engine/config-array/extracted-config.js +++ b/lib/cli-engine/config-array/extracted-config.js @@ -29,6 +29,12 @@ class ExtractedConfig { constructor() { + /** + * The config name what `noInlineConfig` setting came from. + * @type {string} + */ + this.configNameOfNoInlineConfig = ""; + /** * Environments. * @type {Record} @@ -41,6 +47,12 @@ class ExtractedConfig { */ this.globals = {}; + /** + * The flag that disables directive comments. + * @type {boolean|undefined} + */ + this.noInlineConfig = void 0; + /** * Parser definition. * @type {DependentParser|null} @@ -84,7 +96,10 @@ class ExtractedConfig { */ toCompatibleObjectAsConfigFileContent() { const { - processor: _ignore, // eslint-disable-line no-unused-vars + /* eslint-disable no-unused-vars */ + configNameOfNoInlineConfig: _ignore1, + processor: _ignore2, + /* eslint-enable no-unused-vars */ ...config } = this; diff --git a/lib/linter/linter.js b/lib/linter/linter.js index a49d850859b..d367cef6cb4 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -198,14 +198,20 @@ function createMissingRuleMessage(ruleId) { /** * creates a linting problem * @param {Object} options to create linting error - * @param {string} options.ruleId the ruleId to report - * @param {Object} options.loc the loc to report - * @param {string} options.message the error message to report - * @returns {Problem} created problem, returns a missing-rule problem if only provided ruleId. + * @param {string} [options.ruleId] the ruleId to report + * @param {Object} [options.loc] the loc to report + * @param {string} [options.message] the error message to report + * @param {string} [options.severity] the error message to report + * @returns {LintMessage} created problem, returns a missing-rule problem if only provided ruleId. * @private */ function createLintingProblem(options) { - const { ruleId, loc = DEFAULT_ERROR_LOC, message = createMissingRuleMessage(options.ruleId) } = options; + const { + ruleId = null, + loc = DEFAULT_ERROR_LOC, + message = createMissingRuleMessage(options.ruleId), + severity = 2 + } = options; return { ruleId, @@ -214,7 +220,7 @@ function createLintingProblem(options) { column: loc.start.column + 1, endLine: loc.end.line, endColumn: loc.end.column + 1, - severity: 2, + severity, nodeType: null }; } @@ -257,10 +263,11 @@ function createDisableDirectives(options) { * @param {string} filename The file being checked. * @param {ASTNode} ast The top node of the AST. * @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules + * @param {string|null} warnInlineConfig If a string then it should warn directive comments as disabled. The string value is the config name what the setting came from. * @returns {{configuredRules: Object, enabledGlobals: {value:string,comment:Token}[], exportedVariables: Object, problems: Problem[], disableDirectives: DisableDirective[]}} * A collection of the directive comments that were found, along with any problems that occurred when parsing */ -function getDirectiveComments(filename, ast, ruleMapper) { +function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) { const configuredRules = {}; const enabledGlobals = Object.create(null); const exportedVariables = {}; @@ -269,16 +276,29 @@ function getDirectiveComments(filename, ast, ruleMapper) { ast.comments.filter(token => token.type !== "Shebang").forEach(comment => { const trimmedCommentText = comment.value.trim(); - const match = /^(eslint(-\w+){0,3}|exported|globals?)(\s|$)/u.exec(trimmedCommentText); + const match = /^(eslint(?:-env|-enable|-disable(?:(?:-next)?-line)?)?|exported|globals?)(?:\s|$)/u.exec(trimmedCommentText); if (!match) { return; } + const lineCommentSupported = /^eslint-disable-(next-)?line$/u.test(match[1]); + + if (warnInlineConfig && (lineCommentSupported || comment.type === "Block")) { + const kind = comment.type === "Block" ? `/*${match[1]}*/` : `//${match[1]}`; + + problems.push(createLintingProblem({ + ruleId: null, + message: `'${kind}' has no effect because you have 'noInlineConfig' setting in ${warnInlineConfig}.`, + loc: comment.loc, + severity: 1 + })); + return; + } const directiveValue = trimmedCommentText.slice(match.index + match[1].length); let directiveType = ""; - if (/^eslint-disable-(next-)?line$/u.test(match[1])) { + if (lineCommentSupported) { if (comment.loc.start.line === comment.loc.end.line) { directiveType = match[1].slice("eslint-".length); } else { @@ -441,16 +461,27 @@ function normalizeFilename(filename) { return index === -1 ? filename : parts.slice(index).join(path.sep); } +// eslint-disable-next-line valid-jsdoc /** * Normalizes the possible options for `linter.verify` and `linter.verifyAndFix` to a * consistent shape. * @param {VerifyOptions} providedOptions Options - * @returns {Required} Normalized options + * @param {ConfigData} config Config. + * @returns {Required & { warnInlineConfig: string|null }} Normalized options */ -function normalizeVerifyOptions(providedOptions) { +function normalizeVerifyOptions(providedOptions, config) { + const disableInlineConfig = config.noInlineConfig === true; + const ignoreInlineConfig = providedOptions.allowInlineConfig === false; + const configNameOfNoInlineConfig = config.configNameOfNoInlineConfig + ? ` (${config.configNameOfNoInlineConfig})` + : ""; + return { filename: normalizeFilename(providedOptions.filename || ""), - allowInlineConfig: providedOptions.allowInlineConfig !== false, + allowInlineConfig: !ignoreInlineConfig, + warnInlineConfig: disableInlineConfig && !ignoreInlineConfig + ? `your config${configNameOfNoInlineConfig}` + : null, reportUnusedDisableDirectives: Boolean(providedOptions.reportUnusedDisableDirectives), disableFixes: Boolean(providedOptions.disableFixes) }; @@ -984,7 +1015,7 @@ class Linter { _verifyWithoutProcessors(textOrSourceCode, providedConfig, providedOptions) { const slots = internalSlotsMap.get(this); const config = providedConfig || {}; - const options = normalizeVerifyOptions(providedOptions); + const options = normalizeVerifyOptions(providedOptions, config); let text; // evaluate arguments @@ -1019,7 +1050,9 @@ class Linter { } // search and apply "eslint-env *". - const envInFile = findEslintEnv(text); + const envInFile = options.allowInlineConfig && !options.warnInlineConfig + ? findEslintEnv(text) + : {}; const resolvedEnvConfig = Object.assign({ builtin: true }, config.env, envInFile); const enabledEnvs = Object.keys(resolvedEnvConfig) .filter(envName => resolvedEnvConfig[envName]) @@ -1062,7 +1095,7 @@ class Linter { const sourceCode = slots.lastSourceCode; const commentDirectives = options.allowInlineConfig - ? getDirectiveComments(options.filename, sourceCode.ast, ruleId => getRule(slots, ruleId)) + ? getDirectiveComments(options.filename, sourceCode.ast, ruleId => getRule(slots, ruleId), options.warnInlineConfig) : { configuredRules: {}, enabledGlobals: {}, exportedVariables: {}, problems: [], disableDirectives: [] }; // augment global scope with declared global variables diff --git a/lib/shared/types.js b/lib/shared/types.js index d8357799944..8a889d21db9 100644 --- a/lib/shared/types.js +++ b/lib/shared/types.js @@ -30,6 +30,7 @@ module.exports = {}; * @property {Record} [env] The environment settings. * @property {string | string[]} [extends] The path to other config files or the package name of shareable configs. * @property {Record} [globals] The global variable settings. + * @property {boolean} [noInlineConfig] The flag that disables directive comments. * @property {OverrideConfigData[]} [overrides] The override settings per kind of files. * @property {string} [parser] The path to a parser or the package name of a parser. * @property {ParserOptions} [parserOptions] The parser options. @@ -47,6 +48,7 @@ module.exports = {}; * @property {string | string[]} [extends] The path to other config files or the package name of shareable configs. * @property {string | string[]} files The glob pattarns for target files. * @property {Record} [globals] The global variable settings. + * @property {boolean} [noInlineConfig] The flag that disables directive comments. * @property {OverrideConfigData[]} [overrides] The override settings per kind of files. * @property {string} [parser] The path to a parser or the package name of a parser. * @property {ParserOptions} [parserOptions] The parser options. diff --git a/tests/lib/cli-engine/cli-engine.js b/tests/lib/cli-engine/cli-engine.js index 427181affce..dc42cfa6828 100644 --- a/tests/lib/cli-engine/cli-engine.js +++ b/tests/lib/cli-engine/cli-engine.js @@ -3495,6 +3495,45 @@ describe("CLIEngine", () => { assert.deepStrictEqual(filenames, ["a.js", "b.js"]); }); }); + + describe("with 'noInlineConfig' setting", () => { + const root = getFixturePath("cli-engine/noInlineConfig"); + + it("should warn directive comments if 'noInlineConfig' was given.", () => { + CLIEngine = defineCLIEngineWithInMemoryFileSystem({ + cwd: () => root, + files: { + "test.js": "/* globals foo */", + ".eslintrc.yml": "noInlineConfig: true" + } + }).CLIEngine; + engine = new CLIEngine(); + + const { results } = engine.executeOnFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 1); + assert.strictEqual(messages[0].message, "'/*globals*/' has no effect because you have 'noInlineConfig' setting in your config (.eslintrc.yml)."); + }); + + it("should show the config file what the 'noInlineConfig' came from.", () => { + CLIEngine = defineCLIEngineWithInMemoryFileSystem({ + cwd: () => root, + files: { + "node_modules/eslint-config-foo/index.js": "module.exports = {noInlineConfig: true}", + "test.js": "/* globals foo */", + ".eslintrc.yml": "extends: foo" + } + }).CLIEngine; + engine = new CLIEngine(); + + const { results } = engine.executeOnFiles(["test.js"]); + const messages = results[0].messages; + + assert.strictEqual(messages.length, 1); + assert.strictEqual(messages[0].message, "'/*globals*/' has no effect because you have 'noInlineConfig' setting in your config (.eslintrc.yml ยป eslint-config-foo)."); + }); + }); }); describe("getConfigForFile", () => { diff --git a/tests/lib/cli-engine/config-array-factory.js b/tests/lib/cli-engine/config-array-factory.js index fe5f9de8440..b9f404dca1d 100644 --- a/tests/lib/cli-engine/config-array-factory.js +++ b/tests/lib/cli-engine/config-array-factory.js @@ -30,6 +30,7 @@ function assertConfigArrayElement(actual, providedExpected) { criteria: null, env: void 0, globals: void 0, + noInlineConfig: void 0, parser: void 0, parserOptions: void 0, plugins: void 0, @@ -53,6 +54,7 @@ function assertConfig(actual, providedExpected) { const expected = { env: {}, globals: {}, + noInlineConfig: void 0, parser: null, parserOptions: {}, plugins: [], diff --git a/tests/lib/cli-engine/config-array/config-array.js b/tests/lib/cli-engine/config-array/config-array.js index 07cf0f2c661..82f3e750729 100644 --- a/tests/lib/cli-engine/config-array/config-array.js +++ b/tests/lib/cli-engine/config-array/config-array.js @@ -423,8 +423,10 @@ describe("ConfigArray", () => { const result = merge(config[0], config[1]); assert.deepStrictEqual(result, { + configNameOfNoInlineConfig: "", env: {}, globals: {}, + noInlineConfig: void 0, parser: null, parserOptions: { ecmaFeatures: { @@ -452,8 +454,10 @@ describe("ConfigArray", () => { const result = merge(config[0], config[1]); assert.deepStrictEqual(result, { + configNameOfNoInlineConfig: "", env: {}, globals: {}, + noInlineConfig: void 0, parser: null, parserOptions: { ecmaFeatures: { @@ -565,6 +569,7 @@ describe("ConfigArray", () => { const result = merge(config[0], config[1]); assert.deepStrictEqual(result, { + configNameOfNoInlineConfig: "", parser: null, parserOptions: { ecmaFeatures: { @@ -601,7 +606,8 @@ describe("ConfigArray", () => { "valid-jsdoc": [2] }, settings: {}, - processor: null + processor: null, + noInlineConfig: void 0 }); assert.deepStrictEqual(config[0], { rules: { diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 3cfef3bbe4c..b1b621935d7 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -74,7 +74,7 @@ const ESLINT_ENV = "eslint-env"; describe("Linter", () => { const filename = "filename.js"; - /** @type {InstanceType} */ + /** @type {InstanceType} */ let linter; beforeEach(() => { @@ -3134,37 +3134,64 @@ describe("Linter", () => { it("should report a violation for env changes", () => { const code = [ - `/*${ESLINT_ENV} browser*/` + `/*${ESLINT_ENV} browser*/ window` ].join("\n"); const config = { rules: { - test: 2 + "no-undef": 2 } }; - let ok = false; + const messages = linter.verify(code, config, { allowInlineConfig: false }); - linter.defineRules({ - test(context) { - return { - Program() { - const scope = context.getScope(); - const sourceCode = context.getSourceCode(); - const comments = sourceCode.getAllComments(); - - assert.strictEqual(1, comments.length); - - const windowVar = getVariable(scope, "window"); + assert.strictEqual(messages.length, 1); + assert.strictEqual(messages[0].ruleId, "no-undef"); + }); + }); - assert.notOk(windowVar.eslintExplicitGlobal); + describe("when evaluating code with 'noInlineComment'", () => { + for (const directive of [ + "globals foo", + "global foo", + "exported foo", + "eslint eqeqeq: error", + "eslint-disable eqeqeq", + "eslint-disable-line eqeqeq", + "eslint-disable-next-line eqeqeq", + "eslint-enable eqeqeq", + "eslint-env es6" + ]) { + // eslint-disable-next-line no-loop-func + it(`should warn '/* ${directive} */' if 'noInlineConfig' was given.`, () => { + const messages = linter.verify(`/* ${directive} */`, { noInlineConfig: true }); + + assert.deepStrictEqual(messages.length, 1); + assert.deepStrictEqual(messages[0].fatal, void 0); + assert.deepStrictEqual(messages[0].ruleId, null); + assert.deepStrictEqual(messages[0].severity, 1); + assert.deepStrictEqual(messages[0].message, `'/*${directive.split(" ")[0]}*/' has no effect because you have 'noInlineConfig' setting in your config.`); + }); + } - ok = true; - } - }; - } + for (const directive of [ + "eslint-disable-line eqeqeq", + "eslint-disable-next-line eqeqeq" + ]) { + // eslint-disable-next-line no-loop-func + it(`should warn '// ${directive}' if 'noInlineConfig' was given.`, () => { + const messages = linter.verify(`// ${directive}`, { noInlineConfig: true }); + + assert.deepStrictEqual(messages.length, 1); + assert.deepStrictEqual(messages[0].fatal, void 0); + assert.deepStrictEqual(messages[0].ruleId, null); + assert.deepStrictEqual(messages[0].severity, 1); + assert.deepStrictEqual(messages[0].message, `'//${directive.split(" ")[0]}' has no effect because you have 'noInlineConfig' setting in your config.`); }); + } - linter.verify(code, config, { allowInlineConfig: false }); - assert(ok); + it("should not warn if 'noInlineConfig' and '--no-inline-config' were given.", () => { + const messages = linter.verify("/* globals foo */", { noInlineConfig: true }, { allowInlineConfig: false }); + + assert.deepStrictEqual(messages.length, 0); }); });