Report unterminated construct errors at opening token - #3827
Merged
kddnewton merged 2 commits intoDec 29, 2025
Conversation
This commit adds an `expect1_opening` function that expects a token and
attaches the error to the opening token location rather than the current
position. This is useful for errors about missing closing tokens, where
we want to point to the line with the opening token rather than the end
of the file.
For example:
```ruby
def foo
def bar
def baz
^ expected an `end` to close the `def` statement
^ expected an `end` to close the `def` statement
^ expected an `end` to close the `def` statement
```
This would previously produce three identical errors at the end of the
file. After this commit, they would be reported at the opening token
location:
```ruby
def foo
^~~ expected an `end` to close the `def` statement
def bar
^~~ expected an `end` to close the `def` statement
def baz
^~~ expected an `end` to close the `def` statement
```
I considered using the end of the line where the opening token is
located, but in some cases that would be less useful than the opening
token location itself. For example:
```ruby
def foo def bar def baz
```
Here the end of the line where the opening token is located would be the
same for each of the unclosed `def` nodes.
kddnewton
approved these changes
Dec 29, 2025
kddnewton
left a comment
Collaborator
There was a problem hiding this comment.
This is great, I love it.
Yeah in reality this is showing a limitation of our error messages where they only refer to a single location. Other languages have more sophisticated setups where it can refer to an origination point as well as when the error was detected, which is really what we should work toward. But I think this is a really good step in that direction (especially because it finds all of the call sites for us to extend later).
This was referenced Jan 7, 2026
matzbot
pushed a commit
to ruby/ruby
that referenced
this pull request
Jan 15, 2026
…ning` Followup to ruby/prism#3827 It sets the start to the opening but it should instead just continue on as usual. Fixes ruby/prism#3851 Notice how the AST actually contains "123" in both the body and end keyword. ruby/prism@8f69c5af08
marcoroth
added a commit
to marcoroth/herb
that referenced
this pull request
Jan 17, 2026
Prism 1.8.0 (ruby/prism#3827) changed how the `closing_loc` is populated for unclosed blocks. Previously it was `NULL` for unclosed blocks, but now it's always set (pointing to body content). This broke our detection logic that relied on checking if `closing_loc` was empty. https://github.com/ruby/prism/releases/tag/v1.8.0
kddnewton
pushed a commit
to kddnewton/ruby
that referenced
this pull request
Jan 27, 2026
…ning` Followup to ruby/prism#3827 It sets the start to the opening but it should instead just continue on as usual. Fixes ruby/prism#3851 Notice how the AST actually contains "123" in both the body and end keyword. ruby/prism@8f69c5af08
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
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.
👋 We're working on integrating Prism into Sorbet and I would like to propose an improvement to how unterminated construct errors are reported to end users.
This PR adds an
expect1_openingfunction that expects a token and attaches the error to the opening token location rather than the current position. This is useful for errors about missing closing tokens, where it could be useful to point to the line with the opening token rather than the end of the file.For example:
This would previously produce three identical errors at the end of the file. After this commit, they would be reported at the opening token location:
This way, end users can see which particular def node is considered unclosed, rather than a seeing a collection of errors all pointing to the end of the file. However, I understand this might be considered less technically correct and potentially less useful in certain situations. In the single-line test examples it might seem preferable to show the error at end-of-input, since that's where the closing token should be, but I think in most practical cases that won't be the case.
Alternative
An alternative approach might be to add "hints" to errors so the error location remains end-of-input but an additional hint location could point to the opening token. Clients could choose whether to surface the original error location, the hint location, or both. What do you think?
Tests
As part of this, I added a handful of tests capturing the original behavior which should make it easier to see the effect this proposed change has.