Repository: eslint/eslint
Pull request #8532 (2017-05-01 16:16:37)

Fix: max-len does not allow comments longer than code when comment length is specified
**What is the purpose of this pull request? (put an "X" next to item)**

[ ] Documentation update
[X] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
[ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
[ ] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->
**Tell us about your environment**

* **ESLint Version:** 3.19.0
* **Node Version:** 6.9.3
* **npm Version:** 3.10.10

**What parser (default, Babel-ESLint, etc.) are you using?**
babel-eslint

**Please show your full configuration:**
```
		"max-len": ["error", {
			"code": 80,
			"comments": 100,
			"tabWidth": 2,
			"ignoreUrls": true,
			"ignoreRegExpLiterals": true,
			"ignoreStrings": true,
			"ignoreTemplateLiterals": true
		}],
```
**What did you do? Please include the actual source code causing the issue.**
I wanted to allow comments up to 100 characters while limiting code to 80 characters.
In one of my modules, I have a comment longer than 80 characters, but less than the 100 characters specified for comments (see below):
```
 * - onLeftIconClick - optional click handler, invoked when icon is clicked (default: undefined)
```

**What did you expect to happen?**
That this 96 character comment would not result in a violation of max-len when configured to allow 100 character comments.

**What actually happened? Please include the actual, raw output from ESLint.**
Error reported indicates that the comment is in violation of the max code length: 
```
Line 61 exceeds the maximum line length of 80. (max-len)
```

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (http://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**
Prior to this PR, comment lines that were not in violation of the specified max comment length would be further validated using the max code length.  This works well for situations where the max comment length is less than the max length for code, but doesn't allow for longer comments.

Modified max-len to respect max comment length if it happens to be greater than max code length.  For situations where a max comment length is not defined, the max code length will be respected.

Added unit tests to validate this change for situations with long comments and short code.

**Is there anything you'd like reviewers to focus on?**

As a side note, I submitted this using the bug template because there was no indication in the documentation that comments must be less than the length specified for code.


mention-bot: @kgregory, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vitorbal, @not-an-aardvark and @ljharb to be potential reviewers.
jsf-clabot: [![CLA assistant check](https://cla.js.foundation/pull/badge/not_signed)](https://cla.js.foundation/eslint/eslint?pullRequest=8532) <br/>Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla.js.foundation/eslint/eslint?pullRequest=8532) before we can accept your contribution.<br/>
eslintbot: Thanks for the pull request, @kgregory! I took a look to make sure it's ready for merging and found some changes are needed:


* The commit summary needs to begin with a tag (such as `Fix:` or `Update:`). Please check out our [guide](http://eslint.org/docs/developer-guide/contributing/pull-requests#step-2-make-your-changes) for how to properly format your commit summary and [update](http://eslint.org/docs/developer-guide/contributing/pull-requests#updating-the-commit-message) it on this pull request.


Can you please update the pull request to address these?

(More information can be found in our [pull request guide](http://eslint.org/docs/developer-guide/contributing/pull-requests).)
eslintbot: LGTM
eslintbot: LGTM
kgregory: @not-an-aardvark thanks for the review and the detailed rebase instructions, which were very helpful.
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM

Patch:
diff --git a/conf/config-schema.json b/conf/config-schema.json
new file mode 100644
index 000000000..c3676bde9
--- /dev/null
+++ b/conf/config-schema.json
@@ -0,0 +1,15 @@
+{
+    "type": "object",
+    "properties": {
+        "root": { "type": "boolean" },
+        "globals": { "type": ["object"] },
+        "parser": { "type": ["string", "null"] },
+        "env": { "type": "object" },
+        "plugins": { "type": ["array"] },
+        "settings": { "type": "object" },
+        "extends": { "type": ["string", "array"] },
+        "rules": { "type": "object" },
+        "parserOptions": { "type": "object" }
+    },
+    "additionalProperties": false
+}
diff --git a/docs/user-guide/migrating-to-4.0.0.md b/docs/user-guide/migrating-to-4.0.0.md
index 84accfea4..284e16a3a 100644
--- a/docs/user-guide/migrating-to-4.0.0.md
+++ b/docs/user-guide/migrating-to-4.0.0.md
@@ -71,8 +71,6 @@ To make the upgrade process easier, we've introduced the [`indent-legacy`](/docs
 
 ## <a name="config-validation"/> Unrecognized properties in config files now cause a fatal error
 
-**Note:** This feature is a work in progress and has not been released yet.
-
 When creating a config, users sometimes make typos or misunderstand how the config is supposed to be structured. Previously, ESLint did not validate the properties of a config file, so a typo in a config could be very tedious to debug. Starting in 4.0.0, ESLint will raise an error if a property in a config file is unrecognized or has the wrong type.
 
 **To address:** If you see a config validation error after upgrading, verify that your config doesn't contain any typos. If you are using an unrecognized property, you should be able to remove it from your config to restore the previous behavior.
diff --git a/lib/config/config-validator.js b/lib/config/config-validator.js
index 36e0e9fdd..50a22b90f 100644
--- a/lib/config/config-validator.js
+++ b/lib/config/config-validator.js
@@ -12,6 +12,7 @@
 const rules = require("../rules"),
     Environments = require("./environments"),
     schemaValidator = require("is-my-json-valid"),
+    configSchema = require("../../conf/config-schema.json"),
     util = require("util");
 
 const validators = {
@@ -95,7 +96,7 @@ function validateRuleSchema(id, localOptions) {
  * Validates a rule's options against its schema.
  * @param {string} id The rule's unique name.
  * @param {array|number} options The given options for the rule.
- * @param {string} source The name of the configuration source.
+ * @param {string} source The name of the configuration source to report in any errors.
  * @returns {void}
  */
 function validateRuleOptions(id, options, source) {
@@ -113,7 +114,7 @@ function validateRuleOptions(id, options, source) {
 /**
  * Validates an environment object
  * @param {Object} environment The environment config object to validate.
- * @param {string} source The location to report with any errors.
+ * @param {string} source The name of the configuration source to report in any errors.
  * @returns {void}
  */
 function validateEnvironment(environment, source) {
@@ -123,40 +124,76 @@ function validateEnvironment(environment, source) {
         return;
     }
 
-    if (Array.isArray(environment)) {
-        throw new Error("Environment must not be an array");
-    }
+    Object.keys(environment).forEach(env => {
+        if (!Environments.get(env)) {
+            const message = `${source}:\n\tEnvironment key "${env}" is unknown\n`;
+
+            throw new Error(message);
+        }
+    });
+}
 
-    if (typeof environment === "object") {
-        Object.keys(environment).forEach(env => {
-            if (!Environments.get(env)) {
-                const message = [
-                    source, ":\n",
-                    "\tEnvironment key \"", env, "\" is unknown\n"
-                ];
-
-                throw new Error(message.join(""));
-            }
-        });
-    } else {
-        throw new Error("Environment must be an object");
+/**
+ * Validates a rules config object
+ * @param {Object} rulesConfig The rules config object to validate.
+ * @param {string} source The name of the configuration source to report in any errors.
+ * @returns {void}
+ */
+function validateRules(rulesConfig, source) {
+    if (!rulesConfig) {
+        return;
     }
+
+    Object.keys(rulesConfig).forEach(id => {
+        validateRuleOptions(id, rulesConfig[id], source);
+    });
 }
 
 /**
- * Validates an entire config object.
+ * Formats an array of schema validation errors.
+ * @param {Array} errors An array of error messages to format.
+ * @returns {string} Formatted error message
+ */
+function formatErrors(errors) {
+
+    return errors.map(error => {
+        if (error.message === "has additional properties") {
+            return `Unexpected top-level property "${error.value.replace(/^data\./, "")}"`;
+        }
+        if (error.message === "is the wrong type") {
+            const formattedField = error.field.replace(/^data\./, "");
+            const formattedExpectedType = typeof error.type === "string" ? error.type : error.type.join("/");
+            const formattedValue = JSON.stringify(error.value);
+
+            return `Property "${formattedField}" is the wrong type (expected ${formattedExpectedType} but got \`${formattedValue}\`)`;
+        }
+        return `"${error.field.replace(/^(data\.)/, "")}" ${error.message}. Value: ${error.value}`;
+    }).map(message => `\t- ${message}.\n`).join("");
+}
+
+/**
+ * Validates the top level properties of the config object.
  * @param {Object} config The config object to validate.
- * @param {string} source The location to report with any errors.
+ * @param {string} source The name of the configuration source to report in any errors.
  * @returns {void}
  */
-function validate(config, source) {
+function validateConfigSchema(config, source) {
+    const validator = schemaValidator(configSchema, { verbose: true });
 
-    if (typeof config.rules === "object") {
-        Object.keys(config.rules).forEach(id => {
-            validateRuleOptions(id, config.rules[id], source);
-        });
+    if (!validator(config)) {
+        throw new Error(`${source}:\n\tESLint configuration is invalid:\n${formatErrors(validator.errors)}`);
     }
+}
 
+/**
+ * Validates an entire config object.
+ * @param {Object} config The config object to validate.
+ * @param {string} source The name of the configuration source to report in any errors.
+ * @returns {void}
+ */
+function validate(config, source) {
+    validateConfigSchema(config, source);
+    validateRules(config.rules, source);
     validateEnvironment(config.env, source);
 }
 
diff --git a/lib/eslint.js b/lib/eslint.js
index ee5d3d2c1..e8aee5d44 100755
--- a/lib/eslint.js
+++ b/lib/eslint.js
@@ -449,9 +449,7 @@ function normalizeEcmaVersion(ecmaVersion, isModule) {
  * @returns {Object}        Processed config
  */
 function prepareConfig(config) {
-
-    config.globals = config.globals || config.global || {};
-    delete config.global;
+    config.globals = config.globals || {};
 
     const copiedRules = Object.assign({}, defaultConfig.rules);
     let parserOptions = Object.assign({}, defaultConfig.parserOptions);
@@ -735,7 +733,7 @@ module.exports = (function() {
      * @property {Object} [parserOptions] Options for the parsed used.
      * @property {Object} [settings] Global settings passed to each rule.
      * @property {Object} [env] The environment to verify in.
-     * @property {Object} [globals] Available globalsto the code.
+     * @property {Object} [globals] Available globals to the code.
      */
 
     /**
diff --git a/lib/rules/max-len.js b/lib/rules/max-len.js
index b0fa95814..137d79135 100644
--- a/lib/rules/max-len.js
+++ b/lib/rules/max-len.js
@@ -321,21 +321,24 @@ module.exports = {
                 }
 
                 const lineLength = computeLineLength(line, tabWidth);
+                const commentLengthApplies = lineIsComment && maxCommentLength;
 
                 if (lineIsComment && ignoreComments) {
                     return;
                 }
 
-                if (lineIsComment && lineLength > maxCommentLength) {
-                    context.report({
-                        node,
-                        loc: { line: lineNumber, column: 0 },
-                        message: "Line {{lineNumber}} exceeds the maximum comment line length of {{maxCommentLength}}.",
-                        data: {
-                            lineNumber: i + 1,
-                            maxCommentLength
-                        }
-                    });
+                if (commentLengthApplies) {
+                    if (lineLength > maxCommentLength) {
+                        context.report({
+                            node,
+                            loc: { line: lineNumber, column: 0 },
+                            message: "Line {{lineNumber}} exceeds the maximum comment line length of {{maxCommentLength}}.",
+                            data: {
+                                lineNumber: i + 1,
+                                maxCommentLength
+                            }
+                        });
+                    }
                 } else if (lineLength > maxLength) {
                     context.report({
                         node,


Test patch:
diff --git a/lib/testers/rule-tester.js b/lib/testers/rule-tester.js
index 2745fa342..9f6bc2a53 100644
--- a/lib/testers/rule-tester.js
+++ b/lib/testers/rule-tester.js
@@ -12,12 +12,12 @@
  * RuleTester.add("{ruleName}", {
  *      valid: [
  *          "{code}",
- *          { code: "{code}", options: {options}, global: {globals}, globals: {globals}, parser: "{parser}", settings: {settings} }
+ *          { code: "{code}", options: {options}, globals: {globals}, parser: "{parser}", settings: {settings} }
  *      ],
  *      invalid: [
  *          { code: "{code}", errors: {numErrors} },
  *          { code: "{code}", errors: ["{errorMessage}"] },
- *          { code: "{code}", options: {options}, global: {globals}, parser: "{parser}", settings: {settings}, errors: [{ message: "{errorMessage}", type: "{errorNodeType}"}] }
+ *          { code: "{code}", options: {options}, globals: {globals}, parser: "{parser}", settings: {settings}, errors: [{ message: "{errorMessage}", type: "{errorNodeType}"}] }
  *      ]
  *  });
  *
@@ -69,8 +69,8 @@ const RuleTesterParameters = [
     "code",
     "filename",
     "options",
-    "args",
-    "errors"
+    "errors",
+    "output"
 ];
 
 const validateSchema = validate(metaSchema, { verbose: true });
diff --git a/tests/lib/config.js b/tests/lib/config.js
index 3bc5dc3b3..1ebd20c2f 100644
--- a/tests/lib/config.js
+++ b/tests/lib/config.js
@@ -773,7 +773,7 @@ describe("Config", () => {
         });
 
         describe("with env in a child configuration file", () => {
-            it("should overwrite parserOptions of the parent with env of the child", () => {
+            xit("should overwrite parserOptions of the parent with env of the child", () => {
                 const config = new Config({ cwd: process.cwd() });
                 const targetPath = getFixturePath("overwrite-ecmaFeatures", "child", "foo.js");
                 const expected = {
diff --git a/tests/lib/config/config-validator.js b/tests/lib/config/config-validator.js
index 8e012f61f..114f8ec8f 100644
--- a/tests/lib/config/config-validator.js
+++ b/tests/lib/config/config-validator.js
@@ -102,116 +102,249 @@ describe("Validator", () => {
             assert.doesNotThrow(fn);
         });
 
-        it("should do nothing with an empty rules object", () => {
-            const fn = validator.validate.bind(null, { rules: {} }, "tests");
+        it("should do nothing with a valid eslint config", () => {
+            const fn = validator.validate.bind(null,
+                {
+                    root: true,
+                    globals: { globalFoo: "bar" },
+                    parser: "parserFoo",
+                    env: { browser: true },
+                    plugins: ["pluginFoo", "pluginBar"],
+                    settings: { foo: "bar" },
+                    extends: ["configFoo", "configBar"],
+                    parserOptions: { foo: "bar" },
+                    rules: {}
+                },
+                "tests");
 
             assert.doesNotThrow(fn);
         });
 
-        it("should do nothing with a valid config", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-rule": [2, "second"] } }, "tests");
+        it("should throw with an unknown property", () => {
+            const fn = validator.validate.bind(null,
+                {
+                    foo: true
+                },
+                "tests");
 
-            assert.doesNotThrow(fn);
+            assert.throws(fn, "Unexpected top-level property \"foo\".");
         });
 
-        it("should do nothing with a valid config when severity is off", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-rule": ["off", "second"] } }, "tests");
+        describe("root", () => {
+            it("should throw with a string value", () => {
+                const fn = validator.validate.bind(null, { root: "true" });
 
-            assert.doesNotThrow(fn);
-        });
+                assert.throws(fn, "Property \"root\" is the wrong type (expected boolean but got `\"true\"`).");
+            });
 
-        it("should do nothing with an invalid config when severity is off", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-required-options-rule": "off" } }, "tests");
+            it("should throw with a numeric value", () => {
+                const fn = validator.validate.bind(null, { root: 0 });
 
-            assert.doesNotThrow(fn);
+                assert.throws(fn, "Property \"root\" is the wrong type (expected boolean but got `0`).");
+            });
         });
 
-        it("should do nothing with an invalid config when severity is an array with 'off'", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-required-options-rule": ["off"] } }, "tests");
+        describe("globals", () => {
+            it("should throw with a string value", () => {
+                const fn = validator.validate.bind(null, { globals: "jQuery" });
 
-            assert.doesNotThrow(fn);
-        });
+                assert.throws(fn, "Property \"globals\" is the wrong type (expected object but got `\"jQuery\"`).");
+            });
 
-        it("should do nothing with a valid config when severity is warn", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-rule": ["warn", "second"] } }, "tests");
+            it("should throw with an array value", () => {
+                const fn = validator.validate.bind(null, { globals: ["jQuery"] });
 
-            assert.doesNotThrow(fn);
+                assert.throws(fn, "Property \"globals\" is the wrong type (expected object but got `[\"jQuery\"]`).");
+            });
         });
 
-        it("should do nothing with a valid config when severity is error", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-rule": ["error", "second"] } }, "tests");
+        describe("parser", () => {
+            it("should not throw with a null value", () => {
+                const fn = validator.validate.bind(null, { parser: null });
 
-            assert.doesNotThrow(fn);
+                assert.doesNotThrow(fn);
+            });
         });
 
-        it("should do nothing with a valid config when severity is Off", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-rule": ["Off", "second"] } }, "tests");
+        describe("env", () => {
 
-            assert.doesNotThrow(fn);
-        });
+            it("should throw with an array environment", () => {
+                const fn = validator.validate.bind(null, { env: [] });
 
-        it("should do nothing with a valid config when severity is Warn", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-rule": ["Warn", "second"] } }, "tests");
+                assert.throws(fn, "Property \"env\" is the wrong type (expected object but got `[]`).");
+            });
 
-            assert.doesNotThrow(fn);
-        });
+            it("should throw with a primitive environment", () => {
+                const fn = validator.validate.bind(null, { env: 1 });
 
-        it("should do nothing with a valid config when severity is Error", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-rule": ["Error", "second"] } }, "tests");
+                assert.throws(fn, "Property \"env\" is the wrong type (expected object but got `1`).");
+            });
 
-            assert.doesNotThrow(fn);
-        });
+            it("should catch invalid environments", () => {
+                const fn = validator.validate.bind(null, { env: { browser: true, invalid: true } });
 
-        it("should catch invalid rule options", () => {
-            const fn = validator.validate.bind(null, { rules: { "mock-rule": [3, "third"] } }, "tests");
+                assert.throws(fn, "Environment key \"invalid\" is unknown\n");
+            });
 
-            assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed '3').\n");
-        });
+            it("should catch disabled invalid environments", () => {
+                const fn = validator.validate.bind(null, { env: { browser: true, invalid: false } });
+
+                assert.throws(fn, "Environment key \"invalid\" is unknown\n");
+            });
 
-        it("should allow for rules with no options", () => {
-            eslint.defineRule("mock-no-options-rule", mockNoOptionsRule);
+            it("should do nothing with an undefined environment", () => {
+                const fn = validator.validate.bind(null, {});
 
-            const fn = validator.validate.bind(null, { rules: { "mock-no-options-rule": 2 } }, "tests");
+                assert.doesNotThrow(fn);
+            });
 
-            assert.doesNotThrow(fn);
         });
 
-        it("should not allow options for rules with no options", () => {
-            eslint.defineRule("mock-no-options-rule", mockNoOptionsRule);
+        describe("plugins", () => {
+            it("should not throw with an empty array", () => {
+                const fn = validator.validate.bind(null, { plugins: [] });
 
-            const fn = validator.validate.bind(null, { rules: { "mock-no-options-rule": [2, "extra"] } }, "tests");
+                assert.doesNotThrow(fn);
+            });
+
+            it("should throw with a string", () => {
+                const fn = validator.validate.bind(null, { plugins: "react" });
 
-            assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-no-options-rule\" is invalid:\n\tValue \"extra\" has more items than allowed.\n");
+                assert.throws(fn, "Property \"plugins\" is the wrong type (expected array but got `\"react\"`).");
+            });
         });
 
-        it("should throw with an array environment", () => {
-            const fn = validator.validate.bind(null, { env: [] });
+        describe("settings", () => {
+            it("should not throw with an empty object", () => {
+                const fn = validator.validate.bind(null, { settings: {} });
 
-            assert.throws(fn, "Environment must not be an array");
-        });
+                assert.doesNotThrow(fn);
+            });
 
-        it("should throw with a primitive environment", () => {
-            const fn = validator.validate.bind(null, { env: 1 });
+            it("should throw with an array", () => {
+                const fn = validator.validate.bind(null, { settings: ["foo"] });
 
-            assert.throws(fn, "Environment must be an object");
+                assert.throws(fn, "Property \"settings\" is the wrong type (expected object but got `[\"foo\"]`).");
+            });
         });
 
-        it("should catch invalid environments", () => {
-            const fn = validator.validate.bind(null, { env: { browser: true, invalid: true } });
+        describe("extends", () => {
+            it("should not throw with an empty array", () => {
+                const fn = validator.validate.bind(null, { extends: [] });
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should not throw with a string", () => {
+                const fn = validator.validate.bind(null, { extends: "react" });
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should throw with an object", () => {
+                const fn = validator.validate.bind(null, { extends: {} });
 
-            assert.throws(fn, "Environment key \"invalid\" is unknown\n");
+                assert.throws(fn, "Property \"extends\" is the wrong type (expected string/array but got `{}`).");
+            });
         });
 
-        it("should catch disabled invalid environments", () => {
-            const fn = validator.validate.bind(null, { env: { browser: true, invalid: false } });
+        describe("parserOptions", () => {
+            it("should not throw with an empty object", () => {
+                const fn = validator.validate.bind(null, { parserOptions: {} });
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should throw with an array", () => {
+                const fn = validator.validate.bind(null, { parserOptions: ["foo"] });
 
-            assert.throws(fn, "Environment key \"invalid\" is unknown\n");
+                assert.throws(fn, "Property \"parserOptions\" is the wrong type (expected object but got `[\"foo\"]`).");
+            });
         });
 
-        it("should do nothing with an undefined environment", () => {
-            const fn = validator.validate.bind(null, {});
+        describe("rules", () => {
 
-            assert.doesNotThrow(fn);
+            it("should do nothing with an empty rules object", () => {
+                const fn = validator.validate.bind(null, { rules: {} }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should do nothing with a valid config with rules", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-rule": [2, "second"] } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should do nothing with a valid config when severity is off", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-rule": ["off", "second"] } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should do nothing with an invalid config when severity is off", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-required-options-rule": "off" } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should do nothing with an invalid config when severity is an array with 'off'", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-required-options-rule": ["off"] } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should do nothing with a valid config when severity is warn", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-rule": ["warn", "second"] } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should do nothing with a valid config when severity is error", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-rule": ["error", "second"] } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should do nothing with a valid config when severity is Off", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-rule": ["Off", "second"] } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should do nothing with a valid config when severity is Warn", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-rule": ["Warn", "second"] } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should do nothing with a valid config when severity is Error", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-rule": ["Error", "second"] } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should catch invalid rule options", () => {
+                const fn = validator.validate.bind(null, { rules: { "mock-rule": [3, "third"] } }, "tests");
+
+                assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-rule\" is invalid:\n\tSeverity should be one of the following: 0 = off, 1 = warn, 2 = error (you passed '3').\n");
+            });
+
+            it("should allow for rules with no options", () => {
+                eslint.defineRule("mock-no-options-rule", mockNoOptionsRule);
+
+                const fn = validator.validate.bind(null, { rules: { "mock-no-options-rule": 2 } }, "tests");
+
+                assert.doesNotThrow(fn);
+            });
+
+            it("should not allow options for rules with no options", () => {
+                eslint.defineRule("mock-no-options-rule", mockNoOptionsRule);
+
+                const fn = validator.validate.bind(null, { rules: { "mock-no-options-rule": [2, "extra"] } }, "tests");
+
+                assert.throws(fn, "tests:\n\tConfiguration for rule \"mock-no-options-rule\" is invalid:\n\tValue \"extra\" has more items than allowed.\n");
+            });
         });
 
     });
diff --git a/tests/lib/rules/linebreak-style.js b/tests/lib/rules/linebreak-style.js
index a0eed487a..1e94051bf 100644
--- a/tests/lib/rules/linebreak-style.js
+++ b/tests/lib/rules/linebreak-style.js
@@ -24,8 +24,7 @@ ruleTester.run("linebreak-style", rule, {
 
     valid: [
         {
-            code: "var a = 'a',\n b = 'b';\n\n function foo(params) {\n /* do stuff */ \n }\n",
-            args: [2]
+            code: "var a = 'a',\n b = 'b';\n\n function foo(params) {\n /* do stuff */ \n }\n"
         },
         {
             code: "var a = 'a',\n b = 'b';\n\n function foo(params) {\n /* do stuff */ \n }\n",
@@ -49,7 +48,6 @@ ruleTester.run("linebreak-style", rule, {
         {
             code: "var a = 'a';\r\n",
             output: "var a = 'a';\n",
-            args: [2],
             errors: [{
                 line: 1,
                 column: 13,
@@ -79,7 +77,6 @@ ruleTester.run("linebreak-style", rule, {
         {
             code: "var a = 'a',\n b = 'b';\n\n function foo(params) {\r\n /* do stuff */ \n }\r\n",
             output: "var a = 'a',\n b = 'b';\n\n function foo(params) {\n /* do stuff */ \n }\n",
-            args: [2],
             errors: [{
                 line: 4,
                 column: 24,
diff --git a/tests/lib/rules/max-len.js b/tests/lib/rules/max-len.js
index 5729c6590..4180a8879 100644
--- a/tests/lib/rules/max-len.js
+++ b/tests/lib/rules/max-len.js
@@ -74,6 +74,11 @@ ruleTester.run("max-len", rule, {
                 "// I like short comments\n" +
                 "function butLongSourceLines() { weird(eh()) }",
             options: [80, { tabWidth: 4, comments: 30 }]
+        }, {
+            code:
+                "// I like longer comments and shorter code\n" +
+                "function see() { odd(eh()) }",
+            options: [30, { tabWidth: 4, comments: 80 }]
         }, {
             code:
                 "// Full line comment\n" +
@@ -308,6 +313,17 @@ ruleTester.run("max-len", rule, {
                     column: 1
                 }
             ]
+        }, {
+            code: "// A comment that exceeds the max comment length and the max code length, but will fail for being too long of a comment",
+            options: [40, 4, { comments: 80 }],
+            errors: [
+                {
+                    message: "Line 1 exceeds the maximum comment line length of 80.",
+                    type: "Program",
+                    line: 1,
+                    column: 1
+                }
+            ]
         }, {
             code: "// A comment that exceeds the max comment length.",
             options: [{ code: 20 }],
diff --git a/tests/lib/rules/no-global-assign.js b/tests/lib/rules/no-global-assign.js
index 0fd0ba382..a7058eaed 100644
--- a/tests/lib/rules/no-global-assign.js
+++ b/tests/lib/rules/no-global-assign.js
@@ -53,7 +53,7 @@ ruleTester.run("no-global-assign", rule, {
 
         // Notifications of readonly are moved from no-undef: https://github.com/eslint/eslint/issues/4504
         { code: "/*global b:false*/ function f() { b = 1; }", errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
-        { code: "function f() { b = 1; }", global: { b: false }, errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
+        { code: "function f() { b = 1; }", globals: { b: false }, errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
         { code: "/*global b:false*/ function f() { b++; }", errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
         { code: "/*global b*/ b = 1;", errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
         { code: "Array = 1;", errors: [{ message: "Read-only global 'Array' should not be modified.", type: "Identifier" }] }
diff --git a/tests/lib/rules/no-invalid-this.js b/tests/lib/rules/no-invalid-this.js
index e36852ebf..d975a385a 100644
--- a/tests/lib/rules/no-invalid-this.js
+++ b/tests/lib/rules/no-invalid-this.js
@@ -79,7 +79,7 @@ function extractPatterns(patterns, type) {
             thisPattern.code += " /* should error */";
         }
 
-        return thisPattern;
+        return lodash.omit(thisPattern, ["valid", "invalid"]);
     }));
 
     // Flatten.
diff --git a/tests/lib/rules/no-multi-spaces.js b/tests/lib/rules/no-multi-spaces.js
index 034f2f944..cfa1ab5e4 100644
--- a/tests/lib/rules/no-multi-spaces.js
+++ b/tests/lib/rules/no-multi-spaces.js
@@ -365,7 +365,6 @@ ruleTester.run("no-multi-spaces", rule, {
         {
             code: "({ a:  6  * 7 })",
             output: "({ a:  6 * 7 })",
-            args: 2,
             errors: [{
                 message: "Multiple spaces found before '*'.",
                 type: "Punctuator"
diff --git a/tests/lib/rules/no-native-reassign.js b/tests/lib/rules/no-native-reassign.js
index fdb08daa8..d01fcdd44 100644
--- a/tests/lib/rules/no-native-reassign.js
+++ b/tests/lib/rules/no-native-reassign.js
@@ -54,7 +54,7 @@ ruleTester.run("no-native-reassign", rule, {
 
         // Notifications of readonly are moved from no-undef: https://github.com/eslint/eslint/issues/4504
         { code: "/*global b:false*/ function f() { b = 1; }", errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
-        { code: "function f() { b = 1; }", global: { b: false }, errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
+        { code: "function f() { b = 1; }", globals: { b: false }, errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
         { code: "/*global b:false*/ function f() { b++; }", errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
         { code: "/*global b*/ b = 1;", errors: [{ message: "Read-only global 'b' should not be modified.", type: "Identifier" }] },
         { code: "Array = 1;", errors: [{ message: "Read-only global 'Array' should not be modified.", type: "Identifier" }] }
diff --git a/tests/lib/rules/no-param-reassign.js b/tests/lib/rules/no-param-reassign.js
index a98a6dfe1..c451f0c40 100644
--- a/tests/lib/rules/no-param-reassign.js
+++ b/tests/lib/rules/no-param-reassign.js
@@ -23,7 +23,8 @@ ruleTester.run("no-param-reassign", rule, {
         "function foo(a) { var b = a; }",
         "function foo(a) { a.prop = 'value'; }",
         "function foo(a) { (function() { var a = 12; a++; })(); }",
-        { code: "function foo() { global = 13; }", globals: ["global"] },
+        { code: "function foo() { someGlobal = 13; }" },
+        { code: "function foo() { someGlobal = 13; }", globals: { someGlobal: false } },
         "function foo(a) { a.b = 0; }",
         "function foo(a) { delete a.b; }",
         "function foo(a) { ++a.b; }",
diff --git a/tests/lib/rules/no-restricted-modules.js b/tests/lib/rules/no-restricted-modules.js
index cadf6bba6..86ee56d96 100644
--- a/tests/lib/rules/no-restricted-modules.js
+++ b/tests/lib/rules/no-restricted-modules.js
@@ -22,7 +22,7 @@ ruleTester.run("no-restricted-modules", rule, {
     valid: [
         { code: "require(\"fs\")", options: ["crypto"] },
         { code: "require(\"path\")", options: ["crypto", "stream", "os"] },
-        { code: "require(\"fs \")", args: 0 },
+        { code: "require(\"fs \")" },
         { code: "require(2)", options: ["crypto"] },
         { code: "require(foo)", options: ["crypto"] },
         { code: "var foo = bar('crypto');", options: ["crypto"] },
diff --git a/tests/lib/rules/no-undef.js b/tests/lib/rules/no-undef.js
index 52fcc6877..43e9e0a84 100644
--- a/tests/lib/rules/no-undef.js
+++ b/tests/lib/rules/no-undef.js
@@ -23,7 +23,6 @@ ruleTester.run("no-undef", rule, {
         "var a = 1, b = 2; a;",
         "/*global b*/ function f() { b; }",
         { code: "function f() { b; }", globals: { b: false } },
-        { code: "function f() { b; }", global: { b: false } },
         "/*global b a:false*/  a;  function f() { b; a; }",
         "function a(){}  a();",
         "function f(b) { b; }",
@@ -64,7 +63,7 @@ ruleTester.run("no-undef", rule, {
 
         // Notifications of readonly are removed: https://github.com/eslint/eslint/issues/4504
         { code: "/*global b:false*/ function f() { b = 1; }" },
-        { code: "function f() { b = 1; }", global: { b: false } },
+        { code: "function f() { b = 1; }", globals: { b: false } },
         { code: "/*global b:false*/ function f() { b++; }" },
         { code: "/*global b*/ b = 1;" },
         { code: "/*global b:false*/ var b = 1;" },
diff --git a/tests/lib/rules/no-warning-comments.js b/tests/lib/rules/no-warning-comments.js
index b7eea1bd5..fd7d0a4e1 100644
--- a/tests/lib/rules/no-warning-comments.js
+++ b/tests/lib/rules/no-warning-comments.js
@@ -22,26 +22,26 @@ ruleTester.run("no-warning-comments", rule, {
     valid: [
         { code: "// any comment", options: [{ terms: ["fixme"] }] },
         { code: "// any comment", options: [{ terms: ["fixme", "todo"] }] },
-        { code: "// any comment", args: [1] },
+        { code: "// any comment" },
         { code: "// any comment", options: [{ location: "anywhere" }] },
         { code: "// any comment with TODO, FIXME or XXX", options: [{ location: "start" }] },
-        { code: "// any comment with TODO, FIXME or XXX", args: [1] },
-        { code: "// any comment with TODO, FIXME or XXX", args: 1 },
+        { code: "// any comment with TODO, FIXME or XXX" },
+        { code: "// any comment with TODO, FIXME or XXX" },
         { code: "/* any block comment */", options: [{ terms: ["fixme"] }] },
         { code: "/* any block comment */", options: [{ terms: ["fixme", "todo"] }] },
-        { code: "/* any block comment */", args: [1] },
+        { code: "/* any block comment */" },
         { code: "/* any block comment */", options: [{ location: "anywhere" }] },
         { code: "/* any block comment with TODO, FIXME or XXX */", options: [{ location: "start" }] },
-        { code: "/* any block comment with TODO, FIXME or XXX */", args: [1] },
-        { code: "/* any block comment with TODO, FIXME or XXX */", args: 1 },
-        { code: "/* any block comment with (TODO, FIXME's or XXX!) */", args: 1 },
+        { code: "/* any block comment with TODO, FIXME or XXX */" },
+        { code: "/* any block comment with TODO, FIXME or XXX */" },
+        { code: "/* any block comment with (TODO, FIXME's or XXX!) */" },
         { code: "// comments containing terms as substrings like TodoMVC", options: [{ terms: ["todo"], location: "anywhere" }] },
         { code: "// special regex characters don't cause problems", options: [{ terms: ["[aeiou]"], location: "anywhere" }] },
         { code: "/*eslint no-warning-comments: [2, { \"terms\": [\"todo\", \"fixme\", \"any other term\"], \"location\": \"anywhere\" }]*/\n\nvar x = 10;\n" },
         { code: "/*eslint no-warning-comments: [2, { \"terms\": [\"todo\", \"fixme\", \"any other term\"], \"location\": \"anywhere\" }]*/\n\nvar x = 10;\n", options: [{ location: "anywhere" }] }
     ],
     invalid: [
-        { code: "// fixme", args: [1], errors: [{ message: "Unexpected 'fixme' comment." }] },
+        { code: "// fixme", errors: [{ message: "Unexpected 'fixme' comment." }] },
         { code: "// any fixme", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'fixme' comment." }] },
         { code: "// any fixme", options: [{ terms: ["fixme"], location: "anywhere" }], errors: [{ message: "Unexpected 'fixme' comment." }] },
         { code: "// any FIXME", options: [{ terms: ["fixme"], location: "anywhere" }], errors: [{ message: "Unexpected 'fixme' comment." }] },
@@ -52,7 +52,7 @@ ruleTester.run("no-warning-comments", rule, {
         { code: "// any fixme or todo", options: [{ terms: ["fixme", "todo"], location: "anywhere" }], errors: [{ message: "Unexpected 'fixme' comment." }, { message: "Unexpected 'todo' comment." }] },
         { code: "/* any fixme or todo */", options: [{ terms: ["fixme", "todo"], location: "anywhere" }], errors: [{ message: "Unexpected 'fixme' comment." }, { message: "Unexpected 'todo' comment." }] },
         { code: "/* any fixme or todo */", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'todo' comment." }, { message: "Unexpected 'fixme' comment." }] },
-        { code: "/* fixme and todo */", args: [1], errors: [{ message: "Unexpected 'fixme' comment." }] },
+        { code: "/* fixme and todo */", errors: [{ message: "Unexpected 'fixme' comment." }] },
         { code: "/* any fixme */", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'fixme' comment." }] },
         { code: "/* fixme! */", options: [{ terms: ["fixme"] }], errors: [{ message: "Unexpected 'fixme' comment." }] },
         { code: "// regex [litera|$]", options: [{ terms: ["[litera|$]"], location: "anywhere" }], errors: [{ message: "Unexpected '[litera|$]' comment." }] },
diff --git a/tests/lib/testers/rule-tester.js b/tests/lib/testers/rule-tester.js
index 543822f48..7e953016d 100644
--- a/tests/lib/testers/rule-tester.js
+++ b/tests/lib/testers/rule-tester.js
@@ -473,10 +473,6 @@ describe("RuleTester", () => {
                     {
                         code: "var test2 = 'bar'",
                         globals: { test: true }
-                    },
-                    {
-                        code: "var test2 = 'bar'",
-                        global: { test: true }
                     }
                 ],
                 invalid: [{ code: "bar", errors: 1 }]
@@ -497,11 +493,6 @@ describe("RuleTester", () => {
                         code: "var test = 'foo'",
                         globals: { foo: true },
                         errors: [{ message: "Global variable foo should not be used." }]
-                    },
-                    {
-                        code: "var test = 'foo'",
-                        global: { foo: true },
-                        errors: [{ message: "Global variable foo should not be used." }]
                     }
                 ]
             });
@@ -621,9 +612,33 @@ describe("RuleTester", () => {
 
     });
 
+    it("throw an error when an unknown config option is included", () => {
+
+        assert.throws(() => {
+            ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), {
+                valid: [
+                    { code: "Eval(foo)", foo: "bar" }
+                ],
+                invalid: []
+            });
+        }, /ESLint configuration is invalid./);
+    });
+
+    it("throw an error when an invalid config value is included", () => {
+
+        assert.throws(() => {
+            ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), {
+                valid: [
+                    { code: "Eval(foo)", env: ["es6"] }
+                ],
+                invalid: []
+            });
+        }, /Property "env" is the wrong type./);
+    });
+
     it("should pass-through the tester config to the rule", () => {
         ruleTester = new RuleTester({
-            global: { test: true }
+            globals: { test: true }
         });
 
         assert.doesNotThrow(() => {
@@ -632,23 +647,23 @@ describe("RuleTester", () => {
                     "var test = 'foo'",
                     "var test2 = test"
                 ],
-                invalid: [{ code: "bar", errors: 1, global: { foo: true } }]
+                invalid: [{ code: "bar", errors: 1, globals: { foo: true } }]
             });
         });
     });
 
-    it("should correctly set the global configuration", () => {
-        const config = { global: { test: true } };
+    it("should correctly set the globals configuration", () => {
+        const config = { globals: { test: true } };
 
         RuleTester.setDefaultConfig(config);
         assert(
-            RuleTester.getDefaultConfig().global.test,
+            RuleTester.getDefaultConfig().globals.test,
             "The default config object is incorrect"
         );
     });
 
     it("should correctly reset the global configuration", () => {
-        const config = { global: { test: true } };
+        const config = { globals: { test: true } };
 
         RuleTester.setDefaultConfig(config);
         RuleTester.resetDefaultConfig();
@@ -680,8 +695,8 @@ describe("RuleTester", () => {
         assert.throw(setConfig(true));
     });
 
-    it("should pass-through the global config to the tester then to the to rule", () => {
-        const config = { global: { test: true } };
+    it("should pass-through the globals config to the tester then to the to rule", () => {
+        const config = { globals: { test: true } };
 
         RuleTester.setDefaultConfig(config);
         ruleTester = new RuleTester();
@@ -692,7 +707,7 @@ describe("RuleTester", () => {
                     "var test = 'foo'",
                     "var test2 = test"
                 ],
-                invalid: [{ code: "bar", errors: 1, global: { foo: true } }]
+                invalid: [{ code: "bar", errors: 1, globals: { foo: true } }]
             });
         });
     });