-
-
Notifications
You must be signed in to change notification settings - Fork 397
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
Add support for quoted symbols #1168
Conversation
Without the fix, the new test "properly tokenizes symbols" would fail with: ``` 1) YARD::Parser::Ruby::RubyParser#parse properly tokenizes symbols Failure/Error: expect(symbols).to eq %w( :'' :BAR :"B+z" ) expected: [":''", ":BAR", ":\"B+z\""] got: [":'", ":BAR", ":B+z"] (compared using ==) ``` which shows that symbols were indeed not reconstructed properly.
Constants containing quoted symbols can now be documented correctly. Prior to the fix, the value `:"sym+bol"` would be shown as `sym+bol` and worse, the empty symbol (`:''`) would have triggered an error in MethodHelper#format_constant.
Sorry, I did run 'rake rubocop' and 'rake spec', but forgot to run 'rake yard': now the latter fails. I'll investigate and provide a follow-up. |
The sequence `:symbeg`, `:ident` (corresponding to symbols like `:ident`) also needs to be taken into account.
Btw, as both the contributing guide and the PR template advise to do a |
Thank you for the PR. I agree that running yard as part of the rake might be worthwhile, but I think we would need some kind of way to validate the results. Without validating, I'm not sure how this would work. One thing we could do is integration test the |
PR #1171 added a smoke test to run yard on yard's source code during CI. |
Description
Constants containing quoted symbols can now be documented correctly.
Prior to the fix, the value
:"sym+bol"
would be shown assym+bol
and worse, the empty symbol (
:''
) would have triggered an error inMethodHelper#format_constant
, as the corresponding constant valuewas the empty String.
For example, this code:
would trigger the following error in YARD 0.9.12:
The
ConstantHandler
didn't treat the:dyna_symbol
node specially, andsimply calling the source method on that statement would return the characters
making up the symbol, not the actual source code for that symbol (i.e. the prefix
:
and the wrapping quotes).
This is fixed in commit 5b1dc8a.
Besides, when investigating the issue, I first followed a wrong track by fixing
an error in the tokenisation code of the parser. This proved to be not
necessary to fix my original error, but I think the change will not harm
so I've left it in the PR as a separate commit (db00597).
Some more details about this other fix: in RubyParser,
a sequence of tokens gets built, and a
[:symbeg, :const]
pairgets replaced by a
:symbol
token.However, there are actually three possibilities (maybe more?)
for constructing a Symbol:
:symbeg
,:const
(e.g.:BAR
):symbeg
,:tstring_content
,:tstring_end
(e.g.:"B+z"
):symbeg
,:tstring_end
(e.g.:''
)So I added support for case 2. and 3. as well.
Completed Tasks
bundle exec rake
locally (if code is attached to PR).