Fix newline inserted in empty block at end of formatting range #48463
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
| previousRange, | ||
| previousRangeStartLine!, | ||
| previousParent!, | ||
| enclosingNode, | ||
| parent, |
There was a problem hiding this comment.
The TLDR is this final parameter is supposed to be the “context node” of the token, as if anyone knows what that means, and enclosingNode may be arbitrarily higher up the tree. Turns out that “context node” means “direct parent” in almost all cases. What was happening in this case was that contextNode should have been the inner if Block ({}), but it was actually the whole outer IfStatement (if (foo) { ... }). The rule NewLineBeforeCloseBraceInBlockContext is supposed to take effect only if the block ending in the close brace is not on a single line. The inner one is, so the rule should not have applied, but it was using the outer one to decide whether to apply the rule.
andrewbranch
force-pushed
the
bug/format-on-brace
branch
from
b947cf4 to
79b600a
Compare
| export function findPrecedingToken(position: number, sourceFile: SourceFileLike, startNode: Node, excludeJsdoc?: boolean): Node | undefined; | ||
| export function findPrecedingToken(position: number, sourceFile: SourceFile, startNode?: Node, excludeJsdoc?: boolean): Node | undefined; |
There was a problem hiding this comment.
Out of curiosity, why couldn't this just change to SourceFileLike? Seems like it'd be a backwards compatible change to accept a wider set of types.
There was a problem hiding this comment.
Oh, I didn't notice that startNode becomes required.
There was a problem hiding this comment.
SourceFileLike has no parse tree structure; it’s just a text container. So if you provide a SourceFileLike you must provide an actual Node to start in.
There was a problem hiding this comment.
Yeah, I just didn't notice the one character ? difference, oops.
|
@typescript-bot cherry-pick this to release-4.6 |
|
Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into |
|
Hey @DanielRosenwasser, I've opened #48477 for you. |
Fixes bug tracked in Azure DevOps found in VS testing. Thanks @zkat for helping figure out a test repro.