Skip to content
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.

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

Fix NoMethodError for tokens_to_s method #1055

Merged
merged 2 commits into from Nov 16, 2023

Conversation

toshimaru
Copy link
Contributor

Calling tokens_to_s gets an error if token_stream is nil:

undefined method `compact' for nil:NilClass (NoMethodError)

So, fall back to an empty array if @token_stream is nil.

The case token_stream is nil

> rdoc_method.inspect
=> "#<RDoc::AnyMethod:0x5b090 AbstractController::Translation#l (public) (alias for localize)>"
> rdoc_method.token_stream
=> nil

Calling `tokens_to_s` gets an error if `token_stream` is nil:

```
undefined method `compact' for nil:NilClass (NoMethodError)
```

So, fall back to an empty array if `@token_stream` is nil.
@hsbt hsbt merged commit 4a1c74b into ruby:master Nov 16, 2023
21 checks passed
@toshimaru toshimaru deleted the fix-tokens_to_s-error branch November 16, 2023 04:16
@toshimaru
Copy link
Contributor Author

toshimaru commented Nov 16, 2023

@nobu thanks for fixing an issue caused by this change. 🙏
72897d3

toshimaru added a commit to toshimaru/sdoc that referenced this pull request Nov 17, 2023
Due to the change in ruby/rdoc#1055,
`token_stream` returns empty Array instead of `nil`,
so update check logic for `token_stream` existence.
toshimaru added a commit to toshimaru/sdoc that referenced this pull request Nov 17, 2023
Due to the change in ruby/rdoc#1055,
`token_stream` returns empty Array instead of `nil`,
so update check logic for `token_stream` existence.
@jonathanhefner
Copy link
Contributor

This seems like a breaking change, as demonstrated by rails/sdoc#346.

token_stream is part of the public API, and now returns a different value when the method source is unavailable.

It is easy to handle this change in SDoc, but I wanted to point it out.

toshimaru added a commit to toshimaru/rdoc that referenced this pull request Nov 19, 2023
The change in ruby#1055 might be a breaking change.
So, just simply return `token_stream.to_s` when it's nil
toshimaru added a commit to toshimaru/rdoc that referenced this pull request Nov 19, 2023
The change in ruby#1055 might be a breaking change.
So, just simply return `token_stream.to_s` when it's nil
toshimaru added a commit to toshimaru/rdoc that referenced this pull request Nov 21, 2023
The change in ruby#1055 might be a breaking change.
So, just simply wrap `token_stream` with `Array`

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
hsbt pushed a commit that referenced this pull request Dec 5, 2023
The change in #1055 might be a breaking change.
So, just simply wrap `token_stream` with `Array`

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants