Repository: eslint/eslint
Pull request #8486 (2017-04-20 22:29:08)

Docs: Update Migrating to v4.0.0 with new comment getters
**What is the purpose of this pull request? (put an "X" next to item)**

[x] Documentation update
[ ] 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.)
-->

<!--
    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)**
This updates the docs to include information about the new comment getters and their deprecated counterparts.

**Is there anything you'd like reviewers to focus on?**
Is this clear enough?



mention-bot: @kaicataldo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @not-an-aardvark to be a potential reviewer.
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM
eslintbot: LGTM

Patch:
diff --git a/docs/rules/object-curly-newline.md b/docs/rules/object-curly-newline.md
index a548f4eac..40f5368e5 100644
--- a/docs/rules/object-curly-newline.md
+++ b/docs/rules/object-curly-newline.md
@@ -16,7 +16,8 @@ This rule has either a string option:
 Or an object option:
 
 * `"multiline": true` (default) requires line breaks if there are line breaks inside properties or between properties
-* `"minProperties"` requires line breaks if the number of properties is more than the given integer
+* `"minProperties"` requires line breaks if the number of properties is more than the given integer. By default, an error will also be reported if an object contains linebreaks and has fewer properties than the given integer. However, the second behavior is disabled if the `consistent` option is set to `true`
+* `"consistent": true` requires that either both curly braces, or neither, directly enclose newlines. Note that enabling this option will also change the behavior of the `minProperties` option. (See `minProperties` above for more information)
 
 You can specify different options for object literals and destructuring assignments:
 
