-
-
Notifications
You must be signed in to change notification settings - Fork 400
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 array-within-word-array parsing #1604
base: main
Are you sure you want to change the base?
Conversation
Hmmm, it looks like I'll work through the |
88e7395
to
f8e75b9
Compare
All right, added simplified regression test cases, found a different fix which seems more robust, and verified it passed tests as well as sord on my application. sord is useful for regression against a codebase here as it uses the output of yard's parse of e.g. constants and feeds the results into sorbet, which parses it again, complaining about syntax errors when yard doesn't give the correct source range. |
lib/yard/parser/ruby/ruby_parser.rb
Outdated
sstart = other.source_range.begin | ||
lstart = other.line_range.begin | ||
node.source_range = Range.new(sstart, @ns_charno - 1) | ||
node.line_range = Range.new(lstart, lineno) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm missing something, this is dead code. Let me know if I'm wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not dead code per se. This is meant to future-proof the implementation for new node types which happens fairly often.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. Just seemed strange those two assignments were immediately overridden. I'll revert that, but definitely curious to hear what I'm misunderstanding there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted
When yard saw a constant like: ```ruby FOO = [%w[bar baz]] ``` it cut off the array at the first closing bracket: ```ruby [%w[bar baz] ``` Fix by ensuring that these types of arrays, which don't have a starting :lbracket, don't pop off :lbracket state in @Map. Drop dead code and add a variety of related regression cases found in my application using sord after trying different fixes.
f8e75b9
to
a5dcc55
Compare
Description
When yard saw a constant like:
it cut off the array at the first closing bracket in the resulting documentation:
Fix by ensuring that these types of arrays, which don't have a
starting :lbracket, don't pop off :lbracket state in
@map
Drop dead code and add a variety of related regression cases found in
my application using sord after trying different fixes.
Completed Tasks
bundle exec rake
locally (if code is attached to PR).