-
Notifications
You must be signed in to change notification settings - Fork 885
[bugfix] callable types suggestion forgot parens on unions and intersections #3342
[bugfix] callable types suggestion forgot parens on unions and intersections #3342
Conversation
src/rules/callableTypesRule.ts
Outdated
|
||
let suggestion = `${text.substr(0, colonPos)} =>${text.substr(colonPos + 1)}`; | ||
if (parent.parent !== undefined && shouldWrapSuggestion(parent.parent)) { |
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.
.parent
can never be undefined here. you can simply assert that it is not undefined with shouldWrapSuggestion(parent.parent!)
@@ -32,6 +32,16 @@ var fn: {(): void;}; | |||
~~~~~~~~~ [type % ('() => void')] | |||
function f(x: { (): void }): void; | |||
~~~~~~~~ [type % ('() => void')] | |||
function f(x: string | { (): void }): void; |
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 add a test where the type is already wrapped in parentheses, e.g. : string | ({ (): void })
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.
The funny thing is that it still works and doesn't add parens in this case. I guess that if there is already parens, parent.parent doesn't reference the Union, but reference the node with the extra parens.
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.
That's right, parent.parent
is ts.ParenthesizedType
in that case. That's why I wanted this test case. To verify that we don't add parens if they are already present.
src/rules/callableTypesRule.ts
Outdated
@@ -46,7 +46,7 @@ export class Rule extends Lint.Rules.AbstractRule { | |||
function walk(ctx: Lint.WalkContext<void>) { | |||
return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void { | |||
if ((isInterfaceDeclaration(node) && noSupertype(node) | |||
|| isTypeLiteralNode(node)) | |||
|| isTypeLiteralNode(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.
the alignment here was actually intentional because of the parens. it makes more sense to just remove the line break
Thanks @cyrilgandon |
PR checklist
Overview of change:
Is there anything you'd like reviewers to focus on?
CHANGELOG.md entry:
[bugfix]
callable-types
auto fix produces invalid results