Repository: eslint/eslint (js)

Issue:
Object-shorthand breaks when a dynamic key has a dot
<!--
This template is for bug reports. If you are reporting a bug, please continue on. If you are here for another reason, please see below:

1. To propose a new rule: http://eslint.org/docs/developer-guide/contributing/new-rules
2. To request a change: http://eslint.org/docs/developer-guide/contributing/changes
3. If you have any questions, please stop by our chatroom: https://gitter.im/eslint/eslint

Note that leaving sections blank will make it difficult for us to troubleshoot and we may have to close the issue.
-->

FYI, this is a dupe of #8351, but just including more information:

**Tell us about your environment**
* **ESLint Version:**
3.19.0
* **Node Version:**
7.9.0
* **npm Version:**
4.2.0

**What parser (default, Babel-ESLint, etc.) are you using?**
babel-eslint

**Please show your full configuration:**

<!-- Paste your configuration below: -->
```
'object-shorthand': [2, 'always', {'ignoreConstructors': true}],
```

**What did you do? Please include the actual source code causing the issue.**

<!-- Paste the source code below: -->

~~const keys = {~~
~~first: 'first',~~
~~second: 'second',~~
~~};~~

~~const o = {~~
~~[keys.first]: true,~~
~~[keys.last]: true,~~
~~};~~


```js
const actionHandlers = {
	[actionTypes.UPDATE_COLLAPSED_ENTRIES]: state => true
};
```


**What did you expect to happen?**
For it not to error out.

**What actually happened? Please include the actual, raw output from ESLint.**
It through an error:
```
TypeError: Cannot read property 'charAt' of undefined
    at isConstructor (/Users/ncavanaugh/Dev/git/check-source-formatting/node_modules/eslint/lib/rules/object-shorthand.js:120:35)
    at EventEmitter.Property:exit (/Users/ncavanaugh/Dev/git/check-source-formatting/node_modules/eslint/lib/rules/object-shorthand.js:395:48)
...
```

It comes down to keys that have a property don't have a name string attached, and so when it tries to run `charAt` on it, it throws an error.

Let me know if you need more info than the above.

Thanks!

