-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Ensure generated property names for methods named "new" are quoted #55750
Conversation
} | ||
|
||
// See getNameForSymbolFromNameType for a stringy equivalent | ||
function getPropertyNameNodeForSymbolFromNameType(symbol: Symbol, context: NodeBuilderContext, singleQuote?: boolean, stringNamed?: boolean) { | ||
function getPropertyNameNodeForSymbolFromNameType(symbol: Symbol, context: NodeBuilderContext, singleQuote: boolean, stringNamed: boolean, isMethod: boolean) { |
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.
Idle observation: Lint rule for functions with optional parameters which are never not supplied? 🤔
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.
Probably better as some external pass, as ts-eslint doesn't really do stuff cross-project. Technically possible with the new ProjectService thing, maybe.
…icrosoft#55750) Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Fixes #55075
Closes #55109
This effectively replaces #55109, instead using a fix in
createPropertyNameNodeForIdentifierOrLiteral
instead of the emitter. If you look at the last commit, you'll see that the only change to TS emit was removed, which I believe to be correct as the code was able to be written that way in the first place so should be legal.While I was here, I made the parameters to
createPropertyNameNodeForIdentifierOrLiteral
required, which showed that there was an existing bug for this in a code fix. Likely I need a test for this. It also shows that this call doesn't respectstringNamed
either, which unfortunately requires some type info I didn't look into getting quite yet.cc @Andarist