-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Fix/22473 #27970
Fix/22473 #27970
Conversation
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.
Also needs a codeFixAll
test
src/compiler/diagnosticMessages.json
Outdated
@@ -4683,5 +4683,13 @@ | |||
"Generate types for all packages without types": { | |||
"category": "Message", | |||
"code": 95068 | |||
}, | |||
"Make all const or readonly expressions reassignable": { |
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.
This sounds like it will change every const
to let
regardless of whether there's an error. Maybe this should be something like Make variables and properties mutable where necessary
.
function getDeclaration(sourceFile: SourceFile, pos: number, checker: TypeChecker): Declaration | undefined { | ||
const node = getTokenAtPosition(sourceFile, pos); | ||
const symbol = checker.getSymbolAtLocation(node); | ||
if (symbol) { |
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.
Should this just be Debug.assertDefined(symbol).valueDeclaration
? If we issued this error it seems like there must be a symbol?
@@ -0,0 +1,11 @@ | |||
/// <reference path='fourslash.ts' /> |
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.
This file should be a .ts
file, not .tsx
changes.replaceNode(sourceFile, oldDeclarationList, declarationList); | ||
} | ||
else if (node.kind === SyntaxKind.PropertyDeclaration) { | ||
const readonlyToken = findAnyChildOfKind(node, SyntaxKind.ReadonlyKeyword); |
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.
changes.deleteModifier(sourceFile, Debug.assertDefined(findModifier(node, SyntaxKind.ReadonlyKeyword)));
We should avoid changing declarations that appear in |
@DanielRosenwasser Updated the PR according to the comments made by @andy-ms |
@muradkhan101 Looks like the latest commit accidentally made a big change to |
d6db77f
to
537c3a5
Compare
@andy-ms Made the time and resolved the merge conflicts with the branch, sorry for the wait |
537c3a5
to
58dd0f2
Compare
@andy-ms This PR's been sitting for a few months now with no activity by the reviewer. Anything I need to do to move it along and get it merged? |
I'm on a different team now -- @DanielRosenwasser @sandersn @sheetalkamat Please review |
@muradkhan101 Any particular reason why you closed this PR? |
Adds a codefix for the scenarios where a user attempts to re-assign a variable that was declared as const or property that was declared readonly. In the former, it changes the initial declaration to type let and in the latter, it removes the readonly modifier.
Added two more diagnostic messages as well since, the existing ones couldn't describe the changes in a localizable way
Fixes #22473