[DO NOT MERGE] `ko/rbs-over-prism` rebased by KaanOzkan · Pull Request #851 · Shopify/sorbet

KaanOzkan · 2026-03-09T21:26:59Z

Motivation

Test plan

See included automated tests.

Squash merge of ko/rbs-over-prism onto master. All conflicts resolved by accepting master's version. Branch-only features will be ported in a follow-up commit.

Remove the `&& false` guard that was disabling the RBS rewriter in the Prism parser path. The RBS rewriter operates on raw Prism AST nodes (pm_node_t*) before translation, so no Translator changes are needed.

Defensive assertion to catch callers passing anonymous parameter names (PM_CONSTANT_ID_UNSET) to resolveConstant(). Originally from cafe258 ("Resolve anonymous arg crash") on ko/rbs-over-prism.

Walk the left-hand side of compound assignment nodes so that RBS comments on send targets (e.g., `a.b &&= val`) are properly associated. Originally from 36367ef ("Handle assignments") on ko/rbs-over-prism.

Port Jesse's PR sorbet#9159 (whitequark CommentsAssociator) to the Prism version. Adds typeAliasAllowedInContext() helper, distinguishes method body vs top-level type alias errors, and handles type aliases in empty class/module bodies. Originally from 9131604 on ko/rbs-over-prism.

The squash merge brought in stale branch code for non-conflicting hunks, reverting recent master improvements (VisibilityChecker API change, packager rework, resolver cleanup, test runner updates, etc.). Reset 19 files to master's version and fixed 2 VisibilityChecker call sites in pipeline.cc that were using the old move-semantics API.

- Add missing `# error:` annotations for strict-mode sig requirement on method_with_only_ensure and method_with_only_rescue - Update rewrite-tree expectations for master's Translator constant resolution (::Kernel, ::NilClass instead of <emptyTree>::<C ...>) - Update intersection type expectation (Numeric & Comparable simplifies to Numeric on master)

The squash merge brought in branch changes that moved IdentKindHelper outside the namespace and added unnecessary includes/forward declarations. These were artifacts of the branch's older Translator code. Reset to master's version which is identical in functionality.

Two changes ported from the RBS-over-Prism branch to master's Translator: 1. Bounds check in desugarStatements: When the RBS rewriter appends synthetic nodes (e.g. self.abstract!()) with comment locations to the end of a statement list, front()->location.start can exceed back()->location.end, violating the LocOffsets invariant. Fall back to the statements node's own location in that case. 2. Constant path optimization in translateConst: Eagerly resolve well-known constant paths (::Sorbet::Private::Static, ::T::Sig::WithoutRuntime, ::Sorbet::Private::Static::Void) to their symbols instead of leaving them as UnresolvedConstant nodes. Adapted from branch's parser::ResolvedConst to master's MK::Constant API.

amomchilov · 2026-03-09T21:39:32Z

common/common.h

 #include "absl/container/flat_hash_map.h"
 #include "absl/container/flat_hash_set.h"
 #include "absl/container/inlined_vector.h"
+#include "absl/types/span.h"


Might be controversial, idk. It's pretty commonly used like everywhere, so it makes sense, but checkin with Stripe first

amomchilov · 2026-03-09T21:40:53Z

main/pipeline/pipeline.h

 #include "common/kvstore/KeyValueStore.h"
 #include "core/FileHash.h"
 #include "main/options/options.h"
+#include "parser/prism/Parser.h"


I assume some code is missing in this file. There's no new APIs here that use Prism APIs, so there's no need to add this header

amomchilov · 2026-03-09T21:41:16Z

parser/prism/Translator.cc

        // 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


Suggested change

// list, those nodes may have locations earlier in the file than the real code (e.g., from comment

// list, those nodes may have locations earlier in the file than the real code (e.g. from comment

amomchilov · 2026-03-09T21:45:43Z

parser/prism/Translator.cc