@@ -312,6 +313,86 @@ let {k = function() {
 }} = obj;
 ```
 
+### consistent
+
+Examples of **incorrect** code for this rule with the `{ "consistent": true }` option:
+
+```js
+/*eslint object-curly-newline: ["error", { "consistent": true }]*/
+/*eslint-env es6*/
+
+let a = {foo: 1
+};
+let b = {
+    foo: 1};
+let c = {foo: 1, bar: 2
+};
+let d = {
+    foo: 1, bar: 2};
+let e = {foo: 1,
+    bar: 2};
+let f = {foo: function() {
+    dosomething();
+}};
+
+let {g
+} = obj;
+let {
+    h} = obj;
+let {i, j
+} = obj;
+let {
+    k, l} = obj;
+let {m,
+    n} = obj;
+let {o = function() {
+    dosomething();
+}} = obj;
+```
+
+Examples of **correct** code for this rule with the `{ "consistent": true }` option:
+
+```js
+/*eslint object-curly-newline: ["error", { "consistent": true }]*/
+/*eslint-env es6*/
+
+let a = {};
+let b = {foo: 1};
+let c = {
+    foo: 1
+};
+let d = {
+    foo: 1, bar: 2
+};
+let e = {
+    foo: 1,
+    bar: 2
+};
+let f = {foo: function() {dosomething();}};
+let g = {
+    foo: function() {
+        dosomething();
+    }
+};
+
+let {} = obj;
+let {h} = obj;
+let {i, j} = obj;
+let {
+    k, l
+} = obj;
+let {
+    m,
+    n
+} = obj;
+let {o = function() {dosomething();}} = obj;
+let {
+    p = function() {
+        dosomething();
+    }
+} = obj;
+```
+
 ### ObjectExpression and ObjectPattern
 
 Examples of **incorrect** code for this rule with the `{ "ObjectExpression": "always", "ObjectPattern": "never" }` options:
diff --git a/docs/user-guide/migrating-to-4.0.0.md b/docs/user-guide/migrating-to-4.0.0.md
index e616dd56f..84accfea4 100644
--- a/docs/user-guide/migrating-to-4.0.0.md
+++ b/docs/user-guide/migrating-to-4.0.0.md
@@ -165,17 +165,45 @@ Starting in 4.0, the `RuleTester` utility will validate properties of test case
 
 Prior to 4.0, ESLint required parsers to implement comment attachment, a process where AST nodes would gain additional properties corresponding to their leading and trailing comments in the source file. This made it difficult for users to develop custom parsers, because they would have to replicate the confusing comment attachment semantics required by ESLint.
 
-In 4.0, comment attachment logic has been moved into ESLint itself. This should make it easier to develop custom parsers, but it also means that AST nodes will no longer have `leadingComments` and `trailingComments` properties. Additionally, `LineComment` and `BlockComment` events will no longer be emitted during AST traversal.
+In 4.0, we have moved away from the concept of comment attachment and have moved all comment handling logic into ESLint itself. This should make it easier to develop custom parsers, but it also means that AST nodes will no longer have `leadingComments` and `trailingComments` properties. Conceptually, rule authors can now think of comments in the context of tokens rather than AST nodes.
 
-**To address:** If you have a custom rule that depends on the `leadingComments` or `trailingComments` properties of an AST node, you can switch to `sourceCode.getComments(node).leading` or `sourceCode.getComments(node).trailing` instead. You should also consider using `sourceCode.getAllComments()` or `sourceCode.getTokenBefore(node, { includeComments: true })`.
+**To address:** If you have a custom rule that depends on the `leadingComments` or `trailingComments` properties of an AST node, you can now use `sourceCode.getCommentsBefore()` and `sourceCode.getCommentsAfter()` instead, respectively.
+
+Additionally, the `sourceCode` object now also has `sourceCode.getCommentsInside()` (which returns all the comments inside a node), `sourceCode.getAllComments()` (which returns all the comments in the file), and allows comments to be accessed through various other token iterator methods (such as `getTokenBefore()` and `getTokenAfter()`) with the `{ includeComments: true }` option.
+
+For rule authors concerned about supporting ESLint v3.0 in addition to v4.0, the now deprecated `sourceCode.getComments()` is still available and will work for both versions.
+
+Finally, please note that the following `SourceCode` methods have been deprecated and will be removed in a future version of ESLint:
+
+* `getComments()` - replaced by `getCommentsBefore()`, `getCommentsAfter()`, and `getCommentsInside()`
+* `getTokenOrCommentBefore()` - replaced by `getTokenBefore()` with the `{ includeComments: true }` option
+* `getTokenOrCommentAfter()` - replaced by `getTokenAfter()` with the `{ includeComments: true }` option
+
+## <a name="event-comments"/> `LineComment` and `BlockComment` events will no longer be emitted during AST traversal
+
+Starting in 4.0, `LineComment` and `BlockComments` events will not be emitted during AST traversal. There are two reasons for this:
+
+* This behavior was relying on comment attachment happening at the parser level, which does not happen anymore, to ensure that all comments would be accounted for
+* Thinking of comments in the context of tokens is more predictable and easier to reason about than thinking about comment tokens in the context of AST nodes
+
+**To address:** Instead of relying on `LineComment` and `BlockComment`, rules can now use `sourceCode.getAllComments()` to get all comments in a file. To check all comments of a specific type, rules can use the following pattern:
+
+```
+sourceCode.getAllComments().filter(comment => comment.type === "Line");
+sourceCode.getAllComments().filter(comment => comment.type === "Block");
+```
 
 ## <a name="shebangs"/> Shebangs are now returned from comment APIs
 
 Prior to 4.0, shebang comments in a source file would not appear in the output of `sourceCode.getAllComments()` or `sourceCode.getComments()`, but they would appear in the output of `sourceCode.getTokenOrCommentBefore` as line comments. This inconsistency led to some confusion for rule developers.
 
-In 4.0, shebang comments are included in the results of all of these methods. Instead of being converted to line comments, they will now have the `Shebang` type.
+In 4.0, shebang comments are treated as comment tokens of type `Shebang` and will be returned by any `SourceCode` method that returns comments. The goal of this change is to make working with shebang comments more consistent with how other tokens are handled.
 
-**To address:** If you have a custom rule that performs operations on comments, make sure to handle shebang comments appropriately.
+**To address:** If you have a custom rule that performs operations on comments, some additional logic might be required to ensure that shebang comments are correctly handled or filtered out:
+
+```
+sourceCode.getAllComments().filter(comment => comment.type !== "Shebang");
+```
 
 ## <a name="type-annotation-traversal"/> Type annotation nodes in an AST are now traversed
 
diff --git a/lib/rules/object-curly-newline.js b/lib/rules/object-curly-newline.js
index a4451154d..b78cb9cfc 100644
--- a/lib/rules/object-curly-newline.js
+++ b/lib/rules/object-curly-newline.js
@@ -30,6 +30,9 @@ const OPTION_VALUE = {
                 minProperties: {
                     type: "integer",
                     minimum: 0
+                },
+                consistent: {
+                    type: "boolean"
                 }
             },
             additionalProperties: false,
@@ -42,11 +45,12 @@ const OPTION_VALUE = {
  * Normalizes a given option value.
  *
  * @param {string|Object|undefined} value - An option value to parse.
- * @returns {{multiline: boolean, minProperties: number}} Normalized option object.
+ * @returns {{multiline: boolean, minProperties: number, consistent: boolean}} Normalized option object.
  */
 function normalizeOptionValue(value) {
     let multiline = false;
     let minProperties = Number.POSITIVE_INFINITY;
+    let consistent = false;
 
     if (value) {
         if (value === "always") {
@@ -56,12 +60,13 @@ function normalizeOptionValue(value) {
         } else {
             multiline = Boolean(value.multiline);
             minProperties = value.minProperties || Number.POSITIVE_INFINITY;
+            consistent = Boolean(value.consistent);
         }
     } else {
         multiline = true;
     }
 
-    return { multiline, minProperties };
+    return { multiline, minProperties, consistent };
 }
 
 /**
@@ -172,7 +177,14 @@ module.exports = {
                     });
                 }
             } else {
-                if (!astUtils.isTokenOnSameLine(openBrace, first)) {
+                const consistent = options.consistent;
+                const hasLineBreakBetweenOpenBraceAndFirst = !astUtils.isTokenOnSameLine(openBrace, first);
+                const hasLineBreakBetweenCloseBraceAndLast = !astUtils.isTokenOnSameLine(last, closeBrace);
+
+                if (
+                    (!consistent && hasLineBreakBetweenOpenBraceAndFirst) ||
+                    (consistent && hasLineBreakBetweenOpenBraceAndFirst && !hasLineBreakBetweenCloseBraceAndLast)
+                ) {
                     context.report({
                         message: "Unexpected line break after this opening brace.",
                         node,
@@ -185,7 +197,10 @@ module.exports = {
                         }
                     });
                 }
-                if (!astUtils.isTokenOnSameLine(last, closeBrace)) {
+                if (
+                    (!consistent && hasLineBreakBetweenCloseBraceAndLast) ||
+                    (consistent && !hasLineBreakBetweenOpenBraceAndFirst && hasLineBreakBetweenCloseBraceAndLast)
+                ) {
                     context.report({
                         message: "Unexpected line break before this closing brace.",
                         node,


Test patch:
diff --git a/tests/lib/rules/object-curly-newline.js b/tests/lib/rules/object-curly-newline.js
index 4d8859742..05376e0d4 100644
--- a/tests/lib/rules/object-curly-newline.js
+++ b/tests/lib/rules/object-curly-newline.js
@@ -236,6 +236,162 @@ ruleTester.run("object-curly-newline", rule, {
             options: [{ multiline: true, minProperties: 2 }]
         },
 
+        // "consistent" ------------------------------------------
+        {
+            code: [
+                "var b = {",
+                "    a: 1",
+                "};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }]
+        },
+        {
+            code: [
+                "var c = {a: 1, b: 2};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }]
+        },
+        {
+            code: [
+                "var c = {",
+                "    a: 1,",
+                "    b: 2",
+                "};"
+            ].join("\n"),
+
+            options: [{ multiline: true, consistent: true }]
+        },
+        {
+            code: [
+                "var e = {a: function() { dosomething();}};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }]
+        },
+        {
+            code: [
+                "var e = {",
+                "    a: function() { dosomething();}",
+                "};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }]
+        },
+        {
+            code: [
+                "let {} = {a: 1};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: [
+                "let {a} = {a: 1};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: [
+                "let {",
+                "} = {a: 1};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: [
+                "let {",
+                "    a",
+                "} = {a: 1};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: [
+                "let {a, b} = {a: 1, b: 1};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: [
+                "let {",
+                "    a, b",
+                "} = {a: 1, b: 1};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: [
+                "let {k = function() {dosomething();}} = obj;"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: [
+                "let {",
+                "    k = function() {",
+                "        dosomething();",
+                "    }",
+                "} = obj;"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: [
+                "var c = {a: 1,",
+                "b: 2};"
+            ].join("\n"),
+            options: [{ multiline: false, consistent: true }]
+        },
+        {
+            code: [
+                "let {a,",
+                "b} = {a: 1, b: 1};"
+            ].join("\n"),
+            options: [{ multiline: false, consistent: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+
+        // "consistent" and "minProperties" ------------------------------------------
+        {
+            code: [
+                "var c = { a: 1 };"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true, minProperties: 2 }]
+        },
+        {
+            code: [
+                "var c = {",
+                "a: 1",
+                "};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true, minProperties: 2 }]
+        },
+        {
+            code: [
+                "let {a} = {",
+                "a: 1",
+                "};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true, minProperties: 2 }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: [
+                "let {",
+                "a",
+                "} = {",
+                "a: 1",
+                "};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true, minProperties: 2 }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+
         // "ObjectExpression" and "ObjectPattern" ---------------------------------------------
         {
             code: [
@@ -659,6 +815,289 @@ ruleTester.run("object-curly-newline", rule, {
             ]
         },
 
+        // "consistent" ------------------------------------------
+        {
+            code: [
+                "var b = {a: 1",
+                "};"
+            ].join("\n"),
+            output: [
+                "var b = {a: 1};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            errors: [
+                { line: 2, column: 1, message: "Unexpected line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "var b = {",
+                "a: 1};"
+            ].join("\n"),
+            output: [
+                "var b = {a: 1};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            errors: [
+                { line: 1, column: 9, message: "Unexpected line break after this opening brace." }
+            ]
+        },
+        {
+            code: [
+                "var c = {a: 1, b: 2",
+                "};"
+            ].join("\n"),
+            output: [
+                "var c = {a: 1, b: 2};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            errors: [
+                { line: 2, column: 1, message: "Unexpected line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "var c = {",
+                "a: 1, b: 2};"
+            ].join("\n"),
+            output: [
+                "var c = {a: 1, b: 2};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            errors: [
+                { line: 1, column: 9, message: "Unexpected line break after this opening brace." }
+            ]
+        },
+        {
+            code: [
+                "var c = {a: 1,",
+                "b: 2};"
+            ].join("\n"),
+            output: [
+                "var c = {",
+                "a: 1,",
+                "b: 2",
+                "};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            errors: [
+                { line: 1, column: 9, message: "Expected a line break after this opening brace." },
+                { line: 2, column: 5, message: "Expected a line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "var e = {a: function() {",
+                "dosomething();",
+                "}};"
+            ].join("\n"),
+            output: [
+                "var e = {",
+                "a: function() {",
+                "dosomething();",
+                "}",
+                "};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            errors: [
+                { line: 1, column: 9, message: "Expected a line break after this opening brace." },
+                { line: 3, column: 2, message: "Expected a line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "let {a",
+                "} = {a: 1}"
+            ].join("\n"),
+            output: [
+                "let {a} = {a: 1}"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { line: 2, column: 1, message: "Unexpected line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "let {",
+                "a} = {a: 1}"
+            ].join("\n"),
+            output: [
+                "let {a} = {a: 1}"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { line: 1, column: 5, message: "Unexpected line break after this opening brace." }
+            ]
+        },
+        {
+            code: [
+                "let {a, b",
+                "} = {a: 1, b: 2}"
+            ].join("\n"),
+            output: [
+                "let {a, b} = {a: 1, b: 2}"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { line: 2, column: 1, message: "Unexpected line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "let {",
+                "a, b} = {a: 1, b: 2}"
+            ].join("\n"),
+            output: [
+                "let {a, b} = {a: 1, b: 2}"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { line: 1, column: 5, message: "Unexpected line break after this opening brace." }
+            ]
+        },
+        {
+            code: [
+                "let {a,",
+                "b} = {a: 1, b: 2}"
+            ].join("\n"),
+            output: [
+                "let {",
+                "a,",
+                "b",
+                "} = {a: 1, b: 2}"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { line: 1, column: 5, message: "Expected a line break after this opening brace." },
+                { line: 2, column: 2, message: "Expected a line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "let {e = function() {",
+                "dosomething();",
+                "}} = a;"
+            ].join("\n"),
+            output: [
+                "let {",
+                "e = function() {",
+                "dosomething();",
+                "}",
+                "} = a;"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { line: 1, column: 5, message: "Expected a line break after this opening brace." },
+                { line: 3, column: 2, message: "Expected a line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "var c = {",
+                "a: 1,",
+                "b: 2};"
+            ].join("\n"),
+            output: [
+                "var c = {a: 1,",
+                "b: 2};"
+            ].join("\n"),
+            options: [{ multiline: false, consistent: true }],
+            errors: [
+                { line: 1, column: 9, message: "Unexpected line break after this opening brace." }
+            ]
+        },
+        {
+            code: [
+                "var c = {a: 1,",
+                "b: 2",
+                "};"
+            ].join("\n"),
+            output: [
+                "var c = {a: 1,",
+                "b: 2};"
+            ].join("\n"),
+            options: [{ multiline: false, consistent: true }],
+            errors: [
+                { line: 3, column: 1, message: "Unexpected line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "let {",
+                "a,",
+                "b} = {a: 1, b: 2};"
+            ].join("\n"),
+            output: [
+                "let {a,",
+                "b} = {a: 1, b: 2};"
+            ].join("\n"),
+            options: [{ multiline: false, consistent: true }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { line: 1, column: 5, message: "Unexpected line break after this opening brace." }
+            ]
+        },
+        {
+            code: [
+                "let {a,",
+                "b",
+                "} = {a: 1, b: 2};"
+            ].join("\n"),
+            output: [
+                "let {a,",
+                "b} = {a: 1, b: 2};"
+            ].join("\n"),
+            options: [{ multiline: false, consistent: true }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { line: 3, column: 1, message: "Unexpected line break before this closing brace." }
+            ]
+        },
+
+        // "consistent" and "minProperties" ------------------------------------------
+        {
+            code: [
+                "var c = {a: 1, b: 2};"
+            ].join("\n"),
+            output: [
+                "var c = {",
+                "a: 1, b: 2",
+                "};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true, minProperties: 2 }],
+            errors: [
+                { line: 1, column: 9, message: "Expected a line break after this opening brace." },
+                { line: 1, column: 20, message: "Expected a line break before this closing brace." }
+            ]
+        },
+        {
+            code: [
+                "let {a, b} = {",
+                "a: 1, b: 2",
+                "};"
+            ].join("\n"),
+            output: [
+                "let {",
+                "a, b",
+                "} = {",
+                "a: 1, b: 2",
+                "};"
+            ].join("\n"),
+            options: [{ multiline: true, consistent: true, minProperties: 2 }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                { line: 1, column: 5, message: "Expected a line break after this opening brace." },
+                { line: 1, column: 10, message: "Expected a line break before this closing brace." }
+            ]
+        },
+
         // "ObjectExpression" and "ObjectPattern" ---------------------------------------------
         {
             code: [