-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Make convertFunctionToEs6Class a codefix #22241
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
522be8d
to
f53adf8
Compare
src/compiler/diagnosticMessages.json
Outdated
@@ -3814,6 +3814,10 @@ | |||
"category": "Suggestion", | |||
"code": 80001 | |||
}, | |||
"Function may be converted to a class.": { |
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.
@DanielRosenwasser any thoughts about the error message
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 think this message can be enhanced, @DanielRosenwasser might have better suggestions.
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.
We should refine this over time, but
This constructor function may be converted to a class declaration.
or
Constructor function may be better represented as a class declaration.
switch (node.kind) { | ||
case SyntaxKind.FunctionDeclaration: | ||
case SyntaxKind.FunctionExpression: | ||
if (isInJavaScriptFile(node)) { |
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.
since this is the only one we have right now, i would move this to the top of the check
function. we can refactor it later as we add more checks.
['1', '2', '3', '4', '5', '6', '7', '8'].forEach(m => verify.applicableRefactorAvailableAtMarker(m)); | ||
verify.fileAfterApplyingRefactorAtMarker('7', | ||
verify.codeFix({ | ||
description: "Convert function to an ES2015 class", |
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.
Can this refer to the resource, rather than including a string literal? That would make it easier to change the resource's value.
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.
Unfortunately, diagnostics are automatically named the same as their message. (See scripts/processDIagnosticMessages.ts
.) So if we change the message we end up needing to rename a bunch of things anyway...
Part of #19392
Discovered #22240