Skip to content

Commit

Permalink
Merge pull request #317 from Shopify/emily/rescue
Browse files Browse the repository at this point in the history 
Fix bugs in translation of `PM_RESCUE_NODE`
  • Loading branch information
@egiurleo
egiurleo authored Nov 5, 2024
2 parents 289ef2e + fcc8a72 commit 5b7a69f
Show file tree
Hide file tree
Showing 6 changed files with 474 additions and 27 deletions.
37 changes: 27 additions & 10 deletions parser/prism/Translator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ unique_ptr<SorbetAssignmentNode> Translator::translateOpAssignment(pm_node_t *un
is_same_v<PrismAssignmentNode, pm_constant_path_and_write_node> ||
is_same_v<PrismAssignmentNode, pm_constant_path_or_write_node>) {
// Handle operator assignment to a constant path, like `A::B::C += 1` or `::C += 1`
lhs = translateConst<pm_constant_path_node, parser::ConstLhs>(node->target);
bool skipDynamicConstantWorkaround = true;
lhs = translateConst<pm_constant_path_node, parser::ConstLhs>(node->target, skipDynamicConstantWorkaround);
} else if constexpr (is_same_v<SorbetLHSNode, parser::Send>) {
// Handle operator assignment to the result of a method call, like `a.b += 1`
auto name = parser.resolveConstant(node->read_name);
Expand Down Expand Up @@ -1585,6 +1586,7 @@ unique_ptr<parser::Node> Translator::translateCallWithBlock(pm_node_t *prismBloc
// (and its linked `subsequent` nodes) and assembling the corresponding `Rescue` and `Resbody` nodes in Sorbet's AST.
unique_ptr<parser::Node> Translator::translateRescue(pm_rescue_node *prismRescueNode, unique_ptr<parser::Node> bodyNode,
unique_ptr<parser::Node> elseNode) {
auto rescueLoc = translateLoc(prismRescueNode->base.location);
NodeVec rescueBodies;

// Each `rescue` clause generates a `Resbody` node, which is a child of the `Rescue` node.
Expand All @@ -1608,7 +1610,7 @@ unique_ptr<parser::Node> Translator::translateRescue(pm_rescue_node *prismRescue
}

// The `Rescue` node combines the main body, the rescue clauses, and the else clause.
return make_unique<parser::Rescue>(bodyNode->loc, move(bodyNode), move(rescueBodies), move(elseNode));
return make_unique<parser::Rescue>(rescueLoc, move(bodyNode), move(rescueBodies), move(elseNode));
}

// Translates the given Prism Statements Node into a `parser::Begin` node or an inlined `parser::Node`.
Expand All @@ -1630,10 +1632,20 @@ unique_ptr<parser::Node> Translator::translateStatements(pm_statements_node *stm

// Handles any one of the Prism nodes that models any kind of assignment to a constant or constant path.
//
// Dynamic constant assignment inside of a method definition will raise a SyntaxError at runtime. In the
// Sorbet validator, there is a check that will crash Sorbet if this is detected statically.
// To work around this, we substitute dynamic constant assignments with a write to a fake local variable
// called `dynamicConstAssign`.
//
// The only exception is that dynamic constant path *operator* assignments inside of a method definition
// do not raise a SyntaxError at runtime, so we want to skip the workaround in that case.
// However, within this method, both regular constant path assignments and constant path operator assignments
// are passed in as `pm_constant_path_node` types, so we need an extra boolean flag to know when to skip the workaround.
//
// Usually returns the `SorbetLHSNode`, but for constant writes and targets,
// it can can return an `LVarLhs` as a workaround in the case of a dynamic constant assignment.
template <typename PrismLhsNode, typename SorbetLHSNode>
unique_ptr<parser::Node> Translator::translateConst(PrismLhsNode *node) {
unique_ptr<parser::Node> Translator::translateConst(PrismLhsNode *node, bool skipDynamicConstantWorkaround) {
static_assert(is_same_v<SorbetLHSNode, parser::Const> || is_same_v<SorbetLHSNode, parser::ConstLhs>,
"Invalid LHS type. Must be one of `parser::Const` or `parser::ConstLhs`.");

Expand Down Expand Up @@ -1668,14 +1680,20 @@ unique_ptr<parser::Node> Translator::translateConst(PrismLhsNode *node) {
auto location = translateLoc(node->base.location);
auto name = parser.resolveConstant(node->name);

if (isInMethodDef) { // Check if this is a dynamic constant assignment (SyntaxError at runtime)
// This is a copy of a workaround from `Desugar.cc`, which substitues in a fake assignment,
// so the parsing can continue. See other usages of `dynamicConstAssign` for more details.
if constexpr (is_same_v<PrismLhsNode, pm_constant_write_node> || is_same_v<PrismLhsNode, pm_constant_path_node> ||
is_same_v<PrismLhsNode, pm_constant_operator_write_node> ||
is_same_v<PrismLhsNode, pm_constant_or_write_node> ||
is_same_v<PrismLhsNode, pm_constant_and_write_node>) {
if (isInMethodDef &&
!skipDynamicConstantWorkaround) { // Check if this is a dynamic constant assignment (SyntaxError at runtime)
// This is a copy of a workaround from `Desugar.cc`, which substitues in a fake assignment,
// so the parsing can continue. See other usages of `dynamicConstAssign` for more details.

// Enter the name of the constant so that it's available for the rest of the pipeline
gs.enterNameConstant(name);
// Enter the name of the constant so that it's available for the rest of the pipeline
gs.enterNameConstant(name);

return make_unique<LVarLhs>(location, core::Names::dynamicConstAssign());
return make_unique<LVarLhs>(location, core::Names::dynamicConstAssign());
}
}

return make_unique<SorbetLHSNode>(location, move(parent), gs.enterNameConstant(name));
Expand Down Expand Up @@ -1768,5 +1786,4 @@ void Translator::reportError(core::LocOffsets loc, const std::string &message) {
e.setHeader("{}", message);
}
}

}; // namespace sorbet::parser::Prism
2 changes: 1 addition & 1 deletion parser/prism/Translator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class Translator final {
std::unique_ptr<SorbetAssignmentNode> translateOpAssignment(pm_node_t *node);

template <typename PrismLhsNode, typename SorbetLHSNode>
std::unique_ptr<parser::Node> translateConst(PrismLhsNode *node);
std::unique_ptr<parser::Node> translateConst(PrismLhsNode *node, bool skipDynamicConstantWorkaround = false);

// Pattern-matching
// ... variations of the main translation functions for pattern-matching related nodes.
Expand Down
296 changes: 282 additions & 14 deletions test/prism_regression/assign_to_constant.parse-tree.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,25 +398,293 @@ Begin {
DefMethod {
name = <U method1>
args = NULL
body = Assign {
lhs = LVarLhs {
name = <U <dynamic-const-assign>>
}
rhs = String {
val = <U This should raise SyntaxError at runtime>
}
body = Begin {
stmts = [
Assign {
lhs = LVarLhs {
name = <U <dynamic-const-assign>>
}
rhs = String {
val = <U These should all raise SyntaxErrors at runtime>
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U &>
right = Integer {
val = "2"
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U ^>
right = Integer {
val = "4"
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U >>>
right = Integer {
val = "5"
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U <<>
right = Integer {
val = "6"
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U ->
right = Integer {
val = "7"
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U %>
right = Integer {
val = "8"
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U |>
right = Integer {
val = "9"
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U />
right = Integer {
val = "10"
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U *>
right = Integer {
val = "11"
}
}
OpAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
op = <U **>
right = Integer {
val = "12"
}
}
AndAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
right = Integer {
val = "13"
}
}
OrAsgn {
left = LVarLhs {
name = <U <dynamic-const-assign>>
}
right = Integer {
val = "14"
}
}
]
}
}
DefMethod {
name = <U method2>
args = NULL
body = Assign {
lhs = LVarLhs {
name = <U <dynamic-const-assign>>
}
rhs = String {
val = <U This should raise SyntaxError at runtime>
}
body = Begin {
stmts = [
Assign {
lhs = LVarLhs {
name = <U <dynamic-const-assign>>
}
rhs = String {
val = <U This should raise a SyntaxError at runtime>
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantBitwiseAnd>>
}
op = <U &>
right = Integer {
val = "2"
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantBitwiseXor>>
}
op = <U ^>
right = Integer {
val = "4"
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantShiftRight>>
}
op = <U >>>
right = Integer {
val = "5"
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantShiftLeft>>
}
op = <U <<>
right = Integer {
val = "6"
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantSubtractAssign>>
}
op = <U ->
right = Integer {
val = "7"
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantModuleAssign>>
}
op = <U %>
right = Integer {
val = "8"
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantBitwiseOr>>
}
op = <U |>
right = Integer {
val = "9"
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantDivideAssign>>
}
op = <U />
right = Integer {
val = "10"
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantMultiplyAssign>>
}
op = <U *>
right = Integer {
val = "11"
}
}
OpAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantExponentiateAssign>>
}
op = <U **>
right = Integer {
val = "12"
}
}
AndAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantLazyAndAssign>>
}
right = Integer {
val = "13"
}
}
OrAsgn {
left = ConstLhs {
scope = Const {
scope = NULL
name = <C <U ConstantPath>>
}
name = <C <U DynamicConstantLazyOrAssgin>>
}
right = Integer {
val = "14"
}
}
]
}
}
]
Expand Down
Loading

0 comments on commit 5b7a69f

Please sign in to comment.