Pull request description:
Fix: object-shorthand crash on some computed keys (fixes #8576)
**What is the purpose of this pull request? (put an "X" next to item)**

[x] Bug fix (https://github.com/eslint/eslint/issues/8576)

**What changes did you make? (Give an overview)**

This commit adds a missing node type check to fix the crash described in https://github.com/eslint/eslint/issues/8576.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular


Patch:
diff --git a/conf/eslint-recommended.js b/conf/eslint-recommended.js
index 29cc3de29..4ce8106ef 100755
--- a/conf/eslint-recommended.js
+++ b/conf/eslint-recommended.js
@@ -248,6 +248,7 @@ module.exports = {
         "space-unary-ops": "off",
         "spaced-comment": "off",
         "strict": "off",
+        "switch-colon-spacing": "off",
         "symbol-description": "off",
         "template-curly-spacing": "off",
         "template-tag-spacing": "off",
diff --git a/docs/rules/switch-colon-spacing.md b/docs/rules/switch-colon-spacing.md
new file mode 100644
index 000000000..baf081e4c
--- /dev/null
+++ b/docs/rules/switch-colon-spacing.md
@@ -0,0 +1,80 @@
+# Enforce spacing around colons of switch statements (switch-colon-spacing)
+
+Spacing around colons improves readability of `case`/`default` clauses.
+
+## Rule Details
+
+This rule controls spacing around colons of `case` and `default` clauses in `switch` statements.
+This rule does the check only if the consecutive tokens exist on the same line.
+
+This rule has 2 options that are boolean value.
+
+```json
+{
+    "switch-colon-spacing": ["error", {"after": true, "before": false}]
+}
+```
+
+- `"after": true` (Default) ... requires one or more spaces after colons.
+- `"after": false` ... disallows spaces after colons.
+- `"before": true` ... requires one or more spaces before colons.
+- `"before": false` (Default) ... disallows before colons.
+
+
+Examples of **incorrect** code for this rule:
+
+```js
+/*eslint switch-colon-spacing: "error"*/
+
+switch (a) {
+    case 0 :break;
+    default :foo();
+}
+```
+
+Examples of **correct** code for this rule:
+
+```js
+/*eslint switch-colon-spacing: "error"*/
+
+switch (a) {
+    case 0: foo(); break;
+    case 1:
+        bar();
+        break;
+    default:
+        baz();
+        break;
+}
+```
+
+Examples of **incorrect** code for this rule with `{"after": false, "before": true}` option:
+
+```js
+/*eslint switch-colon-spacing: ["error", {"after": false, "before": true}]*/
+
+switch (a) {
+    case 0: break;
+    default: foo();
+}
+```
+
+Examples of **correct** code for this rule with `{"after": false, "before": true}` option:
+
+```js
+/*eslint switch-colon-spacing: ["error", {"after": false, "before": true}]*/
+
+switch (a) {
+    case 0 :foo(); break;
+    case 1 :
+        bar();
+        break;
+    default :
+        baz();
+        break;
+}
+```
+
+## When Not To Use It
+
+If you don't want to notify spacing around colons of switch statements, then it's safe to disable this rule.
diff --git a/lib/rules/indent.js b/lib/rules/indent.js
index 30d9401dc..887a57b86 100644
--- a/lib/rules/indent.js
+++ b/lib/rules/indent.js
@@ -610,8 +610,8 @@ module.exports = {
         }
 
         /**
-         * Check indentation for blocks
-         * @param {ASTNode} node node to check
+         * Check indentation for blocks and class bodies
+         * @param {ASTNode} node The BlockStatement or ClassBody node to indent
          * @returns {void}
          */
         function addBlockIndent(node) {
@@ -765,10 +765,13 @@ module.exports = {
         * @returns {void}
         */
         function addClassIndent(node) {
-            const tokens = getTokensAndComments(node);
+            if (node.superClass) {
+                const classToken = sourceCode.getFirstToken(node);
+                const extendsToken = sourceCode.getTokenBefore(node.superClass, astUtils.isNotOpeningParenToken);
 
-            offsets.setDesiredOffsets(tokens, tokens[0], 1);
-            offsets.matchIndentOf(tokens[0], tokens[tokens.length - 1]);
+                offsets.setDesiredOffset(extendsToken, classToken, 1);
+                offsets.setDesiredOffsets(sourceCode.getTokensBetween(extendsToken, node.body, { includeComments: true }), classToken, 1);
+            }
         }
 
         /**
@@ -837,6 +840,8 @@ module.exports = {
 
             CallExpression: addFunctionCallIndent,
 
+            ClassBody: addBlockIndent,
+
             ClassDeclaration: addClassIndent,
 
             ClassExpression: addClassIndent,
diff --git a/lib/rules/object-shorthand.js b/lib/rules/object-shorthand.js
index 1d3d9dae1..2f7b0ccf9 100644
--- a/lib/rules/object-shorthand.js
+++ b/lib/rules/object-shorthand.js
@@ -392,7 +392,7 @@ module.exports = {
                         });
                     }
                 } else if (APPLY_TO_METHODS && !node.value.id && (node.value.type === "FunctionExpression" || node.value.type === "ArrowFunctionExpression")) {
-                    if (IGNORE_CONSTRUCTORS && isConstructor(node.key.name)) {
+                    if (IGNORE_CONSTRUCTORS && node.key.type === "Identifier" && isConstructor(node.key.name)) {
                         return;
                     }
                     if (AVOID_QUOTES && isStringLiteral(node.key)) {
diff --git a/lib/rules/switch-colon-spacing.js b/lib/rules/switch-colon-spacing.js
new file mode 100644
index 000000000..84820956e
--- /dev/null
+++ b/lib/rules/switch-colon-spacing.js
@@ -0,0 +1,133 @@
+/**
+ * @fileoverview Rule to enforce spacing around colons of switch statements.
+ * @author Toru Nagashima
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const astUtils = require("../ast-utils");
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+    meta: {
+        docs: {
+            description: "enforce spacing around colons of switch statements",
+            category: "Stylistic Issues",
+            recommended: false
+        },
+        schema: [
+            {
+                type: "object",
+                properties: {
+                    before: { type: "boolean" },
+                    after: { type: "boolean" }
+                },
+                additionalProperties: false
+            }
+        ],
+        fixable: "whitespace"
+    },
+
+    create(context) {
+        const sourceCode = context.getSourceCode();
+        const options = context.options[0] || {};
+        const beforeSpacing = options.before === true; // false by default
+        const afterSpacing = options.after !== false; // true by default
+
+        /**
+         * Get the colon token of the given SwitchCase node.
+         * @param {ASTNode} node The SwitchCase node to get.
+         * @returns {Token} The colon token of the node.
+         */
+        function getColonToken(node) {
+            if (node.test) {
+                return sourceCode.getTokenAfter(node.test, astUtils.isColonToken);
+            }
+            return sourceCode.getFirstToken(node, 1);
+        }
+
+        /**
+         * Check whether the spacing between the given 2 tokens is valid or not.
+         * @param {Token} left The left token to check.
+         * @param {Token} right The right token to check.
+         * @param {boolean} expected The expected spacing to check. `true` if there should be a space.
+         * @returns {boolean} `true` if the spacing between the tokens is valid.
+         */
+        function isValidSpacing(left, right, expected) {
+            return (
+                astUtils.isClosingBraceToken(right) ||
+                !astUtils.isTokenOnSameLine(left, right) ||
+                sourceCode.isSpaceBetweenTokens(left, right) === expected
+            );
+        }
+
+        /**
+         * Check whether comments exist between the given 2 tokens.
+         * @param {Token} left The left token to check.
+         * @param {Token} right The right token to check.
+         * @returns {boolean} `true` if comments exist between the given 2 tokens.
+         */
+        function commentsExistBetween(left, right) {
+            return sourceCode.getFirstTokenBetween(
+                left,
+                right,
+                {
+                    includeComments: true,
+                    filter: astUtils.isCommentToken
+                }
+            ) !== null;
+        }
+
+        /**
+         * Fix the spacing between the given 2 tokens.
+         * @param {RuleFixer} fixer The fixer to fix.
+         * @param {Token} left The left token of fix range.
+         * @param {Token} right The right token of fix range.
+         * @param {boolean} spacing The spacing style. `true` if there should be a space.
+         * @returns {Fix|null} The fix object.
+         */
+        function fix(fixer, left, right, spacing) {
+            if (commentsExistBetween(left, right)) {
+                return null;
+            }
+            if (spacing) {
+                return fixer.insertTextAfter(left, " ");
+            }
+            return fixer.removeRange([left.range[1], right.range[0]]);
+        }
+
+        return {
+            SwitchCase(node) {
+                const colonToken = getColonToken(node, sourceCode);
+                const beforeToken = sourceCode.getTokenBefore(colonToken);
+                const afterToken = sourceCode.getTokenAfter(colonToken);
+
+                if (!isValidSpacing(beforeToken, colonToken, beforeSpacing)) {
+                    context.report({
+                        node,
+                        loc: colonToken.loc,
+                        message: "{{verb}} space(s) before this colon.",
+                        data: { verb: beforeSpacing ? "Expected" : "Unexpected" },
+                        fix: fixer => fix(fixer, beforeToken, colonToken, beforeSpacing)
+                    });
+                }
+                if (!isValidSpacing(colonToken, afterToken, afterSpacing)) {
+                    context.report({
+                        node,
+                        loc: colonToken.loc,
+                        message: "{{verb}} space(s) after this colon.",
+                        data: { verb: afterSpacing ? "Expected" : "Unexpected" },
+                        fix: fixer => fix(fixer, colonToken, afterToken, afterSpacing)
+                    });
+                }
+            }
+        };
+    }
+};


Test patch:
diff --git a/tests/lib/rules/indent.js b/tests/lib/rules/indent.js
index dbe140a44..0b4e074c6 100644
--- a/tests/lib/rules/indent.js
+++ b/tests/lib/rules/indent.js
@@ -1543,6 +1543,18 @@ ruleTester.run("indent", rule, {
             parserOptions: { ecmaVersion: 6 },
             options: [2]
         },
+        {
+            code: unIndent`
+                class Foo extends
+                  (
+                    Bar
+                  ) {
+                  baz() {}
+                }
+            `,
+            parserOptions: { ecmaVersion: 6 },
+            options: [2]
+        },
         {
             code: unIndent`
                 fs.readdirSync(path.join(__dirname, '../rules')).forEach(name => {
@@ -3204,6 +3216,70 @@ ruleTester.run("indent", rule, {
             `,
             parserOptions: { ecmaVersion: 6 }
         },
+        {
+            code: unIndent`
+                class Foo
+                {
+                    constructor()
+                    {
+                        foo();
+                    }
+
+                    bar()
+                    {
+                        baz();
+                    }
+                }
+            `,
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: unIndent`
+                class Foo
+                    extends Bar
+                {
+                    constructor()
+                    {
+                        foo();
+                    }
+
+                    bar()
+                    {
+                        baz();
+                    }
+                }
+            `,
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: unIndent`
+                (
+                    class Foo
+                    {
+                        constructor()
+                        {
+                            foo();
+                        }
+
+                        bar()
+                        {
+                            baz();
+                        }
+                    }
+                )
+            `,
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: unIndent`
+                switch (foo)
+                {
+                    case 1:
+                        bar();
+                }
+            `,
+            options: [4, { SwitchCase: 1 }]
+        },
         {
             code: unIndent`
                 foo
diff --git a/tests/lib/rules/object-shorthand.js b/tests/lib/rules/object-shorthand.js
index d961849ae..ffa077126 100644
--- a/tests/lib/rules/object-shorthand.js
+++ b/tests/lib/rules/object-shorthand.js
@@ -374,6 +374,10 @@ ruleTester.run("object-shorthand", rule, {
                 }
             `,
             options: ["always", { avoidExplicitReturnArrows: true }]
+        },
+        {
+            code: "({ [foo.bar]: () => {} })",
+            options: ["always", { ignoreConstructors: true }]
         }
     ],
     invalid: [
diff --git a/tests/lib/rules/switch-colon-spacing.js b/tests/lib/rules/switch-colon-spacing.js
new file mode 100644
index 000000000..009f21638
--- /dev/null
+++ b/tests/lib/rules/switch-colon-spacing.js
@@ -0,0 +1,171 @@
+/**
+ * @fileoverview Tests for switch-colon-spacing rule.
+ * @author Toru Nagashima
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require("../../../lib/rules/switch-colon-spacing"),
+    RuleTester = require("../../../lib/testers/rule-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester();
+
+ruleTester.run("switch-colon-spacing", rule, {
+    valid: [
+        "switch(a){}",
+        "({foo:1,bar : 2});",
+        "A:foo(); B : foo();",
+        "switch(a){case 0: break;}",
+        "switch(a){case 0:}",
+        "switch(a){case 0\n:\nbreak;}",
+        "switch(a){default: break;}",
+        "switch(a){default:}",
+        "switch(a){default\n:\nbreak;}",
+        { code: "switch(a){case 0:break;}", options: [{ before: false, after: false }] },
+        { code: "switch(a){case 0:}", options: [{ before: false, after: false }] },
+        { code: "switch(a){case 0\n:\nbreak;}", options: [{ before: false, after: false }] },
+        { code: "switch(a){default:break;}", options: [{ before: false, after: false }] },
+        { code: "switch(a){default:}", options: [{ before: false, after: false }] },
+        { code: "switch(a){default\n:\nbreak;}", options: [{ before: false, after: false }] },
+        { code: "switch(a){case 0: break;}", options: [{ before: false, after: true }] },
+        { code: "switch(a){case 0:}", options: [{ before: false, after: true }] },
+        { code: "switch(a){case 0\n:\nbreak;}", options: [{ before: false, after: true }] },
+        { code: "switch(a){default: break;}", options: [{ before: false, after: true }] },
+        { code: "switch(a){default:}", options: [{ before: false, after: true }] },
+        { code: "switch(a){default\n:\nbreak;}", options: [{ before: false, after: true }] },
+        { code: "switch(a){case 0 :break;}", options: [{ before: true, after: false }] },
+        { code: "switch(a){case 0 :}", options: [{ before: true, after: false }] },
+        { code: "switch(a){case 0\n:\nbreak;}", options: [{ before: true, after: false }] },
+        { code: "switch(a){default :break;}", options: [{ before: true, after: false }] },
+        { code: "switch(a){default :}", options: [{ before: true, after: false }] },
+        { code: "switch(a){default\n:\nbreak;}", options: [{ before: true, after: false }] },
+        { code: "switch(a){case 0 : break;}", options: [{ before: true, after: true }] },
+        { code: "switch(a){case 0 :}", options: [{ before: true, after: true }] },
+        { code: "switch(a){case 0\n:\nbreak;}", options: [{ before: true, after: true }] },
+        { code: "switch(a){default : break;}", options: [{ before: true, after: true }] },
+        { code: "switch(a){default :}", options: [{ before: true, after: true }] },
+        { code: "switch(a){default\n:\nbreak;}", options: [{ before: true, after: true }] }
+    ],
+    invalid: [
+        {
+            code: "switch(a){case 0 :break;}",
+            output: "switch(a){case 0: break;}",
+            errors: [
+                "Unexpected space(s) before this colon.",
+                "Expected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){default :break;}",
+            output: "switch(a){default: break;}",
+            errors: [
+                "Unexpected space(s) before this colon.",
+                "Expected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){case 0 : break;}",
+            output: "switch(a){case 0:break;}",
+            options: [{ before: false, after: false }],
+            errors: [
+                "Unexpected space(s) before this colon.",
+                "Unexpected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){default : break;}",
+            output: "switch(a){default:break;}",
+            options: [{ before: false, after: false }],
+            errors: [
+                "Unexpected space(s) before this colon.",
+                "Unexpected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){case 0 :break;}",
+            output: "switch(a){case 0: break;}",
+            options: [{ before: false, after: true }],
+            errors: [
+                "Unexpected space(s) before this colon.",
+                "Expected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){default :break;}",
+            output: "switch(a){default: break;}",
+            options: [{ before: false, after: true }],
+            errors: [
+                "Unexpected space(s) before this colon.",
+                "Expected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){case 0: break;}",
+            output: "switch(a){case 0 :break;}",
+            options: [{ before: true, after: false }],
+            errors: [
+                "Expected space(s) before this colon.",
+                "Unexpected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){default: break;}",
+            output: "switch(a){default :break;}",
+            options: [{ before: true, after: false }],
+            errors: [
+                "Expected space(s) before this colon.",
+                "Unexpected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){case 0:break;}",
+            output: "switch(a){case 0 : break;}",
+            options: [{ before: true, after: true }],
+            errors: [
+                "Expected space(s) before this colon.",
+                "Expected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){default:break;}",
+            output: "switch(a){default : break;}",
+            options: [{ before: true, after: true }],
+            errors: [
+                "Expected space(s) before this colon.",
+                "Expected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){case 0 /**/ :break;}",
+            output: "switch(a){case 0 /**/ : break;}",
+            errors: [
+                "Unexpected space(s) before this colon.",
+                "Expected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){case 0 :/**/break;}",
+            output: "switch(a){case 0:/**/break;}",
+            errors: [
+                "Unexpected space(s) before this colon.",
+                "Expected space(s) after this colon."
+            ]
+        },
+        {
+            code: "switch(a){case (0) :break;}",
+            output: "switch(a){case (0): break;}",
+            errors: [
+                "Unexpected space(s) before this colon.",
+                "Expected space(s) after this colon."
+            ]
+        }
+    ]
+});