Skip to content
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

Investigate how the rubocop warning are working #49

Closed
DanielePalombo opened this issue Jul 24, 2023 · 5 comments
Closed

Investigate how the rubocop warning are working #49

DanielePalombo opened this issue Jul 24, 2023 · 5 comments
Assignees

Comments

@DanielePalombo
Copy link
Collaborator

If possible support it and update the readme and documentation as well

@abhishekgupta5
Copy link
Contributor

abhishekgupta5 commented Jul 24, 2023

From the rubocop docs

Severity

Each cop has a default severity level based on which department it belongs to. The level is normally warning for Lint and convention for all the others, but this can be changed in user configuration. Cops can customize their severity level. Allowed values are info, refactor, convention, warning, error and fatal.

Cops with severity info will be reported but will not cause rubocop to return a non-zero value.

There is one exception from the general rule above and that is Lint/Syntax, a special cop that checks for syntax errors before the other cops are invoked. It cannot be disabled and its severity (fatal) cannot be changed in configuration.

Lint:
Severity: error

Metrics/CyclomaticComplexity:
Severity: warning

@piyushswain
Copy link
Contributor

piyushswain commented Jul 31, 2023

@DanielePalombo @MassimilianoLattanzio @abhishekgupta5
From the Rubocop Docs, Rubydoc.info and checking the source code

Hey guys,
I have found a way to set a severity on a cop offense and also to test this behaviour in the specs.


Modifications required in a Cop

Lets take this cop for example.

class ClassEvalDecorator < Base
  MSG = 'Do not use `class_eval` flag. Use a decorator module instead. Check this link for an example https://guides.solidus.io/cookbook/redefining-checkout-steps'

  RESTRICT_ON_SEND = %i[class_eval].freeze

  def_node_matcher :on_class_eval?, <<~PATTERN
    (send  ($...) :class_eval)
  PATTERN

  def on_send(node)
    return unless on_class_eval?(node)

    add_offense(node)
  end
end

Now if we want to modify this offense from a error to warning then we need to modify the following line

add_offense(node, severity: :warning)

The above line modifies the severity of the RuboCop::Cop::Offense object returned by the add_offense method.

Returned Object (For Default Offense)

[#<RuboCop::Cop::Offense:0x000000011e1de830
  @cop_name="Solidus/ClassEvalDecorator",
  @corrector=nil,
  @location=#<Parser::Source::Range (string) 0...18>,
  @message="Do not use `class_eval` flag. Use a decorator module instead. Check this link for an example https://guides.solidus.io/cookbook/redefining-checkout-steps",
  @severity=#<RuboCop::Cop::Severity:0x000000011e1de808 @name=:convention>]

Returned Object (For Warning Level Offense)

[#<RuboCop::Cop::Offense:0x000000011cda1250
  @cop_name="Solidus/ClassEvalDecorator",
  @corrector=nil,
  @location=#<Parser::Source::Range (string) 0...18>,
  @message="Do not use `class_eval` flag. Use a decorator module instead. Check this link for an example https://guides.solidus.io/cookbook/redefining-checkout-steps",
  @severity=#<RuboCop::Cop::Severity:0x000000011cda1228 @name=:warning>]

Now, onto RSpec
I checked the expect_offense method of rspec and it had an argument for severity of the offense as well.
So, we can test our offenses in the following way.

Older Tests

expect_offense(<<~RUBY)
      Product.class_eval do
      ^^^^^^^^^^^^^^^^^^ Do not use `class_eval` flag. Use a decorator module instead. Check this link for an example https://guides.solidus.io/cookbook/redefining-checkout-steps\n
      end
RUBY

New Tests with severity warning

expect_offense(<<~RUBY, severity: :warning)
      Product.class_eval do
      ^^^^^^^^^^^^^^^^^^ Do not use `class_eval` flag. Use a decorator module instead. Check this link for an example https://guides.solidus.io/cookbook/redefining-checkout-steps\n
      end

RUBY

When the severity level does not match in a test then we get the following error.

Failure/Error:
       expected: [:warning]
       got: [#<RuboCop::Cop::Severity:0x000000011e1de808 @name=:convention>]

One thing to keep in mind is the heirarchy of severity in rubocop
Rubocop has multiple severity levels for a cop

  • [info refactor convention warning error fatal]

I have noticed that the expect_offense method works in a way where the higher severity will always catch an offense with a lower severity and pass it through.

  • Example 1 - If my offense has a severity of refactor but the expect_offense method checks for warning severity then this example will pass.
  • Example 2 - If my offense has a severity of error but the expect_offense method checks for warning severity then this example will cause a failure in the test

@MassimilianoLattanzio
Copy link
Collaborator

Thanks for the investigation @piyushswain! Do we need to modify some severities in our current cops?

@abhishekgupta5
Copy link
Contributor

Good find @piyushswain. I think we can use this severity concept to be lenient towards some of the cops where we are not 100% sure (as we have seen plenty till now)

@piyushswain
Copy link
Contributor

@MassimilianoLattanzio I'm not sure, we should probably discuss which cops we would like to set a severity for in a donut probably or if you have some cops in mind we can start changing them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants