-
Notifications
You must be signed in to change notification settings - Fork 541
Comparing changes
Open a pull request
base repository: sorbet/sorbet
base: master
head repository: Shopify/sorbet
compare: prism-squashed
Commits on Feb 18, 2025
-
Configuration menu - View commit details
-
Copy full SHA for 14cd050 - Browse repository at this point
Copy the full SHA 14cd050View commit details -
Initial attempt at Prism to Sorbet translation layer Create and run Prism benchmarking script Translate more node types in `pipeline.cc` Wrap Prism C API into new `Prism::Parser` class And move all other Prism related code to the new parser/prism component Translate more node types Introduce memory comparison script for Prism and Sorbet parsers Translate more node types Write memory comparison script for Prism and Sorbet parser Translate more node types Add VS Code tasks for common operations Use `setup-bazel` to cache Bazel in CI Compare location data between Prism and Sorbet parsers To make sure our translation layer correctly translates node locations, we create a task that, for every Prism regression test, runs the test with location comparison on. Merge location and regression testing tasks Translate more node types CI improvements Translate more node types Use a `repository_rule` to get the RUBY_ROOT env var Up to this point, in order to build Prism, we've needed to manually pass the path to the correct RUBY version to the build process because Bazel does not take the user's environment into account and will just use the system's default Ruby version (which is often very old). This is a workaround for our workaround, where we use a Bazel `repository_rule` to get the `RUBY_ROOT` environment variable, write it to a file within the Bazel file system, and then pull the value from that file while building Prism. Fix output of Prism location test errors Co-Authored-By: Vinicius Stock <vinicius.stock@shopify.com> Co-Authored-By: Alex Momchilov <alexander.momchilov@shopify.com> Co-Authored-By: Emily Samp <emily.samp@shopify.com> Co-Authored-By: Alexander Momchilov <amomchilov@users.noreply.github.com>
Configuration menu - View commit details
-
Copy full SHA for b9cf0f2 - Browse repository at this point
Copy the full SHA b9cf0f2View commit details -
Configuration menu - View commit details
-
Copy full SHA for 6991f89 - Browse repository at this point
Copy the full SHA 6991f89View commit details -
Configuration menu - View commit details
-
Copy full SHA for 679d5a1 - Browse repository at this point
Copy the full SHA 679d5a1View commit details -
Add launch.json config to debug Sorbet with Prism
This debug config launches the main Sorbet executable to type check the provided file path (with Prism). Add vscode-lldb to recommended extensions list Add support for debugging Prism tests in VSCode Remove unnecessary task to set BAZEL_EXEC_ROOT It turns out that we only need `sourceMap`'s value to be set to the workspace folder with any arbitrary key. So not setting `BAZEL_EXEC_ROOT` is fine.
Configuration menu - View commit details
-
Copy full SHA for 82f5288 - Browse repository at this point
Copy the full SHA 82f5288View commit details -
Configuration menu - View commit details
-
Copy full SHA for 4b19f96 - Browse repository at this point
Copy the full SHA 4b19f96View commit details -
nullptr
check duringENFORCE
indowncast
In our `downcast` helper, we check that the node argument is actually a Prism node. However, doing this requires us to check the `type` field, which will fail if the node is a `nullptr`. By doing a `nullptr` check in the `ENFORCE`, we guarantee we won't crash by checking for a field on a `nullptr`. Co-authored-by: Alex Momchilov <alexander.momchilov@shopify.com>
Configuration menu - View commit details
-
Copy full SHA for 8006ffb - Browse repository at this point
Copy the full SHA 8006ffbView commit details -
Fix bugs in translation of PM_RESCUE_NODE
Only do dynamic constant assignment workaround in specific cases Only do the dynamic constant assignment workaround when translating a constant from a `PM_CONSTANT_WRITE_NODE` and `PM_CONSTANT_PATH_NODE`. Test dynamic constant workaround for all constant assignment types Add more test cases to rescue prism regression tests Handle all constant path assignment node types Shopify#317
Configuration menu - View commit details
-
Copy full SHA for 0f67c16 - Browse repository at this point
Copy the full SHA 0f67c16View commit details -
Stop surfacing Prism's EOF error It usually appears when there's an unclosed statements/assignments, where the root error will also be surfaced. So it's not exactly important to surface this error. But more importantly, because Prism puts this error at the very last line of the file when there's a missing `end`, it can't be represented in Sorbet's error comments. Keep Prism error id's type for easy comparison We currently don't use the converted value and keeping the `pm_diagnostic_id_t` type makes it easy to compare the value with `PM_ERR_*` constants directly. Add a task to display Prism's parse errors Address feedback Change location test's naming convention By not replacing `/` with `_`, the test naming structure is more consistent with normal regression tests. But more importantly, it allows us to run location tests that are under a folder like `error_recovery/assign`, more easily. Generate error for Sorbet's `assign.rb` error recovery fixture The error location doesn't match what Sorbet currently generates. But the idea is to first make parse trees match, accumulate more recovery tests, and then come up with a strategy to generate errors that match locations. Shopify#318 Shopify#319
Configuration menu - View commit details
-
Copy full SHA for 4806905 - Browse repository at this point
Copy the full SHA 4806905View commit details -
To make it clearer that these are the regression tests we've built, as opposed to pre-existing Sorbet tests Add test corpus with Prism The purpose of these tests is to run the Sorbet tests that already exist, but to use the Prism parser instead of the Sorbet parser. As we fix more edge cases, we will include more of the test suite and run them in CI. Add tasks to run corpus tests with Prism Update the test target on CI Add Prism handling for packager tests too Because pipeline_tests handles packager tests differently, we need to add Prism handling for them too. Otherwise, we'll get test conflict errors when `test_corpus_prism`'s pattern covers packager tests. Follow up on #323 - Fix the command of single Prism regression test task - Update launch.json with the new task name Fix `launch.json` for prism regression tests Make test Pipeline parse with Prism in the case of `indexOne` Seperate test input for regression and corpus tests Remove error recovery tests from prism test corpus And start running corpus tests in CI Co-Authored-By: Emily Samp <emily.samp@shopify.com>
Configuration menu - View commit details
-
Copy full SHA for a267bfa - Browse repository at this point
Copy the full SHA a267bfaView commit details -
Configuration menu - View commit details
-
Copy full SHA for 91f54de - Browse repository at this point
Copy the full SHA 91f54deView commit details -
Configuration menu - View commit details
-
Copy full SHA for 5aaf5c9 - Browse repository at this point
Copy the full SHA 5aaf5c9View commit details -
Translator should not treat
~[Integer]
as a method callSorbet's legacy parser treats `~[Integer]` and `-[Integer]` as integer literals, but Prism treats `~[Integer]` as a method call. (Technically, both of them are method calls.) So in the translator, we need to give `~` special handling when its receiver is an integer. We don't need the same for Float or Numeric because only Integer has the `~` method.
Configuration menu - View commit details
-
Copy full SHA for c6bb135 - Browse repository at this point
Copy the full SHA c6bb135View commit details -
Properly translate complex literals with prefix
-
or+
Unlike Integer or Float, Complex literals with prefix `-` or `+` should be translated to method calls on the Complex literal.
Configuration menu - View commit details
-
Copy full SHA for 24d5c1d - Browse repository at this point
Copy the full SHA 24d5c1dView commit details -
Configuration menu - View commit details
-
Copy full SHA for 413da3c - Browse repository at this point
Copy the full SHA 413da3cView commit details -
Configuration menu - View commit details
-
Copy full SHA for a9dff1d - Browse repository at this point
Copy the full SHA a9dff1dView commit details -
Configuration menu - View commit details
-
Copy full SHA for 3459680 - Browse repository at this point
Copy the full SHA 3459680View commit details -
Persist unique counter value between translator instances
When we create a new translator instance, we haven't been maintaining the value of the unique counter, which keeps track of anonymous arguments in order to give them unique names. This was resulting in different parse trees for Sorbet and Prism.
Configuration menu - View commit details
-
Copy full SHA for 7395cc3 - Browse repository at this point
Copy the full SHA 7395cc3View commit details -
Add desugar tests to Prism corpus
Exclude any that are failing because of error recovery or legitimate errors.
Configuration menu - View commit details
-
Copy full SHA for 511a563 - Browse repository at this point
Copy the full SHA 511a563View commit details -
Make LHS of conditional call operator assignment
CSend
Remove desugar/csend test from exclusions
Configuration menu - View commit details
-
Copy full SHA for 548f088 - Browse repository at this point
Copy the full SHA 548f088View commit details -
Improve handling of
uniqueCounter
in Prism translator`uniqueCounter` is a counter that is used to give unique ids to anonymous arguments while parsing them in Sorbet. Thus far, we have coordinated the `uniqueCounter` between multiple Translators by passing the value back and forth, but this approach is brittle. With this new approach, we can maintain the same `uniqueCounter` between multiple translators: 1. The parent translator maintains the source-of-truth counter in a variable called `uniqueCounterStorage` 2. It also maintains a pointer to that value called `uniqueCounter` 3. Child translators are created with a dummy value in `uniqueCounterStorage` that will never be used, as well as the `uniqueCounter` pointer that points to their parent translator's storage 4. Incrementing the `uniqueCounter` happens via the pointer, which will always point to the parent's `uniqueCounter` value Co-authored-by: Alexander Momchilov <alexander.momchilov@shopify.com>
Configuration menu - View commit details
-
Copy full SHA for 8d5c96e - Browse repository at this point
Copy the full SHA 8d5c96eView commit details -
Translate def nodes with receivers as
DefS
Add prism regression test for singleton method defs Add more newly-passing tests to Prism corpus Add `desugar/defs_not_self` to Prism corpus tests
Configuration menu - View commit details
-
Copy full SHA for ddb7168 - Browse repository at this point
Copy the full SHA ddb7168View commit details -
Standardize handling of block arguments in Prism -> Sorbet translation
Handle forwarding super node with block argument Correctly handle calls to `super` with block Change argument name to `blockArgumentNode` Refactor `translateArguments` to remove unused parameter And make block parameter optional Refactor index operator writes to use refactored arguments logic Refactor `PM_CALL_NODE` case to use refactored arguments logic Refactor `PM_INDEX_TARGET_NODE` case to use refactored arguments logic Refactor `PM_SUPER_NODE` case to use refactored arguments logic Refactor `translateArguments` to handle block argument as well Add a `super` test case to `def_block_param` regression test Pass block arguments to super calls Add case to `def_singleton` Prism regression test Testing the translation of a call to super that passes a block argument.
Configuration menu - View commit details
-
Copy full SHA for 058e653 - Browse repository at this point
Copy the full SHA 058e653View commit details -
Fix splat arg translation in Prism
When the splat arg's expression is an `Arg`, we need to translate it to a `Restarg` instead of a `SplatLhs`.
Configuration menu - View commit details
-
Copy full SHA for bd62043 - Browse repository at this point
Copy the full SHA bd62043View commit details -
Handle implicit rest nodes in array patterns
1. We don't need to translate implicit rest nodes in array patterns 2. We need to return an `ArrayPatternWithTail` instead of an `ArrayPattern` when the pattern ends with an implicit rest node
Configuration menu - View commit details
-
Copy full SHA for 039adb0 - Browse repository at this point
Copy the full SHA 039adb0View commit details -
Configuration menu - View commit details
-
Copy full SHA for 4b230df - Browse repository at this point
Copy the full SHA 4b230dfView commit details -
Skip error message checking when running Corpus tests with Prism
Currently, corpus tests may fail with Prism for a few reasons: 1. The error checking fails with Prism. This is expected. 2. There are tree mismatches. And sometimes, a test fail because of both 1 and 2. This means that by skipping the test altogether, we are potentially skipping legitimately failing tests. This commit updates the test runner to skip the error message checking when running corpus tests with Prism and updates the skipped tests in the BUILD file. So now if a test is skipped, it's only because of the second reason. And we should minimize the number of tests that are skipped in that case. Co-authored-by: Emily Samp <emily.samp@shopify.com>
Configuration menu - View commit details
-
Copy full SHA for 9feb9fb - Browse repository at this point
Copy the full SHA 9feb9fbView commit details -
Rather than specifying whether to *skip* the logic, we should say whether to replace the constant node with a dynamic const assignment. Then, remove unnecessary type check and move this logic to the top of the method so we short-circuit any unnecessary code in that case. Run two more tests that are only failing because of error differences Remove `desugar/regex` from failing tests Remove `desugar/range` from excluded tests Co-Authored-By: Stan Lo <stan.lo@shopify.com>
Configuration menu - View commit details
-
Copy full SHA for de154bc - Browse repository at this point
Copy the full SHA de154bcView commit details -
Improve pattern matching support with hash patterns
1. It fixes the case where a hash pattern doesn't have a value, which will generate a `PM_IMPLICIT_NODE` that we currently don't handle. 2. It fixes the translation of hash patterns with a named splat node, like `in **o`.
Configuration menu - View commit details
-
Copy full SHA for 07e70be - Browse repository at this point
Copy the full SHA 07e70beView commit details -
Configuration menu - View commit details
-
Copy full SHA for 7cb802e - Browse repository at this point
Copy the full SHA 7cb802eView commit details -
Configuration menu - View commit details
-
Copy full SHA for c51836c - Browse repository at this point
Copy the full SHA c51836cView commit details -
Introduce
isKeywordHashElement
method to fixdef_delegator
testRun `rewriter/def_delegator` test as part of prism corpus Remove `rewriter/rails/delegate` test from skipped rewriter tests Refactor `translateHash` into `translateKeyValuePairs` The logic of `translateHash` was kind of confusing because it was handling both vanilla hashes and also assocs/assoc splat nodes. By isolating one part of that method, which generates key/value pairs, and then calling that method in both cases, the two code paths become cleaner and more obvious.
Configuration menu - View commit details
-
Copy full SHA for 5fd9a35 - Browse repository at this point
Copy the full SHA 5fd9a35View commit details -
Configuration menu - View commit details
-
Copy full SHA for 69f69c2 - Browse repository at this point
Copy the full SHA 69f69c2View commit details -
Fix bracket method call's location translation
When the method call is `[]`, the message location should be just the begin location of `[]`.
Configuration menu - View commit details
-
Copy full SHA for efa115a - Browse repository at this point
Copy the full SHA efa115aView commit details -
Configuration menu - View commit details
-
Copy full SHA for 26ce359 - Browse repository at this point
Copy the full SHA 26ce359View commit details -
Handle translation of post params
Add `parser/kwnil_errors` to skipped tests with a note Run `local_vars/z_super_args_splats_blocks` test in Prism corpus
Configuration menu - View commit details
-
Copy full SHA for 379e603 - Browse repository at this point
Copy the full SHA 379e603View commit details -
Configuration menu - View commit details
-
Copy full SHA for b31dd3d - Browse repository at this point
Copy the full SHA b31dd3dView commit details -
Configuration menu - View commit details
-
Copy full SHA for 3cc25ae - Browse repository at this point
Copy the full SHA 3cc25aeView commit details -
Add namer tests to prism corpus
And exclude failing tests
Configuration menu - View commit details
-
Copy full SHA for 220ec4f - Browse repository at this point
Copy the full SHA 220ec4fView commit details -
Modify locations of kw arguments and params to match legacy parser
Add `namer/arguments` test to Prism corpus Run two more namer tests that now pass
Configuration menu - View commit details
-
Copy full SHA for 96babd5 - Browse repository at this point
Copy the full SHA 96babd5View commit details -
Fix call with empty message's location translation
In cases like `foo.()`, the message would be `:call`, but the message location would have null start and end. In this case, we need to fall back to use the call operator location. Include LSP tests in Prism Corpus tests
Configuration menu - View commit details
-
Copy full SHA for b026c49 - Browse repository at this point
Copy the full SHA b026c49View commit details -
Correctly translate constant write nodes' location
This is not covered in parse tree location tests, but reflected in symbol table tests: when we write `FOO = 1` (or other constant write nodes), the location of `FOO` should just be the name, not the entire assignment. In those cases, we should use `name_loc` instead of `base.location`.
Configuration menu - View commit details
-
Copy full SHA for 1b2de19 - Browse repository at this point
Copy the full SHA 1b2de19View commit details -
Fix symbol location for hash, keyword argument keys
When the symbol is used as `a: 1`, the location should not include the colon. Run Prism Corpus against all Sorbet component tests
Configuration menu - View commit details
-
Copy full SHA for c5d2dc7 - Browse repository at this point
Copy the full SHA c5d2dc7View commit details -
Use improved libprism source to build Prism without dependency on Ruby The main blocker for upstreaming Prism parser to Sorbet is that we currently rely on Ruby to build Prism, as its required by Prism's `rake template` task. But by package the files generated by `rake template` in Prim's release, we will be able to remove the dependency on Ruby while significantly simplify the build configurations for Prism. Use another mock source
Configuration menu - View commit details
-
Copy full SHA for cb10db7 - Browse repository at this point
Copy the full SHA cb10db7View commit details -
Add formatter for Prism node types
When ENFORCE macros fail, they use fmt (via spdlog) to format error messages. Add a formatter specialization for pm_node_type to properly display Prism node types in these error messages. Instead of showing numeric values, the formatter uses pm_node_type_to_str to display human-readable node type names.
Configuration menu - View commit details
-
Copy full SHA for fdbf9b6 - Browse repository at this point
Copy the full SHA fdbf9b6View commit details -
Configuration menu - View commit details
-
Copy full SHA for c1f8f1b - Browse repository at this point
Copy the full SHA c1f8f1bView commit details -
Configuration menu - View commit details
-
Copy full SHA for 8266cdb - Browse repository at this point
Copy the full SHA 8266cdbView commit details -
Add
sliceLocation
helper to simplify location operations (#406)* Add sliceLocation helper to simplify location operations * Run CI on prism-squahed too
Configuration menu - View commit details
-
Copy full SHA for 55a42d4 - Browse repository at this point
Copy the full SHA 55a42d4View commit details -
Always raise in unreachable code (#407)
Given that some problem may only be found when running a release build against a large codebase, raising in unreachable code makes sure we spot the issues at its source instead of crashing later downstream. Example: https://github.com/sorbet/sorbet/pull/8485/files#r1943210573
Configuration menu - View commit details
-
Copy full SHA for 9c27038 - Browse repository at this point
Copy the full SHA 9c27038View commit details -
Configuration menu - View commit details
-
Copy full SHA for fc831c6 - Browse repository at this point
Copy the full SHA fc831c6View commit details
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.