Implement translation for PM_CONSTANT_PATH_WRITE_NODE
#237
Conversation
| @@ -26,19 +26,23 @@ std::unique_ptr<parser::Assign> Translator::translateAssignment(pm_node_t *untyp | |||
| auto node = reinterpret_cast<PrismAssignmentNode *>(untypedNode); | |||
| auto *loc = &node->base.location; | |||
|
|
|||
| auto name = parser.resolveConstant(node->name); | |||
There was a problem hiding this comment.
I've moved this assignment to lines 34 and 45 so it only happens in conditional branches where node->name will exist. Is there a more elegant way to do this?
There was a problem hiding this comment.
That seems best IMO, because 1 of the 3 branches of the if/else ladder doesn't set it.
| pm_location_t *loc = &constantPathNode->base.location; | ||
|
|
||
| std::string_view name = parser.resolveConstant(constantPathNode->name); | ||
|
|
||
| std::unique_ptr<parser::Node> parent; | ||
| if (constantPathNode->parent) { | ||
| // This constant reference is chained onto another constant reference. | ||
| // E.g. if `node` is pointing to `B`, then then `A` is the `parent` in `A::B::C`. | ||
| parent = translate(constantPathNode->parent); | ||
| } else { // This is a fully qualified constant reference, like `::A`. | ||
| pm_location_t *delimiterLoc = &constantPathNode->delimiter_loc; // The location of the `::` | ||
| parent = make_unique<parser::Cbase>(parser.translateLocation(delimiterLoc)); | ||
| } | ||
|
|
||
| return make_unique<parser::Const>(parser.translateLocation(loc), std::move(parent), | ||
| gs.enterNameConstant(name)); |
There was a problem hiding this comment.
I have pulled this out into a helper method because depending on the context, Sorbet has different representations of PM_CONSTANT_PATH_NODEs. If it's a path node on its own, then Sorbet represents it as a Const, but if it's being assigned a value, it's a ConstLhs.
| if (isAssignment) { | ||
| return make_unique<parser::ConstLhs>(parser.translateLocation(loc), std::move(parent), | ||
| gs.enterNameConstant(name)); | ||
| } else { | ||
| return make_unique<parser::Const>(parser.translateLocation(loc), std::move(parent), gs.enterNameConstant(name)); | ||
| } |
There was a problem hiding this comment.
Here is where we make different Sorbet nodes based on the context of the constant path node.
| @@ -26,19 +26,23 @@ std::unique_ptr<parser::Assign> Translator::translateAssignment(pm_node_t *untyp | |||
| auto node = reinterpret_cast<PrismAssignmentNode *>(untypedNode); | |||
| auto *loc = &node->base.location; | |||
|
|
|||
| auto name = parser.resolveConstant(node->name); | |||
There was a problem hiding this comment.
That seems best IMO, because 1 of the 3 branches of the if/else ladder doesn't set it.
|
|
||
| return make_unique<parser::Const>(parser.translateLocation(loc), std::move(parent), | ||
| gs.enterNameConstant(name)); | ||
| auto isAssignment = false; |
There was a problem hiding this comment.
Big same :( I took this pattern from you tbh -- I like the way you've been assigning them to variables to make it clearer what they're for.
| # Regular assignment | ||
| ConstantPath::RegularAssignment = 1 | ||
| ::FullyQualified::ConstantPath::RegularAssignment = 2 |
There was a problem hiding this comment.
Really minor, but I think it'd be more understandable if these to live in assign_to_const.rb. The fact that they're different node types is non-obvious, and seeing the contrast between the two on the same file could be useful to future readers
There was a problem hiding this comment.
Done! Agreed, I think the tests are clearer now.
egiurleo
force-pushed
the
emily/const-path-write
branch
from
1f6ec1c
to
5d371a4
Compare
Closes #77
Test plan
See included automated tests.