-
Notifications
You must be signed in to change notification settings - Fork 12.8k
prefix-unused-parameter-with-_ codefix now works in jsdoc @param #36152
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
Conversation
…name JSDoc @param. Fix test for quick fix "Prefix all unused declarations with '_' where possible". Fixes microsoft#33021.
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.
Thanks for taking this on! I have one suggestion if you have time to take a look.
@sergeir82 do you intend to keep working on this? Let us know if we can help -- I started tracking community PRs a couple of months ago so we don't forget to review ones like this. |
Yes, I will try to fix it. |
…l to more ligher call of getJSDocParameterTags when searching for a parameter in jsdoc.
Hi! Changes were done. You may check it. |
@@ -154,6 +154,13 @@ namespace ts.codefix { | |||
} | |||
if (isIdentifier(token) && canPrefix(token)) { | |||
changes.replaceNode(sourceFile, token, createIdentifier(`_${token.text}`)); | |||
if (isParameter(token.parent)) { | |||
getJSDocParameterTags(token.parent).forEach((tag) => { |
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.
Don’t you need to do this only for the JSDoc parameter tags whose name is the same as token.text
? Try adding this test:
/**
* @param a
* @param b
*/
function f(a, b) {
const x = a;
}
to ensure that b
gets renamed but a
does not.
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.
Ah, I see getJSDocParameterTags
already handles that. My bad. Thanks for adding the test anyway.
@@ -154,6 +154,13 @@ namespace ts.codefix { | |||
} | |||
if (isIdentifier(token) && canPrefix(token)) { | |||
changes.replaceNode(sourceFile, token, createIdentifier(`_${token.text}`)); | |||
if (isParameter(token.parent)) { | |||
getJSDocParameterTags(token.parent).forEach((tag) => { |
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.
Ah, I see getJSDocParameterTags
already handles that. My bad. Thanks for adding the test anyway.
Fixes #33021