New: noInlineConfig setting (refs eslint/rfcs#22) (#12091)

* New: noInlineConfig setting (refs eslint/rfcs#22)

* fix typo

Co-Authored-By: Kevin Partington <platinum.azure@kernelpanicstudios.com>

* fix typo in tests

* add tests for line comments

* add config name

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
 };

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,

lib/cli-engine/config-array/config-array.js

@@ -54,6 +54,7 @@ const { ExtractedConfig } = require("./extracted-config");
  * @property {InstanceType<OverrideTester>|null} criteria The tester for the `files` and `excludedFiles` of this config element.
  * @property {Record<string, boolean>|undefined} env The environment settings.
  * @property {Record<string, GlobalConf>|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<string, DependentPlugin>|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);

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<string, boolean>}
@@ -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;
 

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<VerifyOptions>} Normalized options
+ * @param {ConfigData} config Config.
+ * @returns {Required<VerifyOptions> & { 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 || "<input>"),
-        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

lib/shared/types.js

@@ -30,6 +30,7 @@ module.exports = {};
  * @property {Record<string, boolean>} [env] The environment settings.
  * @property {string | string[]} [extends] The path to other config files or the package name of shareable configs.
  * @property {Record<string, GlobalConf>} [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<string, GlobalConf>} [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.

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", () => {

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: [],

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: {