-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
tools: auto fix custom eslint rule for lowercase-name-for-primitive.js #17715
Changes from 2 commits
a389548
b2d1f5b
42009c2
1799ec3
a23ca88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,11 +22,11 @@ module.exports = function(context) { | |
|
||
switch (names.type) { | ||
case 'Literal': | ||
checkName(node, names.value); | ||
checkName(names, names.value); | ||
break; | ||
case 'ArrayExpression': | ||
names.elements.forEach((name) => { | ||
checkName(node, name.value); | ||
names.elements.forEach((name, index) => { | ||
checkName(names.elements[index], name.value); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't this just be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏻 |
||
}); | ||
break; | ||
} | ||
|
@@ -36,8 +36,18 @@ module.exports = function(context) { | |
const lowercaseName = name.toLowerCase(); | ||
if (primitives.includes(lowercaseName) && !primitives.includes(name)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Also, pretty sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made the changes. |
||
const msg = `primitive should use lowercase: ${name}`; | ||
context.report(node, msg); | ||
context.report({ | ||
node, | ||
message: msg, | ||
fix: (fixer) => { | ||
return fixer.replaceText( | ||
node, | ||
`'${lowercaseName}'` | ||
); | ||
} | ||
}); | ||
} | ||
|
||
} | ||
|
||
return { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI failed due to a small problem:
The input code and expected output are using double quotes
"
, but the autofixer will change it to single quotes, which makes the actual output cannot match the expected output.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shobhitchittora Could you change these double quotes in
code
andoutput
to using single quotes?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@starkwang pushed a fix. How did you check the problem in Jenkins that the output is not correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I modified some codes in eslint (eslint/lib/testers/rule-tester.js) to expose the error message.
You can see eslint/eslint#9769 for more detail. 😉