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.

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

Fix: ignore aligning single line in key-spacing (fixes #11414) #12652

Merged
merged 3 commits into from Dec 20, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
49 changes: 34 additions & 15 deletions lib/rules/key-spacing.js
Expand Up @@ -42,6 +42,18 @@ function isSingleLine(node) {
return (node.loc.end.line === node.loc.start.line);
}

/**
* Checks whether the properties on a single line.
* @param {ASTNode[]} properties List of Property AST nodes.
* @returns {boolean} True if all properies is on a single line.
*/
function isSingleLineProperties(properties) {
const [firstProp] = properties,
lastProp = last(properties);

return firstProp.loc.start.line === lastProp.loc.end.line;
}

/**
* Initializes a single option property from the configuration with defaults for undefined values
* @param {Object} toOptions Object to be initialized
Expand Down Expand Up @@ -583,17 +595,6 @@ module.exports = {
}
}

/**
* Verifies vertical alignment, taking into account groups of properties.
* @param {ASTNode} node ObjectExpression node being evaluated.
* @returns {void}
*/
function verifyAlignment(node) {
createGroups(node).forEach(group => {
verifyGroupAlignment(group.filter(isKeyValueProperty));
});
}

/**
* Verifies spacing of property conforms to specified options.
* @param {ASTNode} node Property node being evaluated.
Expand All @@ -611,17 +612,35 @@ module.exports = {

/**
* Verifies spacing of each property in a list.
* @param {ASTNode[]} properties List of Property AST nodes.
* @param {ASTNode[]} properties List of Property AST nodes.
* @param {Object} lineOptions Configured singleLine or multiLine options
* @returns {void}
*/
function verifyListSpacing(properties) {
function verifyListSpacing(properties, lineOptions) {
const length = properties.length;

for (let i = 0; i < length; i++) {
verifySpacing(properties[i], singleLineOptions);
verifySpacing(properties[i], lineOptions);
}
}

/**
* Verifies vertical alignment, taking into account groups of properties.
* @param {ASTNode} node ObjectExpression node being evaluated.
* @returns {void}
*/
function verifyAlignment(node) {
createGroups(node).forEach(group => {
const properties = group.filter(isKeyValueProperty);

if (properties.length > 0 && isSingleLineProperties(properties)) {
kaicataldo marked this conversation as resolved.
Show resolved Hide resolved
verifyListSpacing(properties, multiLineOptions);
} else {
verifyGroupAlignment(properties);
}
});
}

//--------------------------------------------------------------------------
// Public API
//--------------------------------------------------------------------------
Expand All @@ -631,7 +650,7 @@ module.exports = {
return {
ObjectExpression(node) {
if (isSingleLine(node)) {
verifyListSpacing(node.properties.filter(isKeyValueProperty));
verifyListSpacing(node.properties.filter(isKeyValueProperty), singleLineOptions);
} else {
verifyAlignment(node);
}
Expand Down
80 changes: 80 additions & 0 deletions tests/lib/rules/key-spacing.js
Expand Up @@ -757,6 +757,34 @@ ruleTester.run("key-spacing", rule, {
}
}],
parserOptions: { ecmaVersion: 6 }
}, {
code: [
"var obj = {",
" foo : 1, 'bar' : 2, baz : 3, longlonglong : 4",
kaicataldo marked this conversation as resolved.
Show resolved Hide resolved
"}"
].join("\n"),
options: [{
singleLine: {
beforeColon: false,
afterColon: false
},
multiLine: {
beforeColon: true,
afterColon: true,
align: "colon"
}
}]
}, {
code: [
"var obj = {",
" foo: 1, 'bar': 2, baz: 3",
"}"
].join("\n"),
options: [{
multiLine: {
align: "value"
}
}]
}],

invalid: [{
Expand Down Expand Up @@ -1769,5 +1797,57 @@ ruleTester.run("key-spacing", rule, {
{ messageId: "missingKey", data: { computed: "", key: "foo" }, line: 1, column: 7, type: "Identifier" },
{ messageId: "missingValue", data: { computed: "", key: "foo" }, line: 1, column: 19, type: "Identifier" }
]
}, {
code: [
"var obj = {",
" foo:1, 'bar':2, baz:3",
"}"
].join("\n"),
output: [
"var obj = {",
" foo : 1, 'bar' : 2, baz : 3",
"}"
].join("\n"),
options: [{
singleLine: {
beforeColon: false,
afterColon: false
},
multiLine: {
beforeColon: true,
afterColon: true,
align: "colon"
}
}],
errors: [
{ messageId: "missingKey", data: { computed: "", key: "foo" }, line: 2, column: 5, type: "Identifier" },
{ messageId: "missingValue", data: { computed: "", key: "foo" }, line: 2, column: 9, type: "Literal" },
{ messageId: "missingKey", data: { computed: "", key: "bar" }, line: 2, column: 12, type: "Literal" },
{ messageId: "missingValue", data: { computed: "", key: "bar" }, line: 2, column: 18, type: "Literal" },
{ messageId: "missingKey", data: { computed: "", key: "baz" }, line: 2, column: 21, type: "Identifier" },
{ messageId: "missingValue", data: { computed: "", key: "baz" }, line: 2, column: 25, type: "Literal" }
]
}, {
code: [
"var obj = {",
" foo : 1, 'bar' : 2, baz : 3, longlonglong : 4",
"}"
].join("\n"),
output: [
"var obj = {",
" foo: 1, 'bar': 2, baz: 3, longlonglong: 4",
"}"
].join("\n"),
options: [{
multiLine: {
align: "value"
}
}],
errors: [
{ messageId: "extraKey", data: { computed: "", key: "foo" }, line: 2, column: 5, type: "Identifier" },
{ messageId: "extraKey", data: { computed: "", key: "bar" }, line: 2, column: 14, type: "Literal" },
{ messageId: "extraKey", data: { computed: "", key: "baz" }, line: 2, column: 25, type: "Identifier" },
{ messageId: "extraKey", data: { computed: "", key: "longlonglong" }, line: 2, column: 34, type: "Identifier" }
]
}]
});