troessner / reek

Code smell detector for Ruby
https://github.com/troessner/reek
MIT License
4k stars 279 forks source link

Code Climate: add config "target_ruby_version" to support different versions of Ruby syntax #1694

Closed dantevvp closed 1 year ago

dantevvp commented 1 year ago

Related issue: #1687

Currently, Reek uses Parser::CurrentRuby to determine which Ruby version to parse when analyzing code, so it always chooses the installed Ruby version's parser. Under the Code Climate engine, this means that it will always choose the parser for Ruby version 2.6, because that's the version that is installed in the Dockerfile.

To let users choose what Ruby version they want to use reek under, expose a new config attribute "target_ruby_version". This overrides the Reek::Source::SourceCode.default_parser method to use whichever parser version the user specifies.

Example

With .codeclimate.yml looking like this:

engines:
  reek:
    enabled: true
    config:
      target_ruby_version: "2.6.8"

The logic will use the first two digits of the chosen version to determine which parser version to use. In this case, it would be Parser::Ruby26

Another example:

engines:
  reek:
    enabled: true
    config:
      target_ruby_version: "3.1"

In this case, we will choose Parser::Ruby31 as the parser, which supports newer syntax.

If the user specifies an unknown version or no version at all, it will default to Parser::CurrentRuby, which is the previous behavior.

mvz commented 1 year ago

Thanks, @dantevvp, I'll try to do a review this weekend.

mvz commented 1 year ago

@dantevvp I'm assuming you will test that this works in the context of Code Climate yourself :slightly_smiling_face:

dantevvp commented 1 year ago

@dantevvp I'm assuming you will test that this works in the context of Code Climate yourself 🙂

Correct! I've tested this with 3.1 syntax and old syntax. It defaults to Parser::CurrentRuby when an invalid/null version number is specified, and it uses the correct specified parser each time.

troessner commented 1 year ago

Looks good to me! Can you please squash to one commit @dantevvp and then.. lets release a new version @mvz ?

mvz commented 1 year ago

I'm not sure if we need to release a new version. Doesn't Code Climate run reek straight from the repository using the Dockerfile? @dantevvp would know this ...

dantevvp commented 1 year ago

@troessner @mvz Commits squashed and Rubucop offense resolved!

I'm not sure if we need to release a new version. Doesn't Code Climate run reek straight from the repository using the Dockerfile? @dantevvp would know this ...

This is correct! We use the commit sha of the repository so a version release would not be necessary to get the latest Reek code.

mvz commented 1 year ago

Thanks, @dantevvp!