fix: don't remove comments between key and value in object-shorthand #18442
Conversation
✅ Deploy Preview for docs-eslint canceled.
|
lib/rules/object-shorthand.js
Outdated
| const lastKeyToken = sourceCode.getLastToken(node.key); | ||
|
|
||
| // x: /* */ x | ||
| if (sourceCode.commentsExistBetween(lastKeyToken, node.value)) { | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
how about if we give a fix here through appending the comment before or after the property something like:
var x = {
a /* comment */: a,
b
}can be fixed to
var x = {
/* comment */ a,
b
}or
var x = {
a, /* comment */
b
}There was a problem hiding this comment.
I think in that case it becomes a feature request, not a bug fix. You can create an issue for this idea.
Whatever the decision is, I'd recommend that both identifiers and function expressions were handled identically (which they aren't right now, hence this PR).
There was a problem hiding this comment.
I think it's fine not to provide the fix when there's a comment between the two identifiers and let the author fix this manually as it isn't clear where the new position for the comment should be.
Tanujkanti4441
added
the
evaluating
mdjermanovic
added
accepted
mdjermanovic
changed the title
lib/rules/object-shorthand.js
Outdated
| const lastKeyToken = sourceCode.getLastToken(node.key); | ||
|
|
||
| // x: /* */ x | ||
| if (sourceCode.commentsExistBetween(lastKeyToken, node.value)) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
| const lastKeyToken = sourceCode.getLastToken(node.key); | |
| // x: /* */ x | |
| if (sourceCode.commentsExistBetween(lastKeyToken, node.value)) { | |
| return null; | |
| } | |
| // x: /* */ x | |
| if (sourceCode.commentsExistBetween(node.key, node.value)) { | |
| return null; | |
| } |
We can use node.key directly.
lib/rules/object-shorthand.js
Outdated
| const lastKeyToken = sourceCode.getLastToken(node.key); | ||
|
|
||
| // "x": /* */ x | ||
| if (sourceCode.commentsExistBetween(lastKeyToken, node.value)) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
| const lastKeyToken = sourceCode.getLastToken(node.key); | |
| // "x": /* */ x | |
| if (sourceCode.commentsExistBetween(lastKeyToken, node.value)) { | |
| return null; | |
| } | |
| // "x": /* */ x | |
| if (sourceCode.commentsExistBetween(node.key, node.value)) { | |
| return null; | |
| } |
Same here.
lib/rules/object-shorthand.js
Outdated
| @@ -497,6 +497,12 @@ module.exports = { | |||
| node, | |||
| messageId: "expectedPropertyShorthand", | |||
| fix(fixer) { | |||
|
|
|||
| // x: /* */ x | |||
| if (sourceCode.commentsExistBetween(node.key, node.value)) { | |||
There was a problem hiding this comment.
| 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.
lib/rules/object-shorthand.js
Outdated
| @@ -510,6 +516,12 @@ module.exports = { | |||
| node, | |||
| messageId: "expectedPropertyShorthand", | |||
| fix(fixer) { | |||
|
|
|||
| // "x": /* */ x | |||
| if (sourceCode.commentsExistBetween(node.key, node.value)) { | |||
There was a problem hiding this comment.
| if (sourceCode.commentsExistBetween(node.key, node.value)) { | |
| if (sourceCode.getCommentsInside(node).length > 0) { |
Same here.
Fixes #18441
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
Check for comments between the last key token and the value in the property node.
Is there anything you'd like reviewers to focus on?