Skip to content

Commit

Permalink
Handle all constant path assignment node types
Browse files Browse the repository at this point in the history
  • Loading branch information
@egiurleo
egiurleo committed Nov 5, 2024
1 parent 8f8bc8f commit fcc8a72
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 13 deletions.
19 changes: 15 additions & 4 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 @@ -1631,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 @@ -1673,7 +1684,8 @@ unique_ptr<parser::Node> Translator::translateConst(PrismLhsNode *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) { // Check if this is a dynamic constant assignment (SyntaxError at runtime)
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.

Expand Down Expand Up @@ -1774,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
172 changes: 165 additions & 7 deletions test/prism_regression/assign_to_constant.parse-tree.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,13 +520,171 @@ Begin {
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
21 changes: 20 additions & 1 deletion test/prism_regression/assign_to_constant.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,24 @@ def method1
end

def method2
ConstantPath::DynamicConstant2 = "This should raise SyntaxError at runtime"
ConstantPath::DynamicConstant2 = "This should raise a SyntaxError at runtime"

# These should *NOT* raise a SyntaxError at runtime
ConstantPath::DynamicConstantBitwiseAnd &= 2
ConstantPath::DynamicConstantBitwiseXor ^= 4
ConstantPath::DynamicConstantShiftRight >>= 5
ConstantPath::DynamicConstantShiftLeft <<= 6
ConstantPath::DynamicConstantSubtractAssign -= 7
ConstantPath::DynamicConstantModuleAssign %= 8
ConstantPath::DynamicConstantBitwiseOr |= 9
ConstantPath::DynamicConstantDivideAssign /= 10
ConstantPath::DynamicConstantMultiplyAssign *= 11
ConstantPath::DynamicConstantExponentiateAssign **= 12

# And / Or assignment; still should not raise a syntax error
ConstantPath::DynamicConstantLazyAndAssign &&= 13
ConstantPath::DynamicConstantLazyOrAssgin ||= 14

# Sorbet doesn't do the dynamic constant workaround for multi-target assignments
# ConstantPath::DynamicConstantTarget1, ConstantPath::DynamicConstantTarget2 = 35, 36
end

0 comments on commit fcc8a72

Please sign in to comment.