Skip to content

Commit b2cfeaf

Browse files
jesse-shopifyamomchilov
authored andcommitted
Align with whitequark on locations of PM_CASE_NODE and PM_IT_PARAMETERS_NODE
1 parent 28cdc1a commit b2cfeaf

File tree

1 file changed

+29
-18
lines changed

1 file changed

+29
-18
lines changed

parser/prism/Translator.cc

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,21 +2252,21 @@ ast::ExpressionPtr Translator::desugar(pm_node_t *node) {
22522252
}
22532253

22542254
auto elseClause = desugarNullable(up_cast(caseNode->else_clause));
2255-
// Capture the start of the line containing the `else` keyword.
2255+
// Capture the start of the line containing the `else` keyword (or `end` if no else clause).
22562256
// Whitequark extends empty when bodies to the start of the next line (column 1), not to the
2257-
// `else` keyword itself. We find the start of the line by scanning backwards for a newline.
2258-
core::LocOffsets elseClauseLoc = core::LocOffsets::none();
2257+
// `else`/`end` keyword itself. We find the start of the line by scanning backwards for a newline.
2258+
core::LocOffsets clauseLoc;
22592259
if (caseNode->else_clause != nullptr) {
2260-
auto elseLoc = translateLoc(caseNode->else_clause->base.location);
2261-
auto source = ctx.file.data(ctx).source();
2262-
uint32_t lineStart = elseLoc.beginPos();
2263-
// Scan backwards to find the start of the line
2264-
while (lineStart > 0 && source[lineStart - 1] != '\n') {
2265-
lineStart--;
2266-
}
2267-
elseClauseLoc = core::LocOffsets{lineStart, elseLoc.endPos()};
2260+
clauseLoc = translateLoc(caseNode->else_clause->base.location);
2261+
} else {
2262+
// No else clause - use the `end` keyword location as the fallback
2263+
clauseLoc = translateLoc(caseNode->end_keyword_loc);
22682264
}
22692265

2266+
auto detail = core::Loc::pos2Detail(ctx.file.data(ctx), clauseLoc.beginPos());
2267+
auto lineStart = core::Loc::detail2Pos(ctx.file.data(ctx), {detail.line, 1}).value();
2268+
core::LocOffsets nextClauseStartLoc = core::LocOffsets{lineStart, clauseLoc.endPos()};
2269+
22702270
if (preserveConcreteSyntax) {
22712271
auto locZeroLen = location.copyWithZeroLength();
22722272

@@ -2303,7 +2303,9 @@ ast::ExpressionPtr Translator::desugar(pm_node_t *node) {
23032303

23042304
core::NameRef tempName;
23052305
core::LocOffsets predicateLoc;
2306-
bool hasPredicate = (predicate != nullptr);
2306+
// Check if the prism node has a predicate, not the desugared result,
2307+
// because desugarNullable returns EmptyTree (not nullptr) for null nodes.
2308+
bool hasPredicate = (caseNode->predicate != nullptr);
23072309

23082310
if (hasPredicate) {
23092311
predicateLoc = predicate.loc();
@@ -2315,9 +2317,9 @@ ast::ExpressionPtr Translator::desugar(pm_node_t *node) {
23152317
// The if/else ladder for the entire case statement, starting with the else clause as the final `else` when
23162318
// building backwards
23172319
ExpressionPtr resultExpr = move(elseClause);
2318-
// Track the location of the next clause (else or when) for extending empty when bodies.
2319-
// Initially this is the else clause location; after each iteration it becomes the If location.
2320-
core::LocOffsets nextClauseLoc = elseClauseLoc;
2320+
// Track the location of the next clause (else, end, or when) for extending empty when bodies.
2321+
// Initially this is the else/end clause location; after each iteration it becomes the If location.
2322+
core::LocOffsets nextClauseLoc = nextClauseStartLoc;
23212323

23222324
// Iterate over Prism when nodes in reverse to build the if/else ladder backwards
23232325
for (auto it = prismWhenNodes.rbegin(); it != prismWhenNodes.rend(); ++it) {
@@ -2363,16 +2365,22 @@ ast::ExpressionPtr Translator::desugar(pm_node_t *node) {
23632365
}
23642366

23652367
auto then = desugarStatements(prismWhen->statements);
2366-
// Whitequark extends the when clause location to include the start of the next clause,
2368+
// Whitequark extends the when clause location to include the start of the next line,
23672369
// but ONLY when the when body is empty. Use the start of nextClauseLoc as the end.
23682370
auto ifLoc = whenLoc;
23692371
bool bodyIsEmpty = (then == nullptr || ast::isa_tree<ast::EmptyTree>(then));
23702372
if (bodyIsEmpty && nextClauseLoc.exists()) {
23712373
ifLoc = core::LocOffsets{whenLoc.beginPos(), nextClauseLoc.beginPos()};
23722374
}
23732375
resultExpr = MK::If(ifLoc, move(patternsResult), move(then), move(resultExpr));
2374-
// Update nextClauseLoc to be this If's location for the next iteration
2375-
nextClauseLoc = ifLoc;
2376+
// Update nextClauseLoc to be the start of this when clause's LINE for the next iteration.
2377+
// This ensures empty when bodies extend to column 1, not to the indented clause start.
2378+
auto source = ctx.file.data(ctx).source();
2379+
uint32_t lineStart = whenLoc.beginPos();
2380+
while (lineStart > 0 && source[lineStart - 1] != '\n') {
2381+
lineStart--;
2382+
}
2383+
nextClauseLoc = core::LocOffsets{lineStart, whenLoc.endPos()};
23762384
}
23772385

23782386
if (hasPredicate) {
@@ -4115,6 +4123,9 @@ core::LocOffsets Translator::findItParamUsageLoc(pm_statements_node *statements)
41154123
core::LocOffsets result;
41164124

41174125
walkPrismAST(up_cast(statements), [this, &result](const pm_node_t *node) -> bool {
4126+
if (result.exists()) {
4127+
return false;
4128+
}
41184129
if (PM_NODE_TYPE_P(node, PM_IT_LOCAL_VARIABLE_READ_NODE)) {
41194130
result = this->translateLoc(node->location);
41204131
// Found the first usage, stop walking

0 commit comments

Comments
 (0)