Parsing of percent array source - see #1005

[MSP-Greg]

YARD does not parse some constant or class variables 'values' correctly.

Steps to reproduce

Following code shows the issue, which originate in the parser. Last case shown is the only one that fails.

require 'yard'

parser = YARD::Parser::Ruby::RubyParser

puts "\n------------ TEST STRING ------------   - qsymbols_literal -"

s = parser.parse("TEST = %i[\n Does \n this \n work]", 'test').ast
puts 'TEST = %i[\n Does \n this \n work]      ' +
    s.jump(:qsymbols_literal).source.gsub(/\s+/, ' ')

s = parser.parse("TEST = %i[\n Does \n this \n work  ]", 'test').ast
puts 'TEST = %i[\n Does \n this \n work   ]   ' +
    s.jump(:qsymbols_literal).source.gsub(/\s+/, ' ')

s = parser.parse("TEST = %i[\n Does \n this \n work \n]", 'test').ast
puts 'TEST = %i[\n Does \n this \n work \n]   ' +
    s.jump(:qsymbols_literal).source.gsub(/\s+/, ' ')

puts

s = parser.parse("TEST = %i[\n Does \n this \n work\n ]", 'test').ast
puts 'TEST = %i[\n Does \n this \n work\n ]   ' +
    s.jump(:qsymbols_literal).source.gsub(/\s+/, ' ')```

Note that the only case which fails is a constant/cvar formatted as below

class Test
  Const = %W[
    does
    not
    parse
  ]
end

Critical issue is that the closing character must be on a new line and be indented.

The PR I spoke of earlier operated in the handlers. You may be able to fix the issue in the parser now that it's clear exactly what fails.

Environment details:

• OS: [Windows]
• Ruby version: [2.3.2]
• YARD version: [0.9.5+]

I have read the Contributing Guide.

[lsegal]

At first glance this seems related to #894.

[MSP-Greg]

I'm definitely not a parser expert. This issue seems related to the parser not having a good/complete set of rules for how white space (and what type of white space) affects various statements that can span multiple lines.

" " and " \n" work, but "\n " doesn't? Really, that's the issue here. #894 doesn't seem to be connected to white space handling.

Re my previously mentioned pr, I decided to address the issue in the handlers, and I saw this issue affecting constants and cvars, in that both have a 'value' that is a portion of a NamespaceObject's source.

For percent arrays, turns out that the source ends with white space if it is incorrectly parsed. I just incremented the range until I hit a non white space character.

[MSP-Greg]

Update, just checked "\n\n". It works.

[MSP-Greg]

Well, I went back to the parser and fixed the issue there, rather than in the handlers.

I just posted a file at --

https://github.com/MSP-Greg/MSP-Greg.github.io/blob/master/test/bench_ripper_lit.rb

It shows the update to the parser, runs it and the current parser thru benchmark (similar to your freeze_tree test file). I found no significant difference in parse time.

It also runs a very complete test of a large combination of percent array formatting styles, and shows which ones fail. Similar to the 'BUILT_IN' constants PR, it loads a string (instead of an array) with all the failures. Current code fails on any percent arrays ending in '\n ]' or any other terminator.

Line 273 of spec/parser/ruby/ruby_parser_spec.rb has one test that could be removed if replaced with similar code. There's also a few similar tests after that one.

Just wondering what tests to remove, or, should I do a PR with a new test based on the above code and you can remove what you want?

[lsegal]

Let's keep all existing tests. If you want to add a new suite of tests I can take a look in the PR. Thanks!

[lsegal]

Oh, also, it might be worth checking if this resolves the related #894 issue. It seems like a similar off-by-1.

[lsegal]

Sidenote (sorry for bunch of messages): make sure to rebase off of the master branch. I added rubocop linting to the build yesterday which should enforce some style rules. rake runs these by default now, so you should be able to check before pushing, which should make for smoother review (and rubocop -a should fix most of the style things automatically).

[MSP-Greg]

Done. I'm freezing.

Added one big test with a fail message. You might run tests with

/lib/yard/parser/ruby/ruby_parser.rb, line 439
@percent_ary = node

commented out, as that disables the patch, then you'll see the error message.

Since I've obviously gotten familiar with the parser, I'll have a look at #894.

[lsegal]

Closing out some old issues. Given the significant age of this, it would be best to re-open with new context if it's still relevant.