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

fix: don't remove comments between key and value in object-shorthand #18442

Merged
merged 4 commits into from
May 16, 2024
Merged

fix: don't remove comments between key and value in object-shorthand #18442

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5ee7f4d
fix: don't collapse comments in object-shorthand
KubaJastrz May 11, 2024
6005b24
add missing test
KubaJastrz May 12, 2024
d5c6fe1
refactor
KubaJastrz May 12, 2024
58fe097
fix: recognize comments inside value node
KubaJastrz May 16, 2024
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
12 changes: 12 additions & 0 deletions lib/rules/object-shorthand.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,12 @@ module.exports = {
node,
messageId: "expectedPropertyShorthand",
fix(fixer) {

// x: /* */ x
if (sourceCode.commentsExistBetween(node.key, node.value)) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (sourceCode.commentsExistBetween(node.key, node.value)) {
if (sourceCode.getCommentsInside(node).length > 0) {

Since we're replacing the entire node with just an identifier name, seems best to check if there are comments anywhere inside the node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

58fe097

return null;
}

return fixer.replaceText(node, node.value.name);
}
});
Expand All @@ -510,6 +516,12 @@ module.exports = {
node,
messageId: "expectedPropertyShorthand",
fix(fixer) {

// "x": /* */ x
if (sourceCode.commentsExistBetween(node.key, node.value)) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (sourceCode.commentsExistBetween(node.key, node.value)) {
if (sourceCode.getCommentsInside(node).length > 0) {

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

58fe097

return null;
}

return fixer.replaceText(node, node.value.name);
}
});
Expand Down
20 changes: 20 additions & 0 deletions tests/lib/rules/object-shorthand.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,26 @@ ruleTester.run("object-shorthand", rule, {
output: null,
errors: [METHOD_ERROR]
},
{
code: "var x = {a: /* comment */ a}",
output: null,
errors: [PROPERTY_ERROR]
},
{
code: "var x = {a /* comment */: a}",
output: null,
errors: [PROPERTY_ERROR]
},
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
KubaJastrz marked this conversation as resolved.
Show resolved Hide resolved
{
code: "var x = {'a': /* comment */ a}",
output: null,
errors: [PROPERTY_ERROR]
},
{
code: "var x = {'a' /* comment */: a}",
output: null,
errors: [PROPERTY_ERROR]
},
KubaJastrz marked this conversation as resolved.
Show resolved Hide resolved
{
code: "var x = {y: function() {}}",
output: "var x = {y() {}}",
Expand Down