Skip to content

[DO NOT MERGE] ko/rbs-over-prism rebased #851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KaanOzkan wants to merge 12 commits into
base: master
Choose a base branch
from
+1,119 −93
Open

[DO NOT MERGE] ko/rbs-over-prism rebased #851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b9b0a6c
[Squashed] ko/rbs-over-prism: RBS over Prism (master-resolved conflicts)
KaanOzkan Mar 4, 2026
777b888
Enable RBS-over-Prism by removing `&& false` guard
KaanOzkan Mar 4, 2026
ad2bd02
Add ENFORCE check in resolveConstant() for PM_CONSTANT_ID_UNSET
KaanOzkan Mar 4, 2026
0da88a7
Walk LHS of AndAsgn, OpAsgn, OrAsgn in CommentsAssociator
KaanOzkan Mar 4, 2026
f99a2ac
Port type alias fix to CommentsAssociatorPrism
KaanOzkan Mar 5, 2026
e609dd3
Revert squash merge auto-merge regressions to match master
KaanOzkan Mar 6, 2026
0ee19d1
Fix test expectations for RBS tests on master
KaanOzkan Mar 6, 2026
13579a7
Reset Helpers.h to master (remove stale branch changes)
KaanOzkan Mar 6, 2026
b443196
Port Translator.cc fixes: bounds check and constant path optimization
KaanOzkan Mar 9, 2026
7a7fd46
Fix Prism+RBS desugaring path in pipeline and test runner
KaanOzkan Mar 10, 2026
cf775f3
Unexclude RBS tests from PrismPosTests
KaanOzkan Mar 10, 2026
e120ef5
Address PR #851 review comments
KaanOzkan Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions main/options/options.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ const vector<PrintOptions> print_options({
{"parse-tree-json", &Printers::ParseTreeJson, false},
{"parse-tree-json-with-locs", &Printers::ParseTreeJsonWithLocs, false},
{"parse-tree-whitequark", &Printers::ParseTreeWhitequark, false},
{"parse-tree-prism", &Printers::ParseTreePrism, false},
{"rbs-rewrite-tree", &Printers::RBSRewriteTree, false},
{"desugar-tree", &Printers::DesugarTree, false},
{"desugar-tree-raw", &Printers::DesugarTreeRaw, false},
Expand Down Expand Up @@ -197,6 +198,7 @@ vector<reference_wrapper<PrinterConfig>> Printers::printers() {
ParseTreeJson,
ParseTreeJsonWithLocs,
ParseTreeWhitequark,
ParseTreePrism,
RBSRewriteTree,
DesugarTree,
DesugarTreeRaw,
Expand Down
1 change: 1 addition & 0 deletions main/options/options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ struct Printers {
PrinterConfig ParseTreeJson;
PrinterConfig ParseTreeJsonWithLocs;
PrinterConfig ParseTreeWhitequark;
PrinterConfig ParseTreePrism;
PrinterConfig RBSRewriteTree;
PrinterConfig DesugarTree;
PrinterConfig DesugarTreeRaw;
Expand Down
20 changes: 9 additions & 11 deletions main/pipeline/pipeline.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,6 @@ parser::ParseResult runPrismParser(core::GlobalState &gs, core::FileRef file, co
{
core::MutableContext ctx(gs, core::Symbols::root(), file);
core::UnfreezeNameTable nameTableAccess(gs); // enters strings from source code as names
// The RBS rewriter produces plain Whitequark nodes and not `NodeWithExpr` which causes errors in
// `PrismDesugar.cc`. For now, disable all direct translation, and fallback to `Desugar.cc`.
auto source = file.data(ctx).source();
parser::Prism::Parser parser{source};
bool collectComments = gs.cacheSensitiveOptions.rbsEnabled;
Expand All @@ -275,9 +273,7 @@ parser::ParseResult runPrismParser(core::GlobalState &gs, core::FileRef file, co

auto node = prismResult.getRawNodePointer();

// TODO: Remove `&& false` once RBS rewriter with Prism AST migration is complete
// https://github.com/sorbet/sorbet/issues/9065
if (gs.cacheSensitiveOptions.rbsEnabled && false) {
if (gs.cacheSensitiveOptions.rbsEnabled) {
node = rbs::runPrismRBSRewrite(gs, file, node, prismResult.getCommentLocations(), ctx, parser);
if (print.RBSRewriteTree.enabled) {
print.RBSRewriteTree.fmt("{}\n", parser.prettyPrint(node));
Expand Down Expand Up @@ -441,9 +437,7 @@ ast::ParsedFile indexOne(const options::Options &opts, core::GlobalState &lgs, c
try {
parseResult = runPrismParser(lgs, file, print, opts);

// The RBS rewriter produces plain Whitequark nodes and not `NodeWithExpr` which causes errors
// in `PrismDesugar.cc`. For now, disable all direct translation, and fallback to `Desugar.cc`.
usePrismDesugar = !lgs.cacheSensitiveOptions.rbsEnabled;
usePrismDesugar = true;
categoryCounterInc("Prism parse kind", "direct");
} catch (parser::Prism::PrismFallback &) {
parseResult = runParser(lgs, file, print, opts.traceLexer, opts.traceParser);
Expand All @@ -455,9 +449,13 @@ ast::ParsedFile indexOne(const options::Options &opts, core::GlobalState &lgs, c
return emptyParsedFile(file);
}

// TODO: Remove this check once runPrismRBSRewrite is no longer no-oped inside of runPrismParser
// https://github.com/sorbet/sorbet/issues/9065
parseTree = runRBSRewrite(lgs, file, move(parseResult), print);
// When RBS is enabled, runPrismParser already ran the Prism RBS rewriter
// internally. Skip the legacy runRBSRewrite to avoid double-rewriting.
if (lgs.cacheSensitiveOptions.rbsEnabled) {
parseTree = move(parseResult.tree);
} else {
parseTree = runRBSRewrite(lgs, file, move(parseResult), print);
}

break;
}
Expand Down
3 changes: 3 additions & 0 deletions parser/prism/Parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ core::LocOffsets Parser::translateLocation(const uint8_t *start, const uint8_t *
}

string_view Parser::resolveConstant(pm_constant_id_t constantId) const {
ENFORCE(constantId != PM_CONSTANT_ID_UNSET,
"resolveConstant called with PM_CONSTANT_ID_UNSET (anonymous parameter name). "
"Caller must check for PM_CONSTANT_ID_UNSET before calling.");
pm_constant_t *constant = pm_constant_pool_id_to_constant(&parser.constant_pool, constantId);

return cast_prism_string(constant->start, constant->length);
Expand Down
68 changes: 67 additions & 1 deletion parser/prism/Translator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4692,7 +4692,17 @@ ast::ExpressionPtr Translator::desugarStatements(pm_statements_node *stmtsNode,
auto prismStatements = absl::MakeSpan(stmtsNode->body.nodes, stmtsNode->body.size);

// Cover the locations spanned from the first to the last statements.
beginNodeLoc = translateLoc(prismStatements.front()->location.start, prismStatements.back()->location.end);
// When the RBS rewriter inserts synthetic nodes (like `self.abstract!()`) at the end of a statement
// list, those nodes may have locations earlier in the file than the real code (e.g. from comment
// locations). This can cause front()->location.start > back()->location.end, which violates the
// LocOffsets invariant. In that case, fall back to using the statements node's own location.
auto start = prismStatements.front()->location.start;
auto end = prismStatements.back()->location.end;
if (start <= end) {
beginNodeLoc = translateLoc(start, end);
} else {
beginNodeLoc = translateLoc(stmtsNode->base.location);
}
}

auto statements = nodeListToStore<ast::InsSeq::STATS_store>(stmtsNode->body);
Expand Down Expand Up @@ -4766,6 +4776,62 @@ ast::ExpressionPtr Translator::translateConst(pm_node_t *anyNode) {
ast::ExpressionPtr parentExpr;

if constexpr (isConstantPath) { // Handle constant paths, has a parent node that needs translation.
// Optimization: resolve well-known root-anchored constant paths eagerly.
// The RBS rewriter generates references to these constants, so resolving them here
// avoids leaving them as unresolved constant paths.
//
// Matches: ::Sorbet::Private::Static, ::T::Sig::WithoutRuntime, ::Sorbet::Private::Static::Void
//
// Uses resolveConstant (string_view lookup into Prism's constant pool) instead of
// enterNameUTF8 to avoid name table overhead for non-matching paths.
{
pm_node_t *current = up_cast(const_cast<PrismLhsNode *>(node));
bool isRootAnchored = false;
// Segments collected innermost-first. Fixed-size array caps at 4 (longest target path).
std::string_view segments[4];
int depth = 0;

while (current != nullptr && depth < 4) {
switch (PM_NODE_TYPE(current)) {
case PM_CONSTANT_PATH_NODE: {
auto *p = down_cast<pm_constant_path_node>(current);
segments[depth++] = parser.resolveConstant(p->name);
current = p->parent;
if (current == nullptr) {
isRootAnchored = true;
}
break;
}
case PM_CONSTANT_READ_NODE: {
auto *r = down_cast<pm_constant_read_node>(current);
segments[depth++] = parser.resolveConstant(r->name);
current = nullptr;
break;
}
default:
current = nullptr;
break;
}
}

// Only match root-anchored paths that were fully resolved (no remaining parent).
if (isRootAnchored) {
// segments: [innermost, ..., outermost]
if (depth == 3 && segments[2] == "Sorbet" && segments[1] == "Private" &&
segments[0] == "Static") {
return MK::Constant(location, core::Symbols::Sorbet_Private_Static());
}
if (depth == 3 && segments[2] == "T" && segments[1] == "Sig" &&
segments[0] == "WithoutRuntime") {
return MK::Constant(location, core::Symbols::T_Sig_WithoutRuntime());
}
if (depth == 4 && segments[3] == "Sorbet" && segments[2] == "Private" &&
segments[1] == "Static" && segments[0] == "Void") {
return MK::Constant(location, core::Symbols::void_());
}
}
}

if (auto *prismParentNode = node->parent) {
// This constant reference is chained onto another constant reference.
// E.g. given `A::B::C`, if `node` is pointing to the root, `A::B` is the `parent`, and `C` is the
Expand Down
3 changes: 3 additions & 0 deletions rbs/CommentsAssociator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ void CommentsAssociator::walkNode(parser::Node *node) {
[&](parser::AndAsgn *andAsgn) {
associateAssertionCommentsToNode(andAsgn->right.get(), true);
walkNode(andAsgn->right.get());
walkNode(andAsgn->left.get());
consumeCommentsInsideNode(node, "and_asgn");
},
[&](parser::Array *array) {
Expand Down Expand Up @@ -599,6 +600,7 @@ void CommentsAssociator::walkNode(parser::Node *node) {
[&](parser::OpAsgn *opAsgn) {
associateAssertionCommentsToNode(opAsgn->right.get(), true);
walkNode(opAsgn->right.get());
walkNode(opAsgn->left.get());
consumeCommentsInsideNode(node, "op_asgn");
},
[&](parser::Or *or_) {
Expand All @@ -610,6 +612,7 @@ void CommentsAssociator::walkNode(parser::Node *node) {
[&](parser::OrAsgn *orAsgn) {
associateAssertionCommentsToNode(orAsgn->right.get(), true);
walkNode(orAsgn->right.get());
walkNode(orAsgn->left.get());
consumeCommentsInsideNode(node, "or_asgn");
},
[&](parser::Pair *pair) {
Expand Down
42 changes: 31 additions & 11 deletions rbs/prism/CommentsAssociatorPrism.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,10 @@ void CommentsAssociatorPrism::associateSignatureCommentsToNode(pm_node_t *node)
signaturesForNode[node] = move(comments);
}

bool CommentsAssociatorPrism::typeAliasAllowedInContext() {
return !contextAllowingTypeAlias.empty() && contextAllowingTypeAlias.back().first;
}

// Finds standalone RBS comments (not attached to any Ruby code) between lastLine and currentLine,
// and inserts synthetic placeholder nodes into the AST. 2 types of placeholders are created:
// 1. Bind comments (#: self as Type): placeholders later replaced with T.bind(self, Type)
Expand Down Expand Up @@ -281,18 +285,20 @@ int CommentsAssociatorPrism::maybeInsertStandalonePlaceholders(pm_node_list_t &n
std::match_results<std::string_view::const_iterator> matches;
if (std::regex_match(commentString.begin(), commentString.end(), matches, TYPE_ALIAS_PATTERN)) {
// Type aliases are only allowed in class/module bodies
if (!contextAllowingTypeAlias.empty()) {
if (auto [allow, loc] = contextAllowingTypeAlias.back(); !allow) {
if (auto e = ctx.beginError(it->second.loc, core::errors::Rewriter::RBSUnusedComment)) {
e.setHeader("Unexpected RBS type alias comment");
e.addErrorLine(
ctx.locAt(loc),
"RBS type aliases are only allowed in class and module bodies, not in method bodies");
if (!typeAliasAllowedInContext()) {
if (auto e = ctx.beginError(it->second.loc, core::errors::Rewriter::RBSUnusedComment)) {
e.setHeader("Unexpected RBS type alias comment");
if (!contextAllowingTypeAlias.empty()) {
auto loc = contextAllowingTypeAlias.back().second;
e.addErrorLine(ctx.locAt(loc),
"RBS type aliases are only allowed in class and module bodies. Found in:");
} else {
e.addErrorNote("RBS type aliases are only allowed in class and module bodies");
}

it = commentByLine.erase(it);
continue;
}

it = commentByLine.erase(it);
continue;
}

// Register the constant name (e.g., "type foo") in the symbol table
Expand Down Expand Up @@ -374,8 +380,22 @@ void CommentsAssociatorPrism::processTrailingComments(pm_node_t *node, pm_node_l
}

pm_node_t *CommentsAssociatorPrism::walkBody(pm_node_t *node, pm_node_t *body) {
if (typeAliasAllowedInContext() && body == nullptr) {
auto loc = translateLocation(node->location);
int endLine = core::Loc::pos2Detail(ctx.file.data(ctx), loc.endPos()).line;
pm_node_list_t nodes = prism.emptyNodeList();

maybeInsertStandalonePlaceholders(nodes, 0, lastLine, endLine);
lastLine = endLine;

if (nodes.size > 0) {
return prism.StatementsNode(loc, absl::MakeSpan(nodes.nodes, nodes.size));
}
return nullptr;
}

if (body == nullptr) {
pm_node_list_t nodes = {0, 0, NULL};
pm_node_list_t nodes = prism.emptyNodeList();
processTrailingComments(node, nodes);

if (nodes.size > 0) {
Expand Down
1 change: 1 addition & 0 deletions rbs/prism/CommentsAssociatorPrism.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class CommentsAssociatorPrism {
core::LocOffsets translateLocation(pm_location_t location);
uint32_t posToLine(uint32_t pos);

bool typeAliasAllowedInContext();
int maybeInsertStandalonePlaceholders(pm_node_list_t &nodes, int index, int lastLine, int currentLine);
pm_node_t *createSyntheticPlaceholder(const CommentNodePrism &comment, pm_constant_id_t marker);
};
Expand Down
56 changes: 8 additions & 48 deletions test/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,12 @@ pipeline_tests(
"testdata/**/*.exp",
],
exclude = [
# Specific test that is modified for Prism, contains minor differences in the exp file
# Specific tests that are modified for Prism, contain minor differences in the exp file
"testdata/rbs/assertions_block_modified.rb",
"testdata/rbs/assertions_csend_assign_modified.rb",
"testdata/rbs/assertions_csend_modified.rb",
"testdata/rbs/assertions_heredoc_modified.rb",
"testdata/rbs/signatures_types_modified.rb",
],
),
"PosTests",
Expand Down Expand Up @@ -533,56 +537,12 @@ pipeline_tests(
"testdata/parser/error_recovery/csend_masgn.rb",
"testdata/parser/misc.rb",

# Fixing a bug in pipeline_test_runner.cc revealed that these tests
# were not actually running because they have RBS support enabled
"testdata/lsp/hover_rbs.rb",
"testdata/lsp/hover_rbs_assertions_let.rb",
"testdata/rbs/annotations_helpers.rb",
"testdata/rbs/assertions_absurd.rb",
"testdata/rbs/assertions_and_assign.rb",
"testdata/rbs/assertions_and_or.rb",
"testdata/rbs/assertions_array.rb",
"testdata/rbs/assertions_assign.rb",
"testdata/rbs/assertions_bind.rb",
"testdata/rbs/assertions_block.rb",
"testdata/rbs/assertions_break.rb",
"testdata/rbs/assertions_case.rb",
"testdata/rbs/assertions_case_pattern.rb",
"testdata/rbs/assertions_class.rb",
# RBS tests whose rewrite-tree.exp files have legacy-parser-specific
# output (temp variable ordering, constant resolution, heredoc handling).
# Prism-specific _modified variants exist for heredoc and signatures_types.
"testdata/rbs/assertions_csend.rb",
"testdata/rbs/assertions_csend_assign.rb",
"testdata/rbs/assertions_def.rb",
"testdata/rbs/assertions_defs.rb",
"testdata/rbs/assertions_ensure.rb",
"testdata/rbs/assertions_errors.rb",
"testdata/rbs/assertions_for.rb",
"testdata/rbs/assertions_hash.rb",
"testdata/rbs/assertions_heredoc.rb",
"testdata/rbs/assertions_heredoc_modified.rb",
"testdata/rbs/assertions_if.rb",
"testdata/rbs/assertions_kwbegin.rb",
"testdata/rbs/assertions_massign.rb",
"testdata/rbs/assertions_module.rb",
"testdata/rbs/assertions_next.rb",
"testdata/rbs/assertions_op_assign.rb",
"testdata/rbs/assertions_or_assign.rb",
"testdata/rbs/assertions_rescue.rb",
"testdata/rbs/assertions_sclass.rb",
"testdata/rbs/assertions_send.rb",
"testdata/rbs/assertions_send_assign.rb",
"testdata/rbs/assertions_super.rb",
"testdata/rbs/assertions_type_params.rb",
"testdata/rbs/assertions_unless.rb",
"testdata/rbs/assertions_until.rb",
"testdata/rbs/assertions_while.rb",
"testdata/rbs/empty_if_else_module.rb",
"testdata/rbs/signatures_attrs.rb",
"testdata/rbs/signatures_attrs_multiline.rb",
"testdata/rbs/signatures_blocks.rb",
"testdata/rbs/signatures_defs.rb",
"testdata/rbs/signatures_defs_multiline.rb",
"testdata/rbs/signatures_generics.rb",
"testdata/rbs/signatures_type_aliases.rb",
"testdata/rbs/signatures_types.rb",
],
),
Expand Down
Loading
Loading