-
Notifications
You must be signed in to change notification settings - Fork 280
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
Support Ruby 3.1 in Code Climate engine #1687
Comments
Given how Reek works, I think there won't be any issue with just using the parser for the latest Ruby syntax. |
@mvz Is the latest ruby parser always backwards compatible with older versions supported by Reek? If that's the case, then updating the Dockerfile's base image might just be enough! |
I am not aware of any syntax that is valid for older supported Rubies but considered a syntax error for Ruby 3.1. If there is any, then that could be an issue. |
I haven't found known cases of the 3.1 parser failing to parse older Ruby code, but after reading this section of the parser gem's README and knowing that this warning is shown whenever If this were to be done, my idea would be to keep the change in the # bin/code_climate_reek
module Reek
module Source
class SourceCode
# override self.default_parser method
def self.default_parser
parser_class.new(AST::Builder.new).tap do |parser|
diagnostics = parser.diagnostics
diagnostics.all_errors_are_fatal = true
diagnostics.ignore_warnings = true
end
end
# config.json file will look like this:
# {
# "include_paths":[
# "lib",
# "spec"
# ],
# "config": {
# "target_ruby_version": "3.1.0"
# }
# }
def self.parser_class
# convert an X.Y.Z version number to an XY two digit number
version_number = CodeClimateToReek.new.target_ruby_version&.gsub(/(\d+)\.(\d+)(\..+)?/, '\1\2').to_i
return Parser::CurrentRuby if version_number.zero?
Reek::CLI::Silencer.silently { require "parser/ruby#{version_number}" }
Module.const_get("Parser::Ruby#{version_number}")
end
rescue LoadError, NameError
# use Parser::CurrentRuby when an invalid version number is provided
Parser::CurrentRuby
end
end
end
end @mvz Of course, feel free to comment on this and let me know what you think, there possibly might be a better solution to this that I haven't seen or thought of. |
@dantevvp something like that seems fine. Until we decide to support such a setting in reek itself, @troessner any thoughts on this? |
Cool, many thanks for the responses! I opened a PR at #1694 |
@dantevvp Hola, Dante! Aslo it would be great if somebody will add ability to configure custom location of Reek configuration file: engines:
reek:
enabled: true
config:
file: linter_configs/.reek.yml I understend it's outside of scope of this issue, I have asked here: codeclimate/codeclimate#1078, but just silence... |
Since #1694 has been merged and released, I'm closing this issue. |
Hello from Code Climate! We're looking to update the
codeclimate-reek
engine to support parsing 3.1 syntax. Currently, theDockerfile
has ruby version2.6
installed, so theParser::CurrentRuby
always picksRuby26
as the parser, because that's the installed ruby version. This prevents reek from successfully reporting issues to Code Climate for files that include ruby 3.1 syntax.Since
reek
always uses the parser corresponding to the installed ruby version, using ruby 3.1 as the base image would be my first suggestion. However, for better compatibility, we could also make the engine use the correct parser for every code base, by allowing a custom configuration attribute in.codeclimate.yml
that lets the user set which ruby version they wantreek
to parse, like so:I'd love to read your thoughts on this. Thanks!
The text was updated successfully, but these errors were encountered: