Directly desugar lambdas#833
Closed
amomchilov wants to merge 12 commits intoAlex/extract-desugarMethodCallfrom
Closed
Directly desugar lambdas#833amomchilov wants to merge 12 commits intoAlex/extract-desugarMethodCallfrom
amomchilov wants to merge 12 commits intoAlex/extract-desugarMethodCallfrom
Conversation
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Jan 9, 2026
amomchilov
force-pushed
the
Alex/extract-desugarMethodCall
branch
from
092387a to
d9b0e05
Compare
amomchilov
force-pushed
the
Alex/desugar-lambda
branch
from
9147d39 to
3d2e07b
Compare
* Include parens in multi-target locs * Delete now-unused loc helpers * Fix location of `to_ary` call
* extract code to check for visible_to * inline visibilityApplies * delete this ENFORCE * no-op: reorganize * no-op: causesModularityError -> validToImport * check visible_to at call sites and don't add imports that would violate visible_to * test for gen-packages mode * test for call sites * use absl::c_any_of
* Skip loop if there's no splats at all * Move `messageLoc` declaration * Narrow scope of `blockPassLoc` * Move `blockExpr` declaration up * Move symbol proc desugaring up * Simplify checks for `blockPassArg` * Simplify checks for `blockExpr` * Inline `blockPassArgIsSymbol` * Invert `if` * Narrow scope of block-related variables * Narrow scope of `blockStatsStore` check * Inline `methodName` * Rename `name` → `methodName` * Narrow scope of `prismBlock` * FIXME: move this comment * Extract `desugarLiteralBlock()` Co-authored-by: Jesse Johnson <[email protected]> * Extract `desugarBlockPassArgument()` Co-authored-by: Jesse Johnson <[email protected]> * Compare NameRefs instead of strings * Move receiver handling up * Lift out `argumentsNode` local * Reuse `receiverNode` local * Rename `messageLoc` → `methodNameLoc` * Extract `desugarBlock()` * Lift `block` desugaring up * Migrate `computeMethodCallLoc()` block param * Lift `block_given?` check up * Lift `isPrivateOk` local * Extract `desugarMethodCall()` Co-authored-by: Alexander Momchilov <[email protected]> --------- Co-authored-by: Jesse Johnson <[email protected]> Co-authored-by: Thomas Marshall <[email protected]>
amomchilov
force-pushed
the
Alex/desugar-lambda
branch
5 times, most recently
from
632e796 to
401023e
Compare
* Move getQuickfixEdits to lsp_helpers * prework: Expand concretizeIfAbstract We currently don't use this ShowOption for an `overridable` parent, so this change is a no-op. I thought about having a separate option, so that callers could opt into the overridable or the abstract behavior independently, but I don't think that there are any cases where you really need that level of granularity, so I left it controlled by a single boolean. I thought for a while about what a good name would be, but couldn't come up with anything more compelling then the verbose `concretizeIfAbstractOrOverridable`. Open to suggestions. * Add a ClassDefResponse to LSP QueryResponse One thing to note is that I opted not to implement the `retType` method on `ClassDefResponse`, because this isn't an expression. (Technically it is: it evaluates to the type of the last thing in the class body, but Sorbet doesn't model that. Sorbet lies and says that the return type is `NilClass`, but I figured it would be best to just raise). I looked at all the places where we are calling `retType` to make sure that this wouldn't cause problems, and ended up only needing to change `hover.cc` For my code action, I only had this match if your cursor is on the `declLoc`, not if it's inside the `loc` that spans the entire class body. I think that's something we could reconsider in the future, but I didn't want to do something where having an extra query response in the vector of query responses broke or changed the behavior of anything currently out there—limiting this to only `declLoc` limits the blast radius a bit. It also means that, for example, if your cursor is inside the class body on an empty line or a comment, you aren't presented with a lightbulb icon. I wanted to avoid having it be the case that there's _always_ a lightbulb icon following your cursor around. * Main implementation and tests * Add a quick screencast to the docs * fastmod getQuickfixEdits autocorrect2DocumentEdits * no-op: Remove core:: * Add test for Class.new * Handle case of no responses * clang-format * Fix the <root> declLoc problem * Fix CheckSize for ClassDefResponse
* Move getQuickfixEdits to lsp_helpers * prework: Expand concretizeIfAbstract We currently don't use this ShowOption for an `overridable` parent, so this change is a no-op. I thought about having a separate option, so that callers could opt into the overridable or the abstract behavior independently, but I don't think that there are any cases where you really need that level of granularity, so I left it controlled by a single boolean. I thought for a while about what a good name would be, but couldn't come up with anything more compelling then the verbose `concretizeIfAbstractOrOverridable`. Open to suggestions. * Add a ClassDefResponse to LSP QueryResponse One thing to note is that I opted not to implement the `retType` method on `ClassDefResponse`, because this isn't an expression. (Technically it is: it evaluates to the type of the last thing in the class body, but Sorbet doesn't model that. Sorbet lies and says that the return type is `NilClass`, but I figured it would be best to just raise). I looked at all the places where we are calling `retType` to make sure that this wouldn't cause problems, and ended up only needing to change `hover.cc` For my code action, I only had this match if your cursor is on the `declLoc`, not if it's inside the `loc` that spans the entire class body. I think that's something we could reconsider in the future, but I didn't want to do something where having an extra query response in the vector of query responses broke or changed the behavior of anything currently out there—limiting this to only `declLoc` limits the blast radius a bit. It also means that, for example, if your cursor is inside the class body on an empty line or a comment, you aren't presented with a lightbulb icon. I wanted to avoid having it be the case that there's _always_ a lightbulb icon following your cursor around. * Main implementation and tests * Add a quick screencast to the docs * fastmod getQuickfixEdits autocorrect2DocumentEdits * no-op: Remove core:: * Add test for Class.new * Handle case of no responses * clang-format * Fix the <root> declLoc problem * Fix CheckSize for ClassDefResponse * prework: Add declLoc to MethodDefResponse Previously, `termLoc` was `declLoc`, so I've also changed all the old usages of `termLoc` to `declLoc` to preserve the existing behavior. * prework: Only store the FileRef once to save space * prework: Add isAttrBestEffortUIOnly * Add completion item to override a method * Add a test * Add docs to website * Ban this test The file fails to parse, and prism doesn't handle those yet.
* prework: Add declLoc to MethodDefResponse Previously, `termLoc` was `declLoc`, so I've also changed all the old usages of `termLoc` to `declLoc` to preserve the existing behavior. * prework: Only store the FileRef once to save space * prework: Add isAttrBestEffortUIOnly * Add completion item to override a method * Add a test * Add docs to website * Ban this test The file fails to parse, and prism doesn't handle those yet.
amomchilov
force-pushed
the
Alex/desugar-lambda
branch
from
401023e to
3ad50da
Compare
amomchilov
force-pushed
the
Alex/desugar-lambda
branch
from
3ad50da to
def8927
Compare
Author
|
Upstreamed in sorbet#9825 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Test plan
See included automated tests.