Repository: eslint/eslint (js)

Issue:
`nonblock-statement-body-position` Errors on Correct Code
<!--
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.
-->

**Tell us about your environment**

* **ESLint Version:** 3.17.0
* **Node Version:** 7.6.0
* **npm Version:** 4.1.2

**What parser (default, Babel-ESLint, etc.) are you using?**
Default

**Please show your full configuration:**

<!-- Paste your configuration below: -->
```
{
    rules: {
        'nonblock-statement-body-position':  2
    }
}
```

**What did you do? Please include the actual source code causing the issue.**

<!-- Paste the source code below: -->
```js

function blah() {
    if (window.hey) return window.doit({
        asdf: 2,
    });
    return 1;
}
```

**What did you expect to happen?**
No errors.

**What actually happened? Please include the actual, raw output from ESLint.**

`3:21  error  Expected no linebreak before this statement  nonblock-statement-body-position`

Even if the rule doesn't allow expressions that extend onto the next line (which would be a bit of a bummer), the error message doesn't make a whole lot of sense.

Pull request description:
Fix: nonblock-statement-body-position multiline error (fixes #8202)
**What is the purpose of this pull request? (put an "X" next to item)**

[x] Bug fix (https://github.com/eslint/eslint/issues/8202)

**What changes did you make? (Give an overview)**

The nonblock-statement-body-position rule is intended to compare the *start* line of a statement with the end line of the previous token. Due to a typo, it was comparing the *end* line of the statement instead, which caused false positives for multiline statements.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular

Patch:
diff --git a/.eslintrc.yml b/.eslintrc.yml
index 0f505ebc7..c857caa67 100644
--- a/.eslintrc.yml
+++ b/.eslintrc.yml
@@ -1,7 +1,13 @@
 root: true
 
 plugins:
+    - eslint-plugin
     - node
 extends:
     - "./packages/eslint-config-eslint/default.yml"
     - "plugin:node/recommended"
+    - "plugin:eslint-plugin/recommended"
+rules:
+    eslint-plugin/consistent-output: "error"
+    eslint-plugin/prefer-placeholders: "error"
+    eslint-plugin/report-message-format: ["error", '[^a-z].*\.$']
diff --git a/lib/internal-rules/internal-no-invalid-meta.js b/lib/internal-rules/internal-no-invalid-meta.js
index d1c78efa6..d13df358b 100644
--- a/lib/internal-rules/internal-no-invalid-meta.js
+++ b/lib/internal-rules/internal-no-invalid-meta.js
@@ -94,16 +94,6 @@ function hasMetaSchema(metaPropertyNode) {
     return getPropertyFromObject("schema", metaPropertyNode.value);
 }
 
-/**
- * Whether this `meta` ObjectExpression has a `fixable` property defined or not.
- *
- * @param {ASTNode} metaPropertyNode The `meta` ObjectExpression for this rule.
- * @returns {boolean} `true` if a `fixable` property exists.
- */
-function hasMetaFixable(metaPropertyNode) {
-    return getPropertyFromObject("fixable", metaPropertyNode.value);
-}
-
 /**
  * Checks the validity of the meta definition of this rule and reports any errors found.
  *
@@ -112,7 +102,7 @@ function hasMetaFixable(metaPropertyNode) {
  * @param {boolean} ruleIsFixable whether the rule is fixable or not.
  * @returns {void}
  */
-function checkMetaValidity(context, exportsNode, ruleIsFixable) {
+function checkMetaValidity(context, exportsNode) {
     const metaProperty = getMetaPropertyFromExportsNode(exportsNode);
 
     if (!metaProperty) {
@@ -142,11 +132,6 @@ function checkMetaValidity(context, exportsNode, ruleIsFixable) {
 
     if (!hasMetaSchema(metaProperty)) {
         context.report(metaProperty, "Rule is missing a meta.schema property.");
-        return;
-    }
-
-    if (ruleIsFixable && !hasMetaFixable(metaProperty)) {
-        context.report(metaProperty, "Rule is fixable, but is missing a meta.fixable property.");
     }
 }
 
@@ -177,7 +162,6 @@ module.exports = {
 
     create(context) {
         let exportsNode;
-        let ruleIsFixable = false;
 
         return {
             AssignmentExpression(node) {
@@ -191,35 +175,13 @@ module.exports = {
                 }
             },
 
-            CallExpression(node) {
-
-                // If the rule has a call for `context.report` and a property `fix`
-                // is being passed in, then we consider that the rule is fixable.
-                //
-                // Note that we only look for context.report() calls in the new
-                // style (with single MessageDescriptor argument), because only
-                // calls in the new style can specify a fix.
-                if (node.callee.type === "MemberExpression" &&
-                    node.callee.object.type === "Identifier" &&
-                    node.callee.object.name === "context" &&
-                    node.callee.property.type === "Identifier" &&
-                    node.callee.property.name === "report" &&
-                    node.arguments.length === 1 &&
-                    node.arguments[0].type === "ObjectExpression") {
-
-                    if (getPropertyFromObject("fix", node.arguments[0])) {
-                        ruleIsFixable = true;
-                    }
-                }
-            },
-
             "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;
                 }
 
-                checkMetaValidity(context, exportsNode, ruleIsFixable);
+                checkMetaValidity(context, exportsNode);
             }
         };
     }
diff --git a/lib/rules/no-invalid-regexp.js b/lib/rules/no-invalid-regexp.js
index dcde234c6..45596f7ee 100644
--- a/lib/rules/no-invalid-regexp.js
+++ b/lib/rules/no-invalid-regexp.js
@@ -74,7 +74,8 @@ module.exports = {
                 } catch (e) {
                     context.report({
                         node,
-                        message: `${e.message}.`
+                        message: "{{message}}.",
+                        data: e
                     });
                 }
 
diff --git a/lib/rules/no-restricted-properties.js b/lib/rules/no-restricted-properties.js
index b6c584c57..e3a3d14e5 100644
--- a/lib/rules/no-restricted-properties.js
+++ b/lib/rules/no-restricted-properties.js
@@ -109,6 +109,7 @@ module.exports = {
             if (matchedObjectProperty) {
                 const message = matchedObjectProperty.message ? ` ${matchedObjectProperty.message}` : "";
 
+                // eslint-disable-next-line eslint-plugin/report-message-format
                 context.report({ node, message: "'{{objectName}}.{{propertyName}}' is restricted from being used.{{message}}", data: {
                     objectName,
                     propertyName,
@@ -117,6 +118,7 @@ module.exports = {
             } else if (globalMatchedProperty) {
                 const message = globalMatchedProperty.message ? ` ${globalMatchedProperty.message}` : "";
 
+                // eslint-disable-next-line eslint-plugin/report-message-format
                 context.report({ node, message: "'{{propertyName}}' is restricted from being used.{{message}}", data: {
                     propertyName,
                     message
diff --git a/lib/rules/no-return-await.js b/lib/rules/no-return-await.js
index 3b7324d8b..a7933460d 100644
--- a/lib/rules/no-return-await.js
+++ b/lib/rules/no-return-await.js
@@ -19,7 +19,7 @@ module.exports = {
             category: "Best Practices",
             recommended: false // TODO: set to true
         },
-        fixable: false,
+        fixable: null,
         schema: [
         ]
     },
diff --git a/lib/rules/nonblock-statement-body-position.js b/lib/rules/nonblock-statement-body-position.js
index d6ee126da..212e36a57 100644
--- a/lib/rules/nonblock-statement-body-position.js
+++ b/lib/rules/nonblock-statement-body-position.js
@@ -77,7 +77,7 @@ module.exports = {
                     message: "Expected a linebreak before this statement.",
                     fix: fixer => fixer.insertTextBefore(node, "\n")
                 });
-            } else if (tokenBefore.loc.end.line !== node.loc.end.line && option === "beside") {
+            } else if (tokenBefore.loc.end.line !== node.loc.start.line && option === "beside") {
                 context.report({
                     node,
                     message: "Expected no linebreak before this statement.",
diff --git a/lib/rules/object-shorthand.js b/lib/rules/object-shorthand.js
index 7c4275ffe..1d32d1722 100644
--- a/lib/rules/object-shorthand.js
+++ b/lib/rules/object-shorthand.js
@@ -368,11 +368,12 @@ module.exports = {
                 // Checks for property/method shorthand.
                 if (isConciseProperty) {
                     if (node.method && (APPLY_NEVER || AVOID_QUOTES && isStringLiteral(node.key))) {
+                        const message = APPLY_NEVER ? "Expected longform method syntax." : "Expected longform method syntax for string literal keys.";
 
                         // { x() {} } should be written as { x: function() {} }
                         context.report({
                             node,
-                            message: `Expected longform method syntax${APPLY_NEVER ? "" : " for string literal keys"}.`,
+                            message,
                             fix: fixer => makeFunctionLongform(fixer, node)
                         });
                     } else if (APPLY_NEVER) {
diff --git a/lib/rules/operator-assignment.js b/lib/rules/operator-assignment.js
index a6bbb177a..43f21abc8 100644
--- a/lib/rules/operator-assignment.js
+++ b/lib/rules/operator-assignment.js
@@ -135,7 +135,7 @@ module.exports = {
                                 const equalsToken = getOperatorToken(node);
                                 const operatorToken = getOperatorToken(expr);
                                 const leftText = sourceCode.getText().slice(node.range[0], equalsToken.range[0]);
-                                const rightText = sourceCode.getText().slice(operatorToken.range[1], node.range[1]);
+                                const rightText = sourceCode.getText().slice(operatorToken.range[1], expr.right.range[1]);
 
                                 return fixer.replaceText(node, `${leftText}${expr.operator}=${rightText}`);
                             }
diff --git a/lib/rules/prefer-destructuring.js b/lib/rules/prefer-destructuring.js
index 38e8e2903..d7fbc3559 100644
--- a/lib/rules/prefer-destructuring.js
+++ b/lib/rules/prefer-destructuring.js
@@ -89,7 +89,7 @@ module.exports = {
          * @returns {void}
          */
         function report(reportNode, type) {
-            context.report({ node: reportNode, message: `Use ${type} destructuring` });
+            context.report({ node: reportNode, message: "Use {{type}} destructuring.", data: { type } });
         }
 
         /**
diff --git a/lib/rules/yoda.js b/lib/rules/yoda.js
index 1a79522e7..bdb7d2a42 100644
--- a/lib/rules/yoda.js
+++ b/lib/rules/yoda.js
@@ -267,8 +267,8 @@ module.exports = {
             const operatorToken = sourceCode.getFirstTokenBetween(node.left, node.right, token => token.value === node.operator);
             const textBeforeOperator = sourceCode.getText().slice(sourceCode.getTokenBefore(operatorToken).range[1], operatorToken.range[0]);
             const textAfterOperator = sourceCode.getText().slice(operatorToken.range[1], sourceCode.getTokenAfter(operatorToken).range[0]);
-            const leftText = sourceCode.getText().slice(sourceCode.getFirstToken(node).range[0], sourceCode.getTokenBefore(operatorToken).range[1]);
-            const rightText = sourceCode.getText().slice(sourceCode.getTokenAfter(operatorToken).range[0], sourceCode.getLastToken(node).range[1]);
+            const leftText = sourceCode.getText().slice(node.range[0], sourceCode.getTokenBefore(operatorToken).range[1]);
+            const rightText = sourceCode.getText().slice(sourceCode.getTokenAfter(operatorToken).range[0], node.range[1]);
 
             return rightText + textBeforeOperator + OPERATOR_FLIP_MAP[operatorToken.value] + textAfterOperator + leftText;
         }
diff --git a/package.json b/package.json
index 72df096bc..cdd8cc0dc 100644
--- a/package.json
+++ b/package.json
@@ -81,6 +81,7 @@
     "coveralls": "^2.11.16",
     "dateformat": "^1.0.8",
     "ejs": "^2.3.3",
+    "eslint-plugin-eslint-plugin": "^0.7.1",
     "eslint-plugin-node": "^4.1.0",
     "eslint-release": "^0.10.0",
     "esprima": "^2.4.1",


Test patch:
diff --git a/tests/lib/internal-rules/internal-no-invalid-meta.js b/tests/lib/internal-rules/internal-no-invalid-meta.js
index 068e3f54a..6dd44ea2f 100644
--- a/tests/lib/internal-rules/internal-no-invalid-meta.js
+++ b/tests/lib/internal-rules/internal-no-invalid-meta.js
@@ -241,37 +241,6 @@ ruleTester.run("internal-no-invalid-meta", rule, {
                 line: 2,
                 column: 5
             }]
-        },
-        {
-            code: [
-                "module.exports = {",
-                "    meta: {",
-                "        docs: {",
-                "            description: 'some rule',",
-                "            category: 'Internal',",
-                "            recommended: false",
-                "        },",
-                "        schema: []",
-                "    },",
-                "    create: function(context) {",
-                "        return {",
-                "            Program: function(node) {",
-                "                context.report({",
-                "                    node: node,",
-                "                    fix: function(fixer) {",
-                "                        return fixer.insertTextAfter(node, ' ');",
-                "                    }",
-                "                });",
-                "            }",
-                "        };",
-                "    }",
-                "};"
-            ].join("\n"),
-            errors: [{
-                message: "Rule is fixable, but is missing a meta.fixable property.",
-                line: 2,
-                column: 5
-            }]
         }
     ]
 });
diff --git a/tests/lib/rules/nonblock-statement-body-position.js b/tests/lib/rules/nonblock-statement-body-position.js
index 37a3b2756..37dffb20f 100644
--- a/tests/lib/rules/nonblock-statement-body-position.js
+++ b/tests/lib/rules/nonblock-statement-body-position.js
@@ -32,6 +32,11 @@ ruleTester.run("nonblock-statement-body-position", rule, {
         "for (foo in bar) baz;",
         "for (foo of bar) baz;",
         "if (foo) bar; else baz;",
+        `
+            if (foo) bar(
+                baz
+            );
+        `,
         {
             code: "if (foo) bar();",
             options: ["beside"]
diff --git a/tests/lib/rules/operator-assignment.js b/tests/lib/rules/operator-assignment.js
index d586878d7..e0f3e4e13 100644
--- a/tests/lib/rules/operator-assignment.js
+++ b/tests/lib/rules/operator-assignment.js
@@ -149,6 +149,11 @@ ruleTester.run("operator-assignment", rule, {
         output: "x += y",
         options: ["always"],
         errors: EXPECTED_OPERATOR_ASSIGNMENT
+    }, {
+        code: "x = (x + y)",
+        output: "x += y",
+        options: ["always"],
+        errors: EXPECTED_OPERATOR_ASSIGNMENT
     }, {
         code: "x += y",
         output: "x = x + y",
diff --git a/tests/lib/rules/prefer-destructuring.js b/tests/lib/rules/prefer-destructuring.js
index ae0633408..54cd76d0d 100644
--- a/tests/lib/rules/prefer-destructuring.js
+++ b/tests/lib/rules/prefer-destructuring.js
@@ -81,21 +81,21 @@ ruleTester.run("prefer-destructuring", rule, {
         {
             code: "var foo = array[0];",
             errors: [{
-                message: "Use array destructuring",
+                message: "Use array destructuring.",
                 type: "VariableDeclarator"
             }]
         },
         {
             code: "foo = array[0];",
             errors: [{
-                message: "Use array destructuring",
+                message: "Use array destructuring.",
                 type: "AssignmentExpression"
             }]
         },
         {
             code: "var foo = object.foo;",
             errors: [{
-                message: "Use object destructuring",
+                message: "Use object destructuring.",
                 type: "VariableDeclarator"
             }]
         },
@@ -103,7 +103,7 @@ ruleTester.run("prefer-destructuring", rule, {
             code: "var foobar = object.bar;",
             options: [{ object: true }, { enforceForRenamedProperties: true }],
             errors: [{
-                message: "Use object destructuring",
+                message: "Use object destructuring.",
                 type: "VariableDeclarator"
             }]
         },
@@ -111,28 +111,28 @@ ruleTester.run("prefer-destructuring", rule, {
             code: "var foo = object[bar];",
             options: [{ object: true }, { enforceForRenamedProperties: true }],
             errors: [{
-                message: "Use object destructuring",
+                message: "Use object destructuring.",
                 type: "VariableDeclarator"
             }]
         },
         {
             code: "var foo = array['foo'];",
             errors: [{
-                message: "Use object destructuring",
+                message: "Use object destructuring.",
                 type: "VariableDeclarator"
             }]
         },
         {
             code: "foo = array.foo;",
             errors: [{
-                message: "Use object destructuring",
+                message: "Use object destructuring.",
                 type: "AssignmentExpression"
             }]
         },
         {
             code: "foo = array['foo'];",
             errors: [{
-                message: "Use object destructuring",
+                message: "Use object destructuring.",
                 type: "AssignmentExpression"
             }]
         }