Add Rubocop and run as part of the build #341
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I did this by running the following command:
rubocop --auto-gen-config
For some reason the following violations were not automatically excluded
by the automatically generated todo list:
lib/mocha/parameter_matchers/not.rb:25:9: C: Naming/MethodName: Use
snake_case for method names.
def Not(matcher)
^^^
test/acceptance/multiple_expectations_failure_message_test.rb:27:6: C:
Layout/IndentArray: Indent the right bracket the same as the start of
the line where the left bracket is.
], test_result.failure_message_lines
^
test/acceptance/multiple_expectations_failure_message_test.rb:66:6: C:
Layout/IndentArray: Indent the right bracket the same as the start of
the line where the left bracket is.
], test_result.failure_message_lines
^
I've disable the cop around monkey-patching code, because the whole point is that this is copy-pasted from the original source code.
Note that auto-correct didn't work, so I fixed this manually.
…correct I've disable the cop around monkey-patching code, because the whole point is that this is copy-pasted from the original source code.
The other violations result in a "Infinite loop detected" when running rubocop, so I'm going to come back to these.
I'm not keen on this cop, because it's easy to miss the while/until.
Using a mixture of auto-correct and manual edits - auto-correct didn't work very well in some of the tests which construct anonymous classes.
I disabled the cop around one block of code which I didn't think should be changed.
Also disable cop around tests using ExecutionPoint, because the tests rely on the instantiation of ExecutionPoint being on the same line as the following statement.
The `frozen_string_literal` magic comment has only been supported since Ruby v2.3 and I don't want to jeopardise support for earlier versions of Ruby.
This cop was only introduced relatively recently and it seems to have stirred up some controversy [1]. It was being triggered in many of the acceptance tests where anonymous classes are defined within a test and their methods' visibility is specified "inline". [1]: rubocop/rubocop#5953
I think the violations in the acceptance tests are all legitimate and they look like they might be related to this rubocop bug [1]. [1]: rubocop/rubocop#5201
Enabling this cop results in an "Infinite loop detected" exception. I can't see any related rubocop bugs, so we should probably investigate this a bit further and raise a rubocop issue about it.
Using the following command:
rubocop --auto-gen-config
Many of the longest lines are comment lines for YARD documentation. I think splitting the lines might change the formatting of the documentation, so I'm happy to allow them by setting the IgnoredPatterns option on the Metrics/LineLength cop. Note that this reduces the offense count from 735 to 501 and the maximum line length needed from 545 to 180. While the latter is still too big, this is a definite improvement.
It's not possible to set TargetRubyVersion to Ruby < v2.2 and the gemspec has `required_ruby_version` set to '>= 1.8.7'.
Ideally we'd set this to match the `required_ruby_version` in the gemspec, i.e. v1.8.7. However, we might as well set to to the earliest possible version in order to avoid triggering cops only intended to operate in more recent versions of Ruby. This means it's safe to no longer disable a couple of such cops in `.rubocop.yml`. I've confirmed this is OK by regenerating the todo list after making the change and seeing no changes to the todo list.
Now that we have set TargetRubyVersion to v2.2, the Style/NumericPredicate cop now only expects us to use Numeric#zero? which has been around since Ruby v1.8.6, and not Numeric#positive? or Numeric#negative? which have only been around since Ruby v2.3. So we can fix the violations and re-enable the cop.
Only if rubocop is available - it's not available for early versions of Ruby.
Ruby v2.2 is the TargetRubyVersion for rubocop.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most configuration has been removed from the to-do list, but there is still more work to do. However, this is a good start and should prevent things getting any worse.
Closes #265.