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: uglified autofix in key-spacing (fixes #11414) #12651

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
65 changes: 45 additions & 20 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 all members in the group are on a single line.
* @param {ASTNode[]} group List of Property AST nodes.
* @returns {boolean} True if all memebers are on a single line.
*/
function isSingleLineGroup(group) {
const [firstMember] = group,
lastMemeber = last(group);

return firstMember.loc.start.line === lastMemeber.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 @@ -321,18 +333,24 @@ module.exports = {

/**
* Checks whether a property is a member of the property group it follows.
* @param {ASTNode} lastMember The last Property known to be in the group.
* @param {ASTNode} group The precede group.
* @param {ASTNode} candidate The next Property that might be in the group.
* @returns {boolean} True if the candidate property is part of the group.
*/
function continuesPropertyGroup(lastMember, candidate) {
const groupEndLine = lastMember.loc.start.line,
candidateStartLine = candidate.loc.start.line;
function continuesPropertyGroup(group, candidate) {
const lastMember = last(group),
groupEndLine = lastMember.loc.start.line,
candidateStartLine = candidate.loc.start.line,
lineDiff = candidateStartLine - groupEndLine;

if (candidateStartLine - groupEndLine <= 1) {
if (lineDiff === 0) {
return true;
}

if (lineDiff === 1) {
return group.length <= 1 || !isSingleLineGroup(group);
}

/*
* Check that the first comment is adjacent to the end of the group, the
* last comment is adjacent to the candidate property, and that successive
Expand Down Expand Up @@ -528,7 +546,7 @@ module.exports = {
const currentGroup = last(groups),
prev = last(currentGroup);

if (!prev || continuesPropertyGroup(prev, property)) {
if (!prev || continuesPropertyGroup(currentGroup, property)) {
currentGroup.push(property);
} else {
groups.push([property]);
Expand Down Expand Up @@ -583,17 +601,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 @@ -612,16 +619,34 @@ module.exports = {
/**
* Verifies spacing of each property in a list.
* @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 targetGroup = group.filter(isKeyValueProperty);

if (targetGroup.length && isSingleLineGroup(targetGroup)) {
verifyListSpacing(targetGroup, multiLineOptions);
} else {
verifyGroupAlignment(targetGroup);
}
});
}

//--------------------------------------------------------------------------
// Public API
//--------------------------------------------------------------------------
Expand All @@ -631,7 +656,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
262 changes: 262 additions & 0 deletions tests/lib/rules/key-spacing.js
Expand Up @@ -757,6 +757,103 @@ ruleTester.run("key-spacing", rule, {
}
}],
parserOptions: { ecmaVersion: 6 }
},

// https://github.com/eslint/eslint/issues/11414
{
code: [
"var obj = {",
" foo: 1, 'bar': 2, baz: 3",
"}"
].join("\n"),
options: [{
singleLine: {
beforeColon: false,
afterColon: false
},
multiLine: {
beforeColon: false,
afterColon: true
}
}]
}, {
code: [
"var obj = {",
" foo: 1, 'bar': 2, baz: 3",
"}"
].join("\n"),
options: [{
singleLine: {
beforeColon: false,
afterColon: false
},
multiLine: {
align: "value"
}
}]
}, {
code: [
"var obj = {",
" foo : 1, 'bar' : 2, baz : 3,",
" short : 4,",
" longlonglong : 5",
"}"
].join("\n"),
options: [{
singleLine: {
afterColon: true,
beforeColon: false
},
multiLine: {
afterColon: true,
beforeColon: true,
align: "colon"
}
}]
}, {
code: [
"var obj = {",
" foo: 1, 'bar': 2, baz: 3,",
" short: 4,",
" longlonglong: 5",
"}"
].join("\n"),
options: [{
align: "value"
}]
}, {
code: [
"var obj = {",
" foo : 1, 'bar' : 2, baz : 3,",
" short : 4,",
" longlonglong : 5",
"}"
].join("\n"),
options: [{
align: "colon",
beforeColon: true
}]
}, {
code: [
"foo({",
" foo: 1, 'bar': 2, baz: 3,",
" short :4,",
" longlonglong:5",
"})"
].join("\n"),
options: [{
multiLine: {
beforeColon: false,
afterColon: true,
mode: "strict"
},
align: {
beforeColon: false,
afterColon: false,
on: "colon",
mode: "minimum"
}
}]
}],

invalid: [{
Expand Down Expand Up @@ -1769,5 +1866,170 @@ 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" }
]
}, // https://github.com/eslint/eslint/issues/11414
{
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: false,
afterColon: true
}
}],
errors: [
{ messageId: "missingValue", data: { computed: "", key: "foo" }, line: 2, column: 8, type: "Literal" },
{ messageId: "missingValue", data: { computed: "", key: "bar" }, line: 2, column: 17, type: "Literal" },
{ messageId: "missingValue", data: { computed: "", key: "baz" }, line: 2, column: 24, type: "Literal" }
]
}, {
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: {
align: "value"
}
}],
errors: [
{ messageId: "extraKey", data: { computed: "", key: "foo" }, line: 2, column: 4, type: "Identifier" },
{ messageId: "extraKey", data: { computed: "", key: "bar" }, line: 2, column: 13, type: "Literal" },
{ messageId: "extraKey", data: { computed: "", key: "baz" }, line: 2, column: 24, type: "Identifier" }
]
}, {
code: [
"var obj = {",
" foo:1, 'bar':2, baz:3,",
" short: 4,",
" longlonglong : 5",
"}"
].join("\n"),
output: [
"var obj = {",
" foo : 1, 'bar' : 2, baz : 3,",
" short : 4,",
" longlonglong : 5",
"}"
].join("\n"),
options: [{
singleLine: {
afterColon: true,
beforeColon: false
},
multiLine: {
afterColon: true,
beforeColon: true,
align: "colon"
}
}],
errors: [
{ messageId: "missingKey", data: { computed: "", key: "foo" }, line: 2, column: 4, type: "Identifier" },
{ messageId: "missingValue", data: { computed: "", key: "foo" }, line: 2, column: 8, type: "Literal" },
{ messageId: "missingKey", data: { computed: "", key: "bar" }, line: 2, column: 11, type: "Literal" },
{ messageId: "missingValue", data: { computed: "", key: "bar" }, line: 2, column: 17, type: "Literal" },
{ messageId: "missingKey", data: { computed: "", key: "baz" }, line: 2, column: 20, type: "Identifier" },
{ messageId: "missingValue", data: { computed: "", key: "baz" }, line: 2, column: 24, type: "Literal" },
{ messageId: "missingKey", data: { computed: "", key: "short" }, line: 3, column: 4, type: "Identifier" }
]
}, {
code: [
"var obj = {",
" foo: 1, 'bar': 2, baz: 3,",
" short:4,",
" longlonglong: 5",
"}"
].join("\n"),
output: [
"var obj = {",
" foo: 1, 'bar': 2, baz: 3,",
" short: 4,",
" longlonglong: 5",
"}"
].join("\n"),
options: [{
align: "value"
}],
errors: [
{ messageId: "missingValue", data: { computed: "", key: "short" }, line: 3, column: 10, type: "Literal" }
]
}, {
code: [
"var obj = {",
" foo : 1, 'bar' : 2, baz : 3,",
" short :4,",
" longlonglong : 5",
"}"
].join("\n"),
output: [
"var obj = {",
" foo : 1, 'bar' : 2, baz : 3,",
" short : 4,",
" longlonglong : 5",
"}"
].join("\n"),
options: [{
align: "colon",
beforeColon: true
}],
errors: [
{ messageId: "missingKey", data: { computed: "", key: "short" }, line: 3, column: 4, type: "Identifier" },
{ messageId: "missingValue", data: { computed: "", key: "short" }, line: 3, column: 11, type: "Literal" }
]
}, {
code: [
"foo({",
" foo : 1, 'bar' : 2, baz : 3,",
" short : 4,",
" longlonglong:5",
"})"
].join("\n"),
output: [
"foo({",
" foo: 1, 'bar': 2, baz: 3,",
" short :4,",
" longlonglong:5",
"})"
].join("\n"),
options: [{
multiLine: {
beforeColon: false,
afterColon: true,
mode: "strict"
},
align: {
beforeColon: false,
afterColon: false,
on: "colon",
mode: "minimum"
}
}],
errors: [
{ messageId: "extraKey", data: { computed: "", key: "foo" }, line: 2, column: 4, type: "Identifier" },
{ messageId: "extraKey", data: { computed: "", key: "bar" }, line: 2, column: 13, type: "Literal" },
{ messageId: "extraKey", data: { computed: "", key: "baz" }, line: 2, column: 24, type: "Identifier" },
{ messageId: "extraValue", data: { computed: "", key: "short" }, line: 3, column: 20, type: "Literal" }
]
}]
});