Merge pull request #17 from codeclimate/devon/update-rubocop-config-to-match-styleguide

Update Rubocop config to match CC styleguide

Author: dblandin

Diff for: .codeclimate.yml

@@ -1,4 +1,15 @@
 engines:
+  bundler-audit:
+    enabled: true
+  duplication:
+    enabled: true
+    config:
+      languages:
+      - ruby
+    exclude_paths:
+    - spec/
+  fixme:
+    enabled: true
   rubocop:
     enabled: true
     exclude_fingerprints:

Diff for: .rubocop.yml

@@ -1,43 +1,120 @@
+################################################################################
+# Ruby Version
+################################################################################
+
 AllCops:
   TargetRubyVersion: 2.2
-Metrics/AbcSize:
-  Enabled: false
+
+################################################################################
+# Metrics
+################################################################################
 
 Metrics/LineLength:
   Enabled: false
 
-Rails/TimeZone:
+Metrics/AbcSize:
   Enabled: false
 
-SignalException:
-  Enabled: false
+################################################################################
+# Style
+################################################################################
 
-Style/StringLiterals:
-  Enabled: false
+# Executables are conventionally named bin/foo-bar
+Style/FileName:
+  Exclude:
+  - bin/**/*
 
+# We don't (currently) document our code
 Style/Documentation:
   Enabled: false
 
+# Always use double-quotes to keep things simple
+Style/StringLiterals:
+  EnforcedStyle: double_quotes
+
+# Use a trailing comma to keep diffs clean when elements are inserted or removed
 Style/TrailingCommaInLiteral:
-  Enabled: false
+  EnforcedStyleForMultiline: comma
 
-Style/ClassAndModuleChildren:
-  Exclude:
-    - 'spec/**/*'
+# We avoid GuardClause because it can result in "suprise return"
+Style/GuardClause:
+  Enabled: false
 
+# We avoid IfUnlessModifier because it can result in "suprise if"
 Style/IfUnlessModifier:
   Enabled: false
 
+# We don't care about the fail/raise distinction
+Style/SignalException:
+  EnforcedStyle: only_raise
+
 Style/DotPosition:
+  EnforcedStyle: trailing
+
+# Common globals we allow
+Style/GlobalVars:
+  AllowedVariables:
+  - "$statsd"
+  - "$mongo"
+  - "$rollout"
+
+# Allow $! in config/initializers
+Style/SpecialGlobalVars:
+  Exclude:
+  - config/initializers/**/*
+
+# We have common cases where has_ and have_ make sense
+Style/PredicateName:
+  Enabled: true
+  NamePrefixBlacklist:
+  - is_
+
+# We use %w[ ], not %w( ) because the former looks like an array
+Style/PercentLiteralDelimiters:
+  PreferredDelimiters:
+    "%w": []
+    "%W": []
+
+# Allow "trivial" accessors when defined as a predicate? method
+Style/TrivialAccessors:
+  AllowPredicates: true
+
+Style/Next:
   Enabled: false
 
-Style/GuardClause:
+# We think it's OK to use the "extend self" module pattern
+Style/ModuleFunction:
   Enabled: false
 
-Style/StringLiteralsInInterpolation:
+################################################################################
+# Rails - disable things because we're primarily non-rails
+################################################################################
+
+Rails/Delegate:
   Enabled: false
 
-Style/PercentLiteralDelimiters:
-  PreferredDelimiters:
-    '%w': []
-    '%W': []
+Rails/Output:
+  Enabled: false
+
+Rails/TimeZone:
+  Enabled: false
+
+################################################################################
+# Specs - be more lenient on length checks and block styles
+################################################################################
+
+Metrics/ModuleLength:
+  Exclude:
+  - spec/**/*
+
+Metrics/MethodLength:
+  Exclude:
+  - spec/**/*
+
+Style/ClassAndModuleChildren:
+  Exclude:
+  - spec/**/*
+
+Style/BlockDelimiters:
+  Exclude:
+  - spec/**/*

Diff for: Rakefile

@@ -1,4 +1,4 @@
-require 'rspec/core/rake_task'
+require "rspec/core/rake_task"
 RSpec::Core::RakeTask.new(:spec)
 
 task default: :spec

Diff for: bin/bundler-audit

@@ -2,6 +2,6 @@
 
 $LOAD_PATH.unshift(File.expand_path(File.join(File.dirname(__FILE__), "../lib")))
 
-require 'cc/engine/bundler_audit'
+require "cc/engine/bundler_audit"
 
 CC::Engine::BundlerAudit::Analyzer.new(directory: "/code").run

Diff for: lib/cc/engine/bundler_audit/issue.rb

@@ -6,7 +6,7 @@ class Issue
         SEVERITIES = {
           high: "critical",
           medium: "normal",
-          low: "info"
+          low: "info",
         }.freeze
 
         def initialize(result, gemfile_lock_lines)
@@ -20,19 +20,19 @@ def to_json(*a)
             categories: %w[Security],
             check_name: "Insecure Dependency",
             content: {
-              body: content_body
+              body: content_body,
             },
             description: advisory.title,
             location: {
               path: "Gemfile.lock",
               lines: {
                 begin: line_number,
-                end: line_number
-              }
+                end: line_number,
+              },
             },
             remediation_points: remediation_points,
             severity: severity,
-            type: "Issue"
+            type: "Issue",
           }.to_json(a)
         end
 
@@ -45,7 +45,7 @@ def content_body
             "**Advisory**: #{identifier}",
             "**Criticality**: #{advisory.criticality.capitalize}",
             "**URL**: #{advisory.url}",
-            "**Solution**: #{solution}"
+            "**Solution**: #{solution}",
           ].join("\n\n")
         end
 

Diff for: spec/cc/engine/bundler_audit/analyzer_spec.rb

@@ -7,8 +7,8 @@ module CC::Engine::BundlerAudit
         directory = fixture_directory("no_gemfile_lock")
         io = StringIO.new
 
-        expect { Analyzer.new(directory: directory, io: io).run }
-          .to raise_error(Analyzer::GemfileLockNotFound)
+        expect { Analyzer.new(directory: directory, io: io).run }.
+          to raise_error(Analyzer::GemfileLockNotFound)
       end
 
       it "emits issues for Gemfile.lock problems" do