Repository: eslint/eslint
Pull request #9698 (2017-12-08 19:22:11)

Chore: no-invalid-meta crash if no export assignment (refs #9534)
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**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:

See #9534.

<!--
    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.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (https://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)**

Added new lint error report for files that don't assign to `module.exports`, avoiding a crash that otherwise occurs later in the rule.

**Is there anything you'd like reviewers to focus on?**

One test had to be commented out due to #9534. Otherwise, not really.

platinumazure: Hmm. Since this doesn't impact end-users (though it could impact developers/contributors?), should this be a Chore?
gyandeeps: yes this should be a chore
platinumazure: @gyandeeps Fixed, this is now a Chore. Thanks!

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 93e1f4a09..0af32749c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+v4.13.0 - December 8, 2017
+
+* 256481b Update: update handling of destructuring in camelcase (fixes #8511) (#9468) (Erin)
+* d067ae1 Docs: Don’t use undocumented array-style configuration for max-len (#9690) (Jed Fox)
+* 1ad3091 Chore: fix test-suite to work with node master (#9688) (Myles Borins)
+* cdb1488 Docs: Adds an example with try/catch. (#9672) (Jaap Taal)
+
 v4.12.1 - November 30, 2017
 
 * 1e362a0 Revert "Fix: Use XML 1.1 on XML formatters (fixes #9607) (#9608)" (#9667) (Kevin Partington)
diff --git a/docs/rules/camelcase.md b/docs/rules/camelcase.md
index 66abcccf8..db660bcee 100644
--- a/docs/rules/camelcase.md
+++ b/docs/rules/camelcase.md
@@ -32,9 +32,27 @@ obj.do_something = function() {
     // ...
 };
 
+function foo({ no_camelcased }) {
+    // ...
+};
+
+function foo({ isCamelcased: no_camelcased }) {
+    // ...
+}
+
+function foo({ no_camelcased = 'default value' }) {
+    // ...
+};
+
 var obj = {
     my_pref: 1
 };
+
+var { category_id = 1 } = query;
+
+var { foo: no_camelcased } = bar;
+
+var { foo: bar_baz = 1 } = quz;
 ```
 
 Examples of **correct** code for this rule with the default `{ "properties": "always" }` option:
@@ -56,6 +74,25 @@ do_something();
 new do_something();
 
 var { category_id: category } = query;
+
+function foo({ isCamelCased }) {
+    // ...
+};
+
+function foo({ isCamelCased: isAlsoCamelCased }) {
+    // ...
+}
+
+function foo({ isCamelCased = 'default value' }) {
+    // ...
+};
+
+var { categoryId = 1 } = query;
+
+var { foo: isCamelCased } = bar;
+
+var { foo: isCamelCased = 1 } = quz;
+
 ```
 
 ### never
diff --git a/lib/rules/camelcase.js b/lib/rules/camelcase.js
index 6fb1475b2..aafa876f6 100644
--- a/lib/rules/camelcase.js
+++ b/lib/rules/camelcase.js
@@ -92,34 +92,45 @@ module.exports = {
                     }
 
                     // Always report underscored object names
-                    if (node.parent.object.type === "Identifier" &&
-                            node.parent.object.name === node.name &&
-                            isUnderscored(name)) {
+                    if (node.parent.object.type === "Identifier" && node.parent.object.name === node.name && isUnderscored(name)) {
                         report(node);
 
                     // Report AssignmentExpressions only if they are the left side of the assignment
-                    } else if (effectiveParent.type === "AssignmentExpression" &&
-                            isUnderscored(name) &&
-                            (effectiveParent.right.type !== "MemberExpression" ||
-                            effectiveParent.left.type === "MemberExpression" &&
-                            effectiveParent.left.property.name === node.name)) {
+                    } else if (effectiveParent.type === "AssignmentExpression" && isUnderscored(name) && (effectiveParent.right.type !== "MemberExpression" || effectiveParent.left.type === "MemberExpression" && effectiveParent.left.property.name === node.name)) {
                         report(node);
                     }
 
-                // Properties have their own rules
-                } else if (node.parent.type === "Property") {
+                /*
+                 * Properties have their own rules, and
+                 * AssignmentPattern nodes can be treated like Properties:
+                 * e.g.: const { no_camelcased = false } = bar;
+                 */
+                } else if (node.parent.type === "Property" || node.parent.type === "AssignmentPattern") {
 
-                    // "never" check properties
-                    if (properties === "never") {
-                        return;
+                    if (node.parent.parent && node.parent.parent.type === "ObjectPattern") {
+
+                        if (node.parent.shorthand && node.parent.value.left && isUnderscored(node.parent.value.left.name)) {
+
+                            report(node);
+                        }
+
+                        // prevent checking righthand side of destructured object
+                        if (node.parent.key === node && node.parent.value !== node) {
+                            return;
+                        }
+
+                        if (node.parent.value.name && isUnderscored(node.parent.value.name)) {
+                            report(node);
+                        }
                     }
 
-                    if (node.parent.parent && node.parent.parent.type === "ObjectPattern" &&
-                            node.parent.key === node && node.parent.value !== node) {
+                    // "never" check properties
+                    if (properties === "never") {
                         return;
                     }
 
-                    if (isUnderscored(name) && !ALLOWED_PARENT_TYPES.has(effectiveParent.type)) {
+                    // don't check right hand side of AssignmentExpression to prevent duplicate warnings
+                    if (isUnderscored(name) && !ALLOWED_PARENT_TYPES.has(effectiveParent.type) && !(node.parent.right === node)) {
                         report(node);
                     }
 
diff --git a/package.json b/package.json
index ca1e8bfe4..a8fcf78bd 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "eslint",
-  "version": "4.12.1",
+  "version": "4.13.0",
   "author": "Nicholas C. Zakas <nicholas+npm@nczconsulting.com>",
   "description": "An AST-based pattern checker for JavaScript.",
   "bin": {
diff --git a/tools/internal-rules/no-invalid-meta.js b/tools/internal-rules/no-invalid-meta.js
index d13df358b..8082d1000 100644
--- a/tools/internal-rules/no-invalid-meta.js
+++ b/tools/internal-rules/no-invalid-meta.js
@@ -175,13 +175,20 @@ module.exports = {
                 }
             },
 
-            "Program:exit"() {
-                if (!isCorrectExportsFormat(exportsNode)) {
-                    context.report({ node: exportsNode, message: "Rule does not export an Object. Make sure the rule follows the new rule format." });
-                    return;
+            "Program:exit"(node) {
+                if (!exportsNode) {
+                    context.report({
+                        node,
+                        message: "Rule does not export anything. Make sure rule exports an object according to new rule format."
+                    });
+                } else if (!isCorrectExportsFormat(exportsNode)) {
+                    context.report({
+                        node: exportsNode,
+                        message: "Rule does not export an Object. Make sure the rule follows the new rule format."
+                    });
+                } else {
+                    checkMetaValidity(context, exportsNode);
                 }
-
-                checkMetaValidity(context, exportsNode);
             }
         };
     }


Test patch:
diff --git a/tests/lib/rules/camelcase.js b/tests/lib/rules/camelcase.js
index 5983fb1c6..12716b5db 100644
--- a/tests/lib/rules/camelcase.js
+++ b/tests/lib/rules/camelcase.js
@@ -63,11 +63,6 @@ ruleTester.run("camelcase", rule, {
             code: "var { category_id: category } = query;",
             parserOptions: { ecmaVersion: 6 }
         },
-        {
-            code: "var { category_id: category } = query;",
-            options: [{ properties: "never" }],
-            parserOptions: { ecmaVersion: 6 }
-        },
         {
             code: "import { camelCased } from \"external module\";",
             parserOptions: { ecmaVersion: 6, sourceType: "module" }
@@ -79,6 +74,18 @@ ruleTester.run("camelcase", rule, {
         {
             code: "import { no_camelcased as camelCased, anoterCamelCased } from \"external-module\";",
             parserOptions: { ecmaVersion: 6, sourceType: "module" }
+        },
+        {
+            code: "function foo({ no_camelcased: camelCased }) {};",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "function foo({ camelCased = 'default value' }) {};",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "function foo({ camelCased }) {};",
+            parserOptions: { ecmaVersion: 6 }
         }
     ],
     invalid: [
@@ -212,6 +219,16 @@ ruleTester.run("camelcase", rule, {
                 }
             ]
         },
+        {
+            code: "var { category_id = 1 } = query;",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    message: "Identifier 'category_id' is not in camel case.",
+                    type: "Identifier"
+                }
+            ]
+        },
         {
             code: "import no_camelcased from \"external-module\";",
             parserOptions: { ecmaVersion: 6, sourceType: "module" },
@@ -301,6 +318,90 @@ ruleTester.run("camelcase", rule, {
                     type: "Identifier"
                 }
             ]
+        },
+        {
+            code: "function foo({ no_camelcased }) {};",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    message: "Identifier 'no_camelcased' is not in camel case.",
+                    type: "Identifier"
+                }
+            ]
+        },
+        {
+            code: "function foo({ no_camelcased = 'default value' }) {};",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    message: "Identifier 'no_camelcased' is not in camel case.",
+                    type: "Identifier"
+                }
+            ]
+        },
+        {
+            code: "const no_camelcased = 0; function foo({ camelcased_value = no_camelcased}) {}",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    message: "Identifier 'no_camelcased' is not in camel case.",
+                    type: "Identifier"
+                },
+                {
+                    message: "Identifier 'camelcased_value' is not in camel case.",
+                    type: "Identifier"
+                }
+            ]
+        },
+        {
+            code: "const { bar: no_camelcased } = foo;",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    message: "Identifier 'no_camelcased' is not in camel case.",
+                    type: "Identifier"
+                }
+            ]
+        },
+        {
+            code: "function foo({ value_1: my_default }) {}",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    message: "Identifier 'my_default' is not in camel case.",
+                    type: "Identifier"
+                }
+            ]
+        },
+        {
+            code: "function foo({ isCamelcased: no_camelcased }) {};",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    message: "Identifier 'no_camelcased' is not in camel case.",
+                    type: "Identifier"
+                }
+            ]
+        },
+        {
+            code: "var { foo: bar_baz = 1 } = quz;",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    message: "Identifier 'bar_baz' is not in camel case.",
+                    type: "Identifier"
+                }
+            ]
+        },
+        {
+            code: "const { no_camelcased = false } = bar;",
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    message: "Identifier 'no_camelcased' is not in camel case.",
+                    type: "Identifier"
+                }
+            ]
         }
     ]
 });
