Ignores .rubycritic.yml file

[cvoltz]

I tried testing with the example .rubycritic.yml configuration file:

mode_ci:
  enabled: true # default is false
  branch: 'production' # default is master
branch: 'production' # default is master
path: '/tmp/mycustompath' # Set path where report will be saved (tmp/rubycritic by default)
threshhold_score: 10 # default is 0
deduplicate_symlinks: true # default is false
suppress_ratings: true # default is false
no_browser: true # default is false
format: console # Available values are: html, json, console, lint. Default value is html.
minimum_score: 95 # default is 0
paths: # Files to analyse.
  - 'app/controllers/'
  - 'app/models/'

Changes in the configuration file (e.g., no_browser, path, and paths), don't affect the settings rubycritic actually uses when it runs. It appears to always use the default settings (i.e., as if the .rubycritic.yml wasn't present) unless command line arguments are specified (then the CLI setting is used). When rubycritic runs, it does read the configuration file. It just appears to ignore the settings.

Also, it looks like the documented format option for the config file has changed to formats but the documentation hasn't been updated to match (see RubyCritic::Cli::Options::File::formats).

[Adre]

I think this is just a simple typo in your config caused by a typo in the example config from the README. I had some issues until I updated the threshold_score key.

I've submitted a pull request to address this: #315

[cvoltz]

It would be nice if RubyCritic would give an error message if the configuration file was incorrect, rather than just silently ignoring mistakes.

Fixing or removing the line doesn't get RubyCritic to use the other settings so the problem is more then just a single key being wrong.

I looked into the issue some more and it looks like format should be formats as well: https://github.com/whitesmith/rubycritic/blob/c22ce0f53156f47ae810717c3a5aa75c1168f47f/lib/rubycritic/cli/options/file.rb#L79

However, fixing the entry in the rubycritic.yml file then results in:

Traceback (most recent call last):
        8: from /home/cvoltz/.rvm/gems/ruby-2.6.3/bin/ruby_executable_hooks:24:in `<main>'
        7: from /home/cvoltz/.rvm/gems/ruby-2.6.3/bin/ruby_executable_hooks:24:in `eval'
        6: from /home/cvoltz/.rvm/gems/ruby-2.6.3/bin/rubycritic:23:in `<main>'
        5: from /home/cvoltz/.rvm/gems/ruby-2.6.3/bin/rubycritic:23:in `load'
        4: from /home/cvoltz/.rvm/gems/ruby-2.6.3/gems/rubycritic-4.1.0/bin/rubycritic:10:in `<top (required)>'
        3: from /home/cvoltz/.rvm/gems/ruby-2.6.3/gems/rubycritic-4.1.0/lib/rubycritic/cli/application.rb:19:in `execute'
        2: from /home/cvoltz/.rvm/gems/ruby-2.6.3/gems/rubycritic-4.1.0/lib/rubycritic/cli/options.rb:24:in `to_h'
        1: from /home/cvoltz/.rvm/gems/ruby-2.6.3/gems/rubycritic-4.1.0/lib/rubycritic/cli/options/file.rb:25:in `to_h'
/home/cvoltz/.rvm/gems/ruby-2.6.3/gems/rubycritic-4.1.0/lib/rubycritic/cli/options/file.rb:80:in `formats': private method `select' called for "console":String (NoMethodError)

That problem is caused by the code assuming formats will always be an array: https://github.com/whitesmith/rubycritic/blob/987d14223348da9fb8e39212e831075eb476c80b/lib/rubycritic/cli/options/file.rb#L80-L82

So either the sample configuration file needs to show formats as an array:

formats:
  - console

or RubyCritic::Cli::Options::File#formats needs to be modified to ensure formats is always an array:

        def formats
-         formats = options['formats'] || []
+         formats = Array(options['formats'])
          formats.select do |format|
            %w[html json console lint].include?(format)
          end
        end

Unfortunately, even that is not enough to fix this problem because there is also a bug in RubyCritic::Cli::Options#to_h when it merges the hash of options it has read from the configuration file with those specified on the command line: https://github.com/whitesmith/rubycritic/blob/a6ddc92235adb2f4353dfec1286850ebbe48a800/lib/rubycritic/cli/options.rb#L23-L30 argv_option[:formats] is an empty array when it isn't specified on the CLI but any empty array isn't nil so the empty array from the CLI options (argv_option) overwrites the setting in the configuration file (file_hash) even when it is specified. This can be fixed with an extra check to see if the argument is an empty array:

      def to_h
        file_hash = file_options.to_h
        argv_hash = argv_options.to_h

        file_hash.merge(argv_hash) do |_, file_option, argv_option|
-         argv_option.nil? ? file_option : argv_option
+         Array(argv_option).empty? ? file_option : argv_option
        end
      end

Note that argv_option can be an Array, a Float, an Integer, or a String. Casting it to an Array allows a simple check for whether the argument is nil or an empty array without having to check its type.

Finally, it appears that the formats must be symbols in the array, rather then strings. If we look at RubyCritic::CLI::Options::Argv#parse we see that it always returns a symbol: https://github.com/whitesmith/rubycritic/blob/723c26f7d5105dffd6a2b75b5c289b1a37127507/lib/rubycritic/cli/options/argv.rb#L40-L51

So, we need to make another change to RubyCritic::Cli::Options::File#formats to get it to also return a symbol:

        def formats
          formats = Array(options['formats'])
          formats.select do |format|
            %w[html json console lint].include?(format)
-         end
+         end.map(&:to_sym)
        end

Created pull request #316 to fix the problems.