lint: Remove unreachable code

[okuramasafumi]

This is an attempt to utilize RuboCop further. RuboCop was added in 9262fdd43a3a87403dc095e096f968576cd611d9 but only a few rules have been enabled. I believe we can utilize RuboCop more for better code quality, especially with Lint cops. This is the first step to enable other Lint cops. This commit also exclude some auto generated files.

[okuramasafumi]

@zenspider Thank you for pointing out. It's weird because bundle exec rubocop is successful and bundle exec rake rubocop fails. Also, only ubuntu-latest fails in CI, but in my local Mac environment it also fails with bundle exec rake rubocop.

[zenspider]

I was curious why it was only failing in one env...

Your difference is even more curious. What does the rake version do differently?

[st0012]

I was curious why it was only failing in one env...

Because the job will only run on ubuntu with Ruby head. In terms of difference between rubocop and rake rubocop, it could be caused by this extra config in Rakefile.

[tompng]

RuboCop is only used to format and autocorrect generated files. In test.yml, rake rubocop is confirming whether generated files are formatted correctly.

parsed_files = [
  "lib/rdoc/rd/block_parser.rb",
  "lib/rdoc/rd/inline_parser.rb",
  "lib/rdoc/markdown.rb",
  "lib/rdoc/markdown/literals.rb"
]

# Rakefile L108
RuboCop::RakeTask.new(:rubocop) do |t|
  t.options = [*parsed_files]
end
task :build => [:generate, "rubocop:autocorrect"]

This means, rubocop.yml can't contain un-auto-correctable cops. Also, rake rubocop only checks these generated files, not hand-written code in rdoc. Maybe we can add another test task that checks non-generated files with two rubocop.yml files, one for auto-correcting generated files and another one for human.

[okuramasafumi]

@tompng Thank you for clarifying it, now I can understand what is going on here.

First of all, we usually run rubocop or bundle exec rubocop to run RuboCop, described as https://docs.rubocop.org/rubocop/1.64/usage/basic_usage.html This bundle exec rake rubocop is special since it's specialized to auto generated files. For DX, enabling bundle exec rubocop to work correctly is important, while keeping CI work is not so important in terms of human interaction.

Therefore, I'd like to suggest creating another rubocop config file such as .ci.rubocop.yml and using it from rake task. This enables to keep running rubocop as simple as possible and doesn't break CI. Also we can point that .rubocop.yml is the file that we tend to change, while it's not safe now.

I'd like to hear your opinions!

[st0012]

• We should rename the current rubocop task to something more explicit, like format_generated_files
• The format_generated_files task should use its own rubocop config with content from the current .rubocop.yml
• $ rubocop should be what developers use for maintaining this project's coding style.
  • $ rake rubocop is probably not needed at the moment
• Linting check should be run as a standalone GH Actions job, not a step under the test job

I will try to implement the above changes in a separate PR. And then we can add more rules to the new .rubocop.yml as we see fit.

That said, I'd really like to see consistent rules across ruby/* projects that already use rubocop, like irb, reline...etc. So if you want to test an effectiveness of a cop, I'd recommend adding it to irb, which already adopted a few. And later we sync its rules to RDoc periodically.

[st0012]

I've merged #1139 that implemented the above changes.

[okuramasafumi]

@st0012 Thank you, rebased.

Do we need to run bundle exec rubocop in CI? I run it locally and found some warnings.

[st0012]

It’s already on CI. And it’ll be very helpful if you can also post what warnings you got.

[okuramasafumi]

The output of bundle exec rubocop is below:

Inspecting 205 files
..............................................C.C...............................................W............................................................................................................

Offenses:

lib/rdoc/markdown.rb:198:36: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
      @memoizations = Hash.new { |h,k| h[k] = {} }
                                   ^
lib/rdoc/markdown.rb:263:26: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
      chr = string[target,1]
                         ^
lib/rdoc/markdown.rb:300:32: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
        "#{@pos} (\"#{@string[0,@pos]}\" @ \"#{@string[@pos,width]}\")"
                               ^
lib/rdoc/markdown.rb:300:60: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
        "#{@pos} (\"#{@string[0,@pos]}\" @ \"#{@string[@pos,width]}\")"
                                                           ^
lib/rdoc/markdown.rb:302:77: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
        "#{@pos} (\"... #{@string[@pos - width, width]}\" @ \"#{@string[@pos,width]}\")"
                                                                            ^
lib/rdoc/markdown.rb:375:21: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
      if @string[pos,len] == str
                    ^
lib/rdoc/markdown.rb:423:31: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
        method = rule.gsub("-","_hyphen_")
                              ^
lib/rdoc/markdown/literals.rb:27:36: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
      @memoizations = Hash.new { |h,k| h[k] = {} }
                                   ^
lib/rdoc/markdown/literals.rb:92:26: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
      chr = string[target,1]
                         ^
lib/rdoc/markdown/literals.rb:129:32: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
        "#{@pos} (\"#{@string[0,@pos]}\" @ \"#{@string[@pos,width]}\")"
                               ^
lib/rdoc/markdown/literals.rb:129:60: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
        "#{@pos} (\"#{@string[0,@pos]}\" @ \"#{@string[@pos,width]}\")"
                                                           ^
lib/rdoc/markdown/literals.rb:131:77: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
        "#{@pos} (\"... #{@string[@pos - width, width]}\" @ \"#{@string[@pos,width]}\")"
                                                                            ^
lib/rdoc/markdown/literals.rb:204:21: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
      if @string[pos,len] == str
                    ^
lib/rdoc/markdown/literals.rb:252:31: C: [Correctable] Layout/SpaceAfterComma: Space missing after comma.
        method = rule.gsub("-","_hyphen_")
                              ^
lib/rdoc/rd/block_parser.rb:1337:5: W: Lint/UnreachableCode: Unreachable code detected.
    result
    ^^^^^^

205 files inspected, 15 offenses detected, 14 offenses autocorrectable