diff --git a/tests/tools/internal-rules/no-invalid-meta.js b/tests/tools/internal-rules/no-invalid-meta.js
index 54b2c9628..5d675e1e3 100644
--- a/tests/tools/internal-rules/no-invalid-meta.js
+++ b/tests/tools/internal-rules/no-invalid-meta.js
@@ -241,6 +241,40 @@ ruleTester.run("no-invalid-meta", rule, {
                 line: 2,
                 column: 5
             }]
+        },
+
+        /*
+         * Rule doesn't export anything: Should warn on the Program node.
+         * See https://github.com/eslint/eslint/issues/9534
+         */
+
+        /*
+         * Should be invalid, but will currently show as valid due to #9534.
+         * FIXME: Uncomment when #9534 is fixed in major release.
+         * {
+         *     code: "",
+         *     errors: [{
+         *         message: "Rule does not export anything. Make sure rule exports an object according to new rule format.",
+         *         line: 1,
+         *         column: 1
+         *     }]
+         * },
+         */
+        {
+            code: "foo();",
+            errors: [{
+                message: "Rule does not export anything. Make sure rule exports an object according to new rule format.",
+                line: 1,
+                column: 1
+            }]
+        },
+        {
+            code: "foo = bar;",
+            errors: [{
+                message: "Rule does not export anything. Make sure rule exports an object according to new rule format.",
+                line: 1,
+                column: 1
+            }]
         }
     ]
 });