Skip to content

Commit

Permalink
rubocop: Fixed many previously-ignored cop offenses (cloudfoundry#653).
Browse files Browse the repository at this point in the history
NOTE: All of the fixed/excluded offenses were resolved incrementally (one cop at a time, in the order indicated below), by either auto-fixing the manually re-enabled offenses or else explicitly disabling/excluding (inline or in the .rubocop.yml, instead of in .rubocop_todo.yml) the previously-ignored offenses.
NOTE: After each incremental change (whether auto-fixed by rubocop or manually fixed/resolved), the incremental code changes were re-reviewed (for both correctness and readability), and the unit test suite was re-executed (to verify no regressions were introduced). Where either the code review and/or unit tests indicated that the auto-fixed changes were unacceptable, the auto-fixed changes were discarded and manual fixes/resolution were applied instead.
NOTE: Most of the fixed offenses were auto-fixed (via `rubocop -x`, `rubocop -a`, or `rubocop -A`, as appropriate).
SQUASHED COMMIT HISTORY:
---
rubocop: updated .rubocop_todo.yml (via `rubocop --regenerate-todo`)
rubocop: Inline-disabled a `Style/ExplicitBlockArgument` offense (plus added code comments).
NOTE: The offending method seems like a candidate for more/different refactoring beyond a fix for the `Style/ExplicitBlockArgument` offense.
rubocop: Excluded 9 offenses: Style/NumericPredicate.
NOTE: The `Style/NumericPredicate` cop was reporting 9 auto-correctable offenses (for the modified lines). But auto-fixing the offenses caused failing specs, so those 9 cop offenses have been disabled inline.
rubocop: Manually fixed: Style/IfUnlessModifier.
NOTE: The 'Cop supports --auto-correct' auto-fixes resulted in harder-to-read code than before, warranting manual refactoring instead.
rubocop: Force-auto-fixed: Style/StringChars.
NOTE: This auto-fix required the use of the `rubocop --auto-correct-all` command.
   -A, --auto-correct-all           Auto-correct offenses (safe and unsafe)
rubocop: Auto-fixed: Style/QuotedSymbols.
rubocop: Auto-fixed: RSpec/Yield.
rubocop: Auto-fixed: RSpec/ReceiveCounts.
rubocop: Auto-fixed: RSpec/NotToNot.
rubocop: Auto-fixed: RSpec/HookArgument.
rubocop: Auto-fixed: RSpec/ExcessiveDocstringSpacing.
rubocop: Auto-fixed: RSpec/EmptyLineAfterHook.
rubocop: Auto-fixed: RSpec/EmptyLineAfterFinalLet.
rubocop: Auto-fixed: RSpec/EmptyLineAfterExampleGroup.
rubocop: Auto-fixed: RSpec/ContextMethod.
rubocop: Auto-fixed: Lint/SymbolConversion.
rubocop: Auto-fixed: Style/StringLiterals.
rubocop: Auto-fixed: Style/SoleNestedConditional.
NOTE: A few of the auto-corrected changes could potentially be simplified further by refactoring them to use the Hash#dig method instead of chained Hash#fetch calls (i.e. chained square brackets).
rubocop: Auto-fixed: Style/SlicingWithRange.
rubocop: Auto-fixed: Style/RedundantRegexpEscape.
rubocop: Auto-fixed: Style/RedundantFileExtensionInRequire.
rubocop: Auto-fixed: Style/RedundantBegin.
rubocop: Auto-fixed: Style/RedundantAssignment.
rubocop: Auto-fixed: Style/PercentLiteralDelimiters.
rubocop: Auto-fixed: Style/NestedTernaryOperator.
rubocop: Auto-fixed: Style/IfInsideElse.
rubocop: Auto-fixed: Style/HashSyntax.
rubocop: Auto-fixed: Style/HashEachMethods.
rubocop: Auto-fixed: Style/HashConversion.
rubocop: Auto-fixed: Style/FrozenStringLiteralComment.
rubocop: Auto-fixed: Style/EmptyLiteral.
rubocop: Auto-fixed: Style/CommentAnnotation.
rubocop: Auto-fixed: Style/ClassEqualityComparison.
rubocop: Auto-fixed: Style/CaseLikeIf.
NOTE: The auto-fix also required that the Metrics/ModuleLength cop's Max (in .rubocop_todo.yml) be updated, to keep that cop from causing a new offense.
rubocop: Auto-fixed: Style/BisectedAttrAccessor.
NOTE: These are the remaining (non-excluded) offenses of this cop, and exclude the offenses which were manually excluded by an earlier commit.
rubocop: Auto-fixed: Lint/AmbiguousRegexpLiteral.
rubocop: Auto-fixed: Lint/AmbiguousOperatorPrecedence.
rubocop: Auto-fixed: Lint/AmbiguousOperator.
rubocop: Excluded 2 offenses: Style/HashTransformValues.
NOTE: The offenses were permanently excluded (in '.rubocop.yml' instead of '.rubocop_todo.yml'), since fixing them would be backwards-incompatible with Ruby < v2.4, and it seems like the file containing the offenses _might_ need to be backwards-compatible with earlier versions.
rubocop: Manually fixed 2 offenses: Lint/ToJSON.
NOTE: The 'Cop supports --auto-correct' auto-fixes resulted in incorrect fixes for these offenses, requiring manual fixing instead.
rubocop: Manually fixed 1 offense: Lint/RaiseException.
NOTE: The offense was a false-positive. In this particular case, we must violate the cop's rule. So the offense is now disabled via an inline rubocop comment.
rubocop: Manually fixed 17 offenses: Lint/AmbiguousBlockAssociation.
rubocop: Manually fixed 1 cop offense: Lint/AmbiguousBlockAssociation.
Fix: set_disk_metadata_spec.rb: Fixed a broken spec.
NOTE: This spec previously passed (despite an incorrect expectation) due to incorrect syntax. The incorrect syntax was one of the (previously-ignored) 'Lint/AmbiguousBlockAssociation' rubocop offenses.
rubocop: Auto-fixed: Layout/TrailingEmptyLines.
rubocop: Auto-fixed: Layout/SpaceInsideHashLiteralBraces.
rubocop: Auto-fixed: Layout/SpaceInsideArrayLiteralBrackets.
rubocop: Auto-fixed: Layout/MultilineMethodCallIndentation.
NOTE: The default `aligned` style is the closest match to the existing code.
rubocop: Auto-fixed: Layout/LineEndStringConcatenationIndentation.
rubocop: Auto-fixed: Layout/FirstHashElementIndentation.
rubocop: Auto-fixed: Layout/FirstArgumentIndentation.
rubocop: Auto-fixed: Layout/ClosingParenthesisIndentation.
rubocop: Auto-fixed 2 cop offenses: Layout/ExtraSpacing, Layout/SpaceAroundOperators.
rubocop: Auto-fixed 2 cop offenses: Layout/EmptyLinesAroundBlockBody.
rubocop: Auto-fixed 1 cop offenses: Layout/EmptyLineBetweenDefs.
rubocop: Auto-fixed 11 cop offenses: Layout/EmptyLinesAroundAttributeAccessor.
rubocop: Auto-fixed 16 offenses: azure_client.rb.
rubocop: Excluded 6 offenses: Style/BisectedAttrAccessor.
NOTE: This cop was permanently disabled (in '.rubocop.yml' instead of '.rubocop_todo.yml') for 1 particular file, for reasons detailed in the comments, since using inline rubocop comments to exclude the 6 specific offenses did not prevent `rubocop -A` from auto-fixing those offenses. This could unintentionally prevent other (future, hypothetical) offenses in that same file from getting detected/fixed, but that seems preferable these 6 offenses getting unintentionally auto-fixed.
rubocop: Excluded 2 offenses: Style/ArgumentsForwarding.
NOTE: The offenses were permanently excluded (in '.rubocop.yml' instead of '.rubocop_todo.yml'), since fixing them would be backwards-incompatible with Ruby < v2.7, and it seems like the file containing the offenses _might_ need to be backwards-compatible with earlier versions.
rubocop: Disable the `Style/TrailingUnderscoreVariable` cop.
NOTE: Each of the existing offenses is less readable if the offense if fixed. So this seems like a cop which reduces readability, and should therefore be disabled project-wide.
rubocop: Inline-disabled the `Metrics/ClassLength` cop for all 12 classes over the default 100 lines.
NOTE: It seems preferable to explicitly disable this cop (inline) for these classes, rather than setting a higher project-wide threshold (in .rubocop_todo.yml) since the threshold doesn't make reveal which classes are over the default threshold.
rubocop: Disable the `Metrics/BlockLength` cop for all '*_spec.rb' files.
NOTE: Since spec files tend to have long blocks. it seems preferable to explicitly disable this cop for those files, in order to be able to set a lower project-wide limit (which will apply to non-spec files).
NOTE: Although 'shared_stuff.rb' is not technically a spec file, it contains rspec code, and should therefore be subject to similar rubocop config as the '*_spec.rb' files (see the NOTE above).
Added 'TODO: Refactoring' comments: config.rb.
SEE: https://rubystyle.guide/#one-class-per-file
NOTE: This file contains 9 "model" classes, of which only 1 is named "Config", and at least 4 of the 9 appear to be completely unrelated to the Config class. Also, several of the 9 classes have class-specific spec files (see: "src/bosh_azure_cpi/spec/unit/models/*_config_spec.rb"). So it seems like the code would be more maintainable if up to 8 of the 9 classes were split into separate files, with filenames corresponding to the class name and spec file name.
Minor layout refactoring: delete_spec.rb.
NOTE: This refactoring makes the modified code section's layout more consistent with the project-wide layout conventions.
Fix typo in comment: stemcell_manager2.rb
  • Loading branch information
Justin-W committed Jan 27, 2022
1 parent 828817f commit 7adc1f7
Show file tree
Hide file tree
Showing 122 changed files with 919 additions and 1,292 deletions.
32 changes: 31 additions & 1 deletion src/bosh_azure_cpi/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ Lint/UnmodifiedReduceAccumulator: # new in 1.1
Lint/UselessRuby2Keywords: # new in 1.23
Enabled: true

Metrics/BlockLength:
Exclude:
- 'spec/unit/vm_manager/create/shared_stuff.rb'
- '**/*_spec.rb'

RSpec/ExcessiveDocstringSpacing: # new in 2.5
Enabled: true

Expand All @@ -100,8 +105,21 @@ RSpec/Rails/AvoidSetupHook: # new in 2.4
Security/IoMethods: # new in 1.22
Enabled: true

# Offense count: 2
# Cop supports --auto-correct.
# Configuration parameters: AllowOnlyRestArgument.
Style/ArgumentsForwarding: # new in 1.1
Enabled: true
Exclude:
# Note: Excluding the following file since it seems like that file might need backwards-compatibility with earlier versions than Ruby v2.7.
- 'spec/unit/bosh_release/jobs/cpi/templates/cpi.json.erb_spec.rb'

# Offense count: 6
# Cop supports --auto-correct.
Style/BisectedAttrAccessor:
Exclude:
# Note: Excluding the following file for the reasons noted within its comments.
- 'lib/cloud/azure/models/vm_cloud_props.rb'

Style/CollectionCompact: # new in 1.2
Enabled: true
Expand All @@ -118,6 +136,13 @@ Style/HashConversion: # new in 1.10
Style/HashExcept: # new in 1.7
Enabled: true

# Offense count: 1
# Cop supports --auto-correct.
Style/HashTransformValues:
Exclude:
# Note: Excluding the following file since it seems like that file might need backwards-compatibility with earlier versions than Ruby v2.4.
- 'spec/unit/bosh_release/jobs/cpi/templates/cpi.json.erb_spec.rb'

Style/IfWithBooleanLiteralBranches: # new in 1.9
Enabled: true

Expand Down Expand Up @@ -158,4 +183,9 @@ Style/StringChars: # new in 1.12
Enabled: true

Style/SwapValues: # new in 1.1
Enabled: true
Enabled: true

# Cop supports --auto-correct.
# Configuration parameters: AllowNamedUnderscoreVariables.
Style/TrailingUnderscoreVariable:
Enabled: false # Note: This cop makes code harder to read, IMO.
Loading

0 comments on commit 7adc1f7

Please sign in to comment.