+        // Optimization: resolve well-known constant paths eagerly instead of leaving them unresolved.
+        // This is important for the RBS rewriter which generates references to these constants.
+        {
+            absl::InlinedVector<core::NameRef, 4> fullPath;


Would benefit from a comment:

Suggested change

absl::InlinedVector<core::NameRef, 4> fullPath;

// Max size of 4, to fit the longest path we're searching for (`Sorbet::Private::Static::Void`)

absl::InlinedVector<core::NameRef, 4> fullPath;

amomchilov · 2026-03-09T21:49:28Z

parser/prism/Translator.cc

+            bool isRootAnchored = false;
+            pm_node_t *current = up_cast(const_cast<PrismLhsNode *>(node));
+
+            while (current != nullptr) {


In a follow up, can we tighten this up a bit?

Cap the iterations to 4. Any more than that, and we start heap-allocating elements of a path that we know for sure won't be matched below

Bail early if the name at each iteration doesn't match one of the patterns we're looking for. Since the PM_CONSTANT_PATH_NODE is backwards:

On the first iteration, we're looking for Static, Void, or WithoutRuntime

On the second iteration we're looking for Private, Sig, or Static,

etc.

amomchilov · 2026-03-09T21:50:45Z

rbs/prism/CommentsAssociatorPrism.cc

+    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 = {0, 0, NULL};


Suggested change

pm_node_list_t nodes = {0, 0, NULL};

pm_node_list_t nodes = prism.emptyNodeList();

amomchilov · 2026-03-09T21:51:33Z

test/testdata/rbs/assertions_bind.rb

+    #: self as Foo
+    T.reveal_type(self) # error: Revealed type: `Foo`


There's no difference between the two branches. What's the intent?

pipeline.cc: - Always use prismDesugar::node2Tree for Prism (translate_TODO output produces ExprOnly nodes incompatible with legacy desugar::node2Tree) - Skip legacy runRBSRewrite when RBS enabled (Prism RBS rewriter already ran inside runPrismParser) pipeline_test_runner.cc: - Add runPrismWithRBS() adapted to master's Translator API - Route Prism+RBS through prismDesugar::node2Tree - Skip rbs-rewrite-tree observation (ExprOnly can't serialize) - Skip parser comparison for Prism+RBS path Delete .rewrite-tree.exp files for modified RBS tests (can't generate with translate_TODO's ExprOnly nodes).

Previously all 48 RBS tests were excluded from PrismPosTests because the test runner didn't support Prism+RBS. Now that runPrismWithRBS() is implemented, they all pass with Prism. 4 tests have rewrite-tree.exp differences between parsers (temp variable ordering for csend, constant resolution, heredoc string merging). Create _modified variants with Prism-specific exp files for csend/csend_assign; heredoc and signatures_types already had _modified variants. When Prism becomes the only parser, the _modified versions become canonical and the originals can be removed.

KaanOzkan added 9 commits March 9, 2026 16:26

[Squashed] ko/rbs-over-prism: RBS over Prism (master-resolved conflicts)

b9b0a6c

Squash merge of ko/rbs-over-prism onto master. All conflicts resolved by accepting master's version. Branch-only features will be ported in a follow-up commit.

Enable RBS-over-Prism by removing && false guard

777b888

Remove the `&& false` guard that was disabling the RBS rewriter in the Prism parser path. The RBS rewriter operates on raw Prism AST nodes (pm_node_t*) before translation, so no Translator changes are needed.

Add ENFORCE check in resolveConstant() for PM_CONSTANT_ID_UNSET

ad2bd02

Defensive assertion to catch callers passing anonymous parameter names (PM_CONSTANT_ID_UNSET) to resolveConstant(). Originally from cafe258 ("Resolve anonymous arg crash") on ko/rbs-over-prism.

Walk LHS of AndAsgn, OpAsgn, OrAsgn in CommentsAssociator

0da88a7

Walk the left-hand side of compound assignment nodes so that RBS comments on send targets (e.g., `a.b &&= val`) are properly associated. Originally from 36367ef ("Handle assignments") on ko/rbs-over-prism.

KaanOzkan changed the title ~~Ko/rbs over prism rebased~~ [DO NOT MERGE] ko/rbs-over-prism rebased Mar 9, 2026

amomchilov reviewed Mar 9, 2026

View reviewed changes

KaanOzkan added 2 commits March 10, 2026 11:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

[DO NOT MERGE] `ko/rbs-over-prism` rebased#851
KaanOzkan wants to merge 11 commits intomasterfrom
ko/rbs-over-prism-rebased

KaanOzkan commented Mar 9, 2026

Uh oh!

amomchilov Mar 9, 2026

Uh oh!

amomchilov Mar 9, 2026

Uh oh!

amomchilov Mar 9, 2026

Uh oh!

amomchilov Mar 9, 2026

Uh oh!

amomchilov Mar 9, 2026

Uh oh!

amomchilov Mar 9, 2026

Uh oh!

amomchilov Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// list, those nodes may have locations earlier in the file than the real code (e.g., from comment
	// list, those nodes may have locations earlier in the file than the real code (e.g. from comment

	absl::InlinedVector<core::NameRef, 4> fullPath;
	// Max size of 4, to fit the longest path we're searching for (`Sorbet::Private::Static::Void`)
	absl::InlinedVector<core::NameRef, 4> fullPath;

	pm_node_list_t nodes = {0, 0, NULL};
	pm_node_list_t nodes = prism.emptyNodeList();

		#: self as Foo
		T.reveal_type(self) # error: Revealed type: `Foo`

Conversation

KaanOzkan commented Mar 9, 2026

Motivation

Test plan

Uh oh!

amomchilov Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

amomchilov Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

amomchilov Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

amomchilov Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

amomchilov Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

amomchilov Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

amomchilov Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants