Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update: Add "consistent" option to array-element-newline (fixes #9457) #10355

Merged
merged 1 commit into from May 28, 2018
+189 −3
Merged

Update: Add "consistent" option to array-element-newline (fixes #9457) #10355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 64 additions & 0 deletions docs/rules/array-element-newline.md
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ This rule has either a string option:

* `"always"` (default) requires line breaks between array elements
* `"never"` disallows line breaks between array elements
* `"consistent"` requires consistent usage of linebreaks between array elements

Or an object option (Requires line breaks if any of properties is satisfied. Otherwise, disallows line breaks):

Expand Down Expand Up @@ -102,6 +103,69 @@ var e = [
];
```

### consistent

Examples of **incorrect** code for this rule with the `"consistent"` option:

```js
/*eslint array-element-newline: ["error", "consistent"]*/

var a = [
1, 2,
3
];
var b = [
function foo() {
dosomething();
}, function bar() {
dosomething();
},
function baz() {
dosomething();
}
];
```

Examples of **correct** code for this rule with the `"consistent"` option:

```js
/*eslint array-element-newline: ["error", "consistent"]*/

var a = [];
var b = [1];
var c = [1, 2];
var d = [1, 2, 3];
var e = [
1,
2
];
var f = [
1,
2,
3
];
var g = [
function foo() {
dosomething();
}, function bar() {
dosomething();
}, function baz() {
dosomething();
}
];
var h = [
function foo() {
dosomething();
},
function bar() {
dosomething();
},
function baz() {
dosomething();
}
];
```

### multiline

Examples of **incorrect** code for this rule with the `{ "multiline": true }` option:
Expand Down
27 changes: 25 additions & 2 deletions lib/rules/array-element-newline.js
View file
Open in desktop
Expand Up @@ -24,7 +24,7 @@ module.exports = {
{
oneOf: [
{
enum: ["always", "never"]
enum: ["always", "never", "consistent"]
},
{
type: "object",
Expand Down Expand Up @@ -63,6 +63,7 @@ module.exports = {
* @returns {{multiline: boolean, minItems: number}} Normalized option object.
*/
function normalizeOptionValue(providedOption) {
let consistent = false;
let multiline = false;
let minItems;

Expand All @@ -72,12 +73,15 @@ module.exports = {
minItems = 0;
} else if (option === "never") {
minItems = Number.POSITIVE_INFINITY;
} else if (option === "consistent") {
consistent = true;
minItems = Number.POSITIVE_INFINITY;
} else {
multiline = Boolean(option.multiline);
minItems = option.minItems || Number.POSITIVE_INFINITY;
}

return { multiline, minItems };
return { consistent, multiline, minItems };
}

/**
Expand Down Expand Up @@ -193,11 +197,30 @@ module.exports = {
.some(element => element.loc.start.line !== element.loc.end.line);
}

const linebreaksCount = node.elements.map((element, i) => {
const previousElement = elements[i - 1];

if (i === 0 || element === null || previousElement === null) {
return false;
}

const commaToken = sourceCode.getFirstTokenBetween(previousElement, element, astUtils.isCommaToken);
const lastTokenOfPreviousElement = sourceCode.getTokenBefore(commaToken);
const firstTokenOfCurrentElement = sourceCode.getTokenAfter(commaToken);

return !astUtils.isTokenOnSameLine(lastTokenOfPreviousElement, firstTokenOfCurrentElement);
}).filter(isBreak => isBreak === true).length;

const needsLinebreaks = (
elements.length >= options.minItems ||
(
options.multiline &&
elementBreak
) ||
(
options.consistent &&
linebreaksCount > 0 &&
linebreaksCount < node.elements.length
)
);

Expand Down
101 changes: 100 additions & 1 deletion tests/lib/rules/array-element-newline.js
View file
Open in desktop
Expand Up @@ -53,7 +53,7 @@ ruleTester.run("array-element-newline", rule, {
{ code: "var foo = [// any comment \n1,\n2];", options: ["always"] },
{ code: "var foo = [1,\n2 // any comment\n];", options: ["always"] },
{ code: "var foo = [1,\n2,\n3];", options: ["always"] },
{ code: "var foo = [\nfunction foo() {\ndosomething();\n},\nfunction bar() {\nosomething();\n}\n];", options: ["always"] },
{ code: "var foo = [\nfunction foo() {\ndosomething();\n},\nfunction bar() {\ndosomething();\n}\n];", options: ["always"] },

// "never"
{ code: "var foo = [];", options: ["never"] },
Expand All @@ -66,6 +66,23 @@ ruleTester.run("array-element-newline", rule, {
{ code: "var foo = [1, (\n2\n), 3];", options: ["never"] },
{ code: "var foo = [\nfunction foo() {\ndosomething();\n}, function bar() {\ndosomething();\n}\n];", options: ["never"] },

// "consistent"
{ code: "var foo = [];", options: ["consistent"] },
{ code: "var foo = [1];", options: ["consistent"] },
{ code: "var foo = [1, 2];", options: ["consistent"] },
{ code: "var foo = [1,\n2];", options: ["consistent"] },
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, why is this consistent? I'd expect "consistent" to require either "every item is on the same line, along with the brackets" or "every item is on its own line, as is each bracket" - but nothing in between.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think brackets are handled separately with array-bracket-newline. array-element-newline just deals with elements and not brackets.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule should not concern the style of brackets.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, ok - i suppose with "multiline", array-bracket-newline would work in concert with this rule and "consistent" to do what I'd expect.

{ code: "var foo = [1, 2, 3];", options: ["consistent"] },
{ code: "var foo = [1,\n2,\n3];", options: ["consistent"] },
{ code: "var foo = [1,\n2,\n,\n3];", options: ["consistent"] },
{ code: "var foo = [1, // any comment\n2];", options: ["consistent"] },
{ code: "var foo = [/* any comment */ 1, 2];", options: ["consistent"] },
{ code: "var foo = [1, (\n2\n), 3];", options: ["consistent"] },
{ code: "var foo = [1,\n(2)\n, 3];", options: ["consistent"] },
{ code: "var foo = [\nfunction foo() {\ndosomething();\n},\nfunction bar() {\ndosomething();\n}\n];", options: ["consistent"] },
{ code: "var foo = [\nfunction foo() {\ndosomething();\n}, function bar() {\ndosomething();\n}\n];", options: ["consistent"] },
{ code: "var foo = [\nfunction foo() {\ndosomething();\n},\nfunction bar() {\ndosomething();\n},\nfunction bar() {\ndosomething();\n}];", options: ["consistent"] },
{ code: "var foo = [\nfunction foo() {\ndosomething();\n}, function bar() {\ndosomething();\n}, function bar() {\ndosomething();\n}];", options: ["consistent"] },

// { multiline: true }
{ code: "var foo = [];", options: [{ multiline: true }] },
{ code: "var foo = [1];", options: [{ multiline: true }] },
Expand Down Expand Up @@ -450,6 +467,88 @@ ruleTester.run("array-element-newline", rule, {
]
},

// "consistent"
{
code: "var foo = [1,\n2, 3];",
output: "var foo = [1,\n2,\n3];",
options: ["consistent"],
errors: [
{
messageId: "missingLineBreak",
line: 2,
column: 3,
endLine: 2,
endColumn: 4
}
]
},
{
code: "var foo = [1, 2,\n3];",
output: "var foo = [1,\n2,\n3];",
options: ["consistent"],
errors: [
{
messageId: "missingLineBreak",
line: 1,
column: 14,
endLine: 1,
endColumn: 15
}
]
},
{
code: "var foo = [1,\n(\n2), 3];",
output: "var foo = [1,\n(\n2),\n3];",
options: ["consistent"],
errors: [
{
messageId: "missingLineBreak",
line: 3,
column: 4,
endLine: 3,
endColumn: 5
}
]
},
{
code: "var foo = [1, \t (\n2\n),\n3];",
output: "var foo = [1,\n(\n2\n),\n3];",
options: ["consistent"],
errors: [
{
messageId: "missingLineBreak",
line: 1,
column: 14,
endLine: 1,
endColumn: 29
}
]
},
{
code: "var foo = [1, /* any comment */(2),\n3];",
output: "var foo = [1, /* any comment */\n(2),\n3];",
options: ["consistent"],
errors: [
{
messageId: "missingLineBreak",
line: 1,
column: 32
}
]
},
{
code: "var foo = [\nfunction foo() {\ndosomething();\n},function bar() {\ndosomething();\n},\nfunction bar() {\ndosomething();\n}];",
output: "var foo = [\nfunction foo() {\ndosomething();\n},\nfunction bar() {\ndosomething();\n},\nfunction bar() {\ndosomething();\n}];",
options: ["consistent"],
errors: [
{
messageId: "missingLineBreak",
line: 4,
column: 3
}
]
},

// { multiline: true }
{
code: "var foo = [1,\n2, 3];",
Expand Down