[DO NOT MERGE] Implement AssertionsRewriterPrism #828
Conversation
| // Multi-assignment | ||
| case PM_MULTI_WRITE_NODE: { | ||
| auto *masgn = down_cast<pm_multi_write_node_t>(node); | ||
| // TODO: Handle target nodes (lefts/rest/rights) |
| // Do we have a block? | ||
| if (!block || !PM_NODE_TYPE_P(block, PM_BLOCK_NODE)) { |
There was a problem hiding this comment.
| // Do we have a block? | |
| if (!block || !PM_NODE_TYPE_P(block, PM_BLOCK_NODE)) { | |
| if (!block || !PM_NODE_TYPE_P(block, PM_BLOCK_NODE)) { |
| if (!((aType == PM_CONSTANT_READ_NODE || aType == PM_CONSTANT_PATH_NODE) && | ||
| (bType == PM_CONSTANT_READ_NODE || bType == PM_CONSTANT_PATH_NODE))) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This looks to be redundant to the two checks below. They'll both be false, and you just fall off the end and return false
| // - G1[Integer] -> <syntheticSquareBrackets> call | ||
| // - G2[T.any(Integer, String)] -> T.any() call | ||
| // - Pair[T.type_parameter(:X)] -> T.type_parameter() call |
There was a problem hiding this comment.
Great docs, super clear to follow with these
| * If `node` is a `.new` call and `type` is a generic type instantiation of the same class, | ||
| * replace the receiver of `.new` with the generic type. | ||
| * | ||
| * For example: `G1.new #: G1[Integer]` becomes `G1[Integer].new`. |
There was a problem hiding this comment.
Not quite, there's also the T.let
| * For example: `G1.new #: G1[Integer]` becomes `G1[Integer].new`. | |
| * For example: | |
| * | |
| * G1.new #: G1[Integer] | |
| * | |
| * becomes: | |
| * | |
| * T.let(G1.new, G1[Integer]) | |
| * | |
| * This approach avoids calling `G1[Integer]`, which would have triggered | |
| * other type-checking errors about `G1::[]` not being defined | |
| * (since `G1` could be typed purely in RBS, and not `extend T::Generic`). | |
| * However, it requires that we clone the `G1` path, so we can use it twice. |
| return false; | ||
| } | ||
|
|
||
| typeParams = extractTypeParamsPrism(ctx, parser, callNode->block); |
There was a problem hiding this comment.
In this case, I think it'd be worth using this-> to clarify that we're mutating the context of the AssertionsRewriterPrism itself. (Pun intended)
| typeParams = extractTypeParamsPrism(ctx, parser, callNode->block); | |
| this->typeParams = extractTypeParamsPrism(ctx, parser, callNode->block); |
| // of the type, not the expression being wrapped. | ||
| auto typeLoc = translateLocation(type->location); | ||
|
|
||
| if (kind == InlineCommentPrism::Kind::LET) { |
| return prism.TBindSelf(loc, prism.TUntyped(loc)); | ||
| } | ||
|
|
||
| auto kind = pair->second; |
There was a problem hiding this comment.
| auto kind = pair->second; | |
| auto [type, kind] = pair; |
| auto kind = pair->second; | ||
| ENFORCE(kind == InlineCommentPrism::Kind::BIND, "Invalid inline comment for synthetic bind"); | ||
|
|
||
| auto type = pair->first; |
There was a problem hiding this comment.
| auto type = pair->first; |
| // Calls | ||
| case PM_CALL_NODE: { | ||
| auto *call = down_cast<pm_call_node_t>(node); | ||
| if (saveTypeParams(node)) { |
There was a problem hiding this comment.
I think saveMethodTypeParams would be more clear.
Up until I saw the use here, I assumed this function/context was used to track all type parameters (class/module generics and methods), but it's only methods
| return node; | ||
| } | ||
| if (parser.isSafeNavigationCall(node)) { | ||
| call->receiver = rewriteNode(maybeInsertCast(call->receiver)); |
There was a problem hiding this comment.
Why is maybeInsertCast but not in the false case? What's the distinction here?
2 participants
Extracted from #681
Equivalent file using whitequark AST https://github.com/sorbet/sorbet/blob/master/rbs/AssertionsRewriter.cc