reviewdog / action-reek

Run reek with reviewdog 🐶
MIT License
10 stars 6 forks source link

Syntax errors on ruby 2.7.2 #15

Closed invke closed 3 years ago

invke commented 3 years ago

When running the latest of reviewdog/action-rubocop@v1, against a project running ruby 2.7.1 I experienced an underlying Parser::SyntaxError. The issue seems to be able to be replicated locally by downgrading the ruby version to 2.6.5, which this action uses and running reek against it. I attempted to update the ruby version to 2.7.2 and the same issue persisted.

Here is the error/trace that is thrown:

        !!!
        Source 'app/query_builders/client_check_in_query_builder.rb' cannot be processed by Reek due to a syntax error in the source file.

        This is a problem that is outside of Reek's scope and should be fixed by you, the
        user, in order for Reek being able to continue.

        Things you can try:
        - Check the syntax of the problematic file
        - If the file is not in fact a Ruby file, exclude it in your .reek.yml file

        Exception message:

        #<Parser::SyntaxError: unexpected token tDOT>

        Original backtrace:

        /usr/local/bundle/gems/parser-2.7.2.0/lib/parser/diagnostic/engine.rb:72:in `process'
    /usr/local/bundle/gems/parser-2.7.2.0/lib/parser/base.rb:285:in `on_error'
    /usr/local/lib/ruby/2.6.0/racc/parser.rb:259:in `_racc_do_parse_c'
    /usr/local/lib/ruby/2.6.0/racc/parser.rb:259:in `do_parse'
    /usr/local/bundle/gems/parser-2.7.2.0/lib/parser/base.rb:189:in `parse'
    /usr/local/bundle/gems/parser-2.7.2.0/lib/parser/base.rb:206:in `parse_with_comments'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/source/source_code.rb:117:in `parse'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/source/source_code.rb:53:in `syntax_tree'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/examiner.rb:116:in `syntax_tree'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/examiner.rb:120:in `examine_tree'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/examiner.rb:94:in `block in run'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/examiner.rb:104:in `wrap_exceptions'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/examiner.rb:93:in `run'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/examiner.rb:61:in `smells'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/examiner.rb:69:in `smells_count'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/report/base_report.rb:44:in `add_examiner'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/report/text_report.rb:23:in `add_examiner'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/cli/command/report_command.rb:26:in `block in populate_reporter_with_smells'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/cli/command/report_command.rb:25:in `each'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/cli/command/report_command.rb:25:in `populate_reporter_with_smells'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/cli/command/report_command.rb:17:in `execute'
    /usr/local/bundle/gems/reek-6.0.2/lib/reek/cli/application.rb:32:in `execute'
    /usr/local/bundle/gems/reek-6.0.2/bin/reek:12:in `<top (required)>'
    /usr/local/bundle/bin/reek:23:in `load'
    /usr/local/bundle/bin/reek:23:in `<main>'

        !!!

It fails on the following code, particularly on the comment line that is between multi-lined method calls (# perform the averaging in sql to make fast). Although it's the parser that errors, it seems to be related to running it on the ruby 2.6 docker image.

# frozen_string_literal: true

class ClientCheckInQueryBuilder < ApplicationQueryBuilder

  # omitting unrelated code for brevity

  query :answers_average_over do |questions, threshold|
    relation
      .merge(all.answered_all_of(questions).relation)
      # perform the averaging in sql to make fast
      .where(
        <<~SQL.squish
          SELECT ((#{questions.join(' + ')}) / #{questions.count}) >= #{threshold}
        SQL
      )
  end
end

This is a very specific issue (with a slightly strange comment), so I understand if it isn't high priority. I've moved the comment to fix it for now but figured it'd be good to capture this incase other people are experiencing it.

Cheers.

mgrachev commented 3 years ago

@invke 👋 Is there this problem if you run reek locally?

invke commented 3 years ago

Hey @mgrachev, sorry I didn't explain the local test properly in the description. It works locally when running the same ruby version that I'm using in the project (2.7.2), but when we swapped to the same major ruby version that the docker image is using to run reek it gave the same error. I think we ran locally with 2.6.5, so that may not be the same minor version as the docker image.

mgrachev commented 3 years ago

Can you test these changes https://github.com/reviewdog/action-reek/pull/16?

name: reviewdog
on: [pull_request]
jobs:
  reek:
    name: runner / reek
    runs-on: ubuntu-latest
    steps:
      - name: Check out code
        uses: actions/checkout@v1
      - name: reek
        uses: reviewdog/action-reek@migrate-to-composite
        with:
          github_token: ${{ secrets.github_token }}
invke commented 3 years ago

Hey, I encountered an error when running with that tag version of the action. Here's the trace:

Run reviewdog/action-reek@migrate-to-composite
  with:
    github_token: ***
    tool_name: reek
    level: error
    reporter: github-pr-check
    filter_mode: added
    fail_on_error: false
🐶 Installing reviewdog ... https://github.com/reviewdog/reviewdog
  reviewdog/reviewdog info checking GitHub for tag 'v0.11.0'
  reviewdog/reviewdog info found version: 0.11.0 for v0.11.0/Linux/x86_64
  reviewdog/reviewdog info installed /tmp/tmp.0NZmGlvfnC/reviewdog
/home/runner/_work/_actions/reviewdog/action-reek/migrate-to-composite/script.sh: 20: [[: not found
 Installing reek ... https://github.com/troessner/reek
  ERROR:  While executing gem ... (Gem::Requirement::BadRequirementError)
      Illformed requirement ["***"]
 Running reek with reviewdog 🐶 ...
  Error: No such file - ***
  2020/12/14 19:54:16 [reek] reported: https://github.com/boost/fincap-client-voices/runs/1552725916 (conclusion=success)
mgrachev commented 3 years ago

I'm sorry for the long response. I have fixed this error. Can you try it again?

invke commented 3 years ago

Sorry @mgrachev for the slow response also, just got back from leave today. Yep, it's all sorted on the latest of v1. Thanks, heaps!