Added rubocop / codeclimate config and fixed current offenses (#45)

cristianbica · David Heinemeier Hansson · commit 6dcdc5c9abb8 · 2017-07-14T00:09:56.000+02:00
diff --git a/.codeclimate.yml b/.codeclimate.yml
@@ -0,0 +1,7 @@
+engines:
+  rubocop:
+    enabled: true
+
+ratings:
+  paths:
+  - "**.rb"
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -0,0 +1,125 @@
+AllCops:
+  TargetRubyVersion: 2.3
+  # RuboCop has a bunch of cops enabled by default. This setting tells RuboCop
+  # to ignore them, so only the ones explicitly set in this file are enabled.
+  DisabledByDefault: true
+  Exclude:
+    - '**/templates/**/*'
+    - '**/vendor/**/*'
+    - 'actionpack/lib/action_dispatch/journey/parser.rb'
+
+# Prefer &&/|| over and/or.
+Style/AndOr:
+  Enabled: true
+
+# Do not use braces for hash literals when they are the last argument of a
+# method call.
+Style/BracesAroundHashParameters:
+  Enabled: true
+  EnforcedStyle: context_dependent
+
+# Align `when` with `case`.
+Style/CaseIndentation:
+  Enabled: true
+
+# Align comments with method definitions.
+Style/CommentIndentation:
+  Enabled: true
+
+# No extra empty lines.
+Style/EmptyLines:
+  Enabled: false
+
+# In a regular class definition, no empty lines around the body.
+Style/EmptyLinesAroundClassBody:
+  Enabled: true
+
+# In a regular method definition, no empty lines around the body.
+Style/EmptyLinesAroundMethodBody:
+  Enabled: true
+
+# In a regular module definition, no empty lines around the body.
+Style/EmptyLinesAroundModuleBody:
+  Enabled: true
+
+# Use Ruby >= 1.9 syntax for hashes. Prefer { a: :b } over { :a => :b }.
+Style/HashSyntax:
+  Enabled: true
+
+# Method definitions after `private` or `protected` isolated calls need one
+# extra level of indentation.
+Style/IndentationConsistency:
+  Enabled: true
+  EnforcedStyle: rails
+
+# Two spaces, no tabs (for indentation).
+Style/IndentationWidth:
+  Enabled: true
+
+Style/SpaceAfterColon:
+  Enabled: true
+
+Style/SpaceAfterComma:
+  Enabled: true
+
+Style/SpaceAroundEqualsInParameterDefault:
+  Enabled: true
+
+Style/SpaceAroundKeyword:
+  Enabled: true
+
+Style/SpaceAroundOperators:
+  Enabled: true
+
+Style/SpaceBeforeFirstArg:
+    Enabled: true
+
+# Defining a method with parameters needs parentheses.
+Style/MethodDefParentheses:
+  Enabled: true
+
+# Use `foo {}` not `foo{}`.
+Style/SpaceBeforeBlockBraces:
+  Enabled: true
+
+# Use `foo { bar }` not `foo {bar}`.
+Style/SpaceInsideBlockBraces:
+  Enabled: true
+
+# Use `{ a: 1 }` not `{a:1}`.
+Style/SpaceInsideHashLiteralBraces:
+  Enabled: true
+
+Style/SpaceInsideParens:
+  Enabled: true
+
+# Check quotes usage according to lint rule below.
+Style/StringLiterals:
+  Enabled: true
+  EnforcedStyle: double_quotes
+
+# Detect hard tabs, no hard tabs.
+Style/Tab:
+  Enabled: true
+
+# Blank lines should not have any spaces.
+Style/TrailingBlankLines:
+  Enabled: true
+
+# No trailing whitespace.
+Style/TrailingWhitespace:
+  Enabled: true
+
+# Use quotes for string literals when they are enough.
+Style/UnneededPercentQ:
+  Enabled: true
+
+# Align `end` with the matching keyword or starting expression except for
+# assignments, where it should be aligned with the LHS.
+Lint/EndAlignment:
+  Enabled: true
+  EnforcedStyleAlignWith: variable
+
+# Use my_method(my_arg) not my_method( my_arg ) or my_method my_arg.
+Lint/RequireParentheses:
+  Enabled: true
diff --git a/Gemfile b/Gemfile
@@ -1,12 +1,14 @@
-source 'https://rubygems.org'
+source "https://rubygems.org"
 
 gemspec
 
-gem 'rake'
-gem 'byebug'
+gem "rake"
+gem "byebug"
 
-gem 'sqlite3'
-gem 'httparty'
+gem "sqlite3"
+gem "httparty"
 
-gem 'aws-sdk', '~> 2', require: false
-gem 'google-cloud-storage', '~> 1.3', require: false
+gem "aws-sdk", "~> 2", require: false
+gem "google-cloud-storage", "~> 1.3", require: false
+
+gem "rubocop", require: false
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -40,6 +40,7 @@ GEM
     addressable (2.5.1)
       public_suffix (~> 2.0, >= 2.0.2)
     arel (8.0.0)
+    ast (2.3.0)
     aws-sdk (2.10.7)
       aws-sdk-resources (= 2.10.7)
     aws-sdk-core (2.10.7)
@@ -107,6 +108,10 @@ GEM
     nokogiri (1.7.2)
       mini_portile2 (~> 2.1.0)
     os (0.9.6)
+    parallel (1.11.2)
+    parser (2.4.0.0)
+      ast (~> 2.2)
+    powerpack (0.1.1)
     public_suffix (2.0.5)
     rack (2.0.3)
     rack-test (0.6.3)
@@ -116,12 +121,22 @@ GEM
       nokogiri (>= 1.6)
     rails-html-sanitizer (1.0.3)
       loofah (~> 2.0)
+    rainbow (2.2.2)
+      rake
     rake (12.0.0)
     representable (3.0.4)
       declarative (< 0.1.0)
       declarative-option (< 0.2.0)
       uber (< 0.2.0)
     retriable (3.0.2)
+    rubocop (0.49.1)
+      parallel (~> 1.10)
+      parser (>= 2.3.3.1, < 3.0)
+      powerpack (~> 0.1)
+      rainbow (>= 1.99.1, < 3.0)
+      ruby-progressbar (~> 1.7)
+      unicode-display_width (~> 1.0, >= 1.0.1)
+    ruby-progressbar (1.8.1)
     signet (0.7.3)
       addressable (~> 2.3)
       faraday (~> 0.9)
@@ -132,6 +147,7 @@ GEM
     tzinfo (1.2.3)
       thread_safe (~> 0.1)
     uber (0.1.0)
+    unicode-display_width (1.3.0)
 
 PLATFORMS
   ruby
@@ -144,6 +160,7 @@ DEPENDENCIES
   google-cloud-storage (~> 1.3)
   httparty
   rake
+  rubocop
   sqlite3
 
 BUNDLED WITH
diff --git a/lib/active_storage/disk_controller.rb b/lib/active_storage/disk_controller.rb
@@ -33,6 +33,6 @@ def decode_verified_key
     end
 
     def disposition_param
-      params[:disposition].presence_in(%w( inline attachment )) || 'inline'
+      params[:disposition].presence_in(%w( inline attachment )) || "inline"
     end
 end
diff --git a/lib/active_storage/download.rb b/lib/active_storage/download.rb
@@ -14,19 +14,19 @@ class ActiveStorage::Download
     application/xhtml+xml
   )
 
-  BINARY_CONTENT_TYPE = 'application/octet-stream'
+  BINARY_CONTENT_TYPE = "application/octet-stream"
 
   def initialize(stored_file)
     @stored_file = stored_file
   end
 
   def headers(force_attachment: false)
     {
-      x_accel_redirect:    '/reproxy',
+      x_accel_redirect:    "/reproxy",
       x_reproxy_url:       reproxy_url,
       content_type:        content_type,
       content_disposition: content_disposition(force_attachment),
-      x_frame_options:     'SAMEORIGIN'
+      x_frame_options:     "SAMEORIGIN"
     }
   end
 
diff --git a/lib/active_storage/service.rb b/lib/active_storage/service.rb
@@ -85,7 +85,7 @@ def url_for_direct_upload(key, expires_in:, content_type:, content_length:)
   private
     def instrument(operation, key, payload = {}, &block)
       ActiveSupport::Notifications.instrument(
-        "service_#{operation}.active_storage", 
+        "service_#{operation}.active_storage",
         payload.merge(key: key, service: service_name), &block)
     end
 
diff --git a/lib/active_storage/service/disk_service.rb b/lib/active_storage/service/disk_service.rb
@@ -20,7 +20,7 @@ def upload(key, io, checksum: nil)
   def download(key)
     if block_given?
       instrument :streaming_download, key do
-        File.open(path_for(key), 'rb') do |file|
+        File.open(path_for(key), "rb") do |file|
           while data = file.read(64.kilobytes)
             yield data
           end
diff --git a/lib/active_storage/service/s3_service.rb b/lib/active_storage/service/s3_service.rb
@@ -49,9 +49,9 @@ def url(key, expires_in:, disposition:, filename:)
     instrument :url, key do |payload|
       generated_url = object_for(key).presigned_url :get, expires_in: expires_in,
         response_content_disposition: "#{disposition}; filename=\"#{filename}\""
-      
+
       payload[:url] = generated_url
-      
+
       generated_url
     end
   end
@@ -60,9 +60,9 @@ def url_for_direct_upload(key, expires_in:, content_type:, content_length:)
     instrument :url, key do |payload|
       generated_url = object_for(key).presigned_url :put, expires_in: expires_in,
         content_type: content_type, content_length: content_length
-      
+
       payload[:url] = generated_url
-      
+
       generated_url
     end
   end
diff --git a/lib/active_storage/verified_key_with_expiration.rb b/lib/active_storage/verified_key_with_expiration.rb
@@ -1,5 +1,5 @@
 class ActiveStorage::VerifiedKeyWithExpiration
-  class_attribute :verifier, default: defined?(Rails) ? Rails.application.message_verifier('ActiveStorage') : nil
+  class_attribute :verifier, default: defined?(Rails) ? Rails.application.message_verifier("ActiveStorage") : nil
 
   class << self
     def encode(key, expires_in: nil)
diff --git a/lib/tasks/activestorage.rake b/lib/tasks/activestorage.rake
@@ -7,7 +7,7 @@ namespace :activestorage do
     FileUtils.mkdir_p Rails.root.join("tmp/storage")
     puts "Made storage and tmp/storage directories for development and testing"
 
-    FileUtils.cp  File.expand_path("../../active_storage/storage_services.yml", __FILE__), Rails.root.join("config")
+    FileUtils.cp File.expand_path("../../active_storage/storage_services.yml", __FILE__), Rails.root.join("config")
     puts "Copied default configuration to config/storage_services.yml"
 
     migration_file_path = "db/migrate/#{Time.now.utc.strftime("%Y%m%d%H%M%S")}_active_storage_create_tables.rb"
diff --git a/test/attachments_test.rb b/test/attachments_test.rb
@@ -66,7 +66,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
 
   test "attach new blobs" do
     @user.highlights.attach(
-      { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" }, 
+      { io: StringIO.new("STUFF"), filename: "town.jpg", content_type: "image/jpg" },
       { io: StringIO.new("IT"), filename: "country.jpg", content_type: "image/jpg" })
 
     assert_equal "town.jpg", @user.highlights.first.filename.to_s
@@ -76,7 +76,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
   test "purge attached blobs" do
     @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg")
     highlight_keys = @user.highlights.collect(&:key)
-    
+
     @user.highlights.purge
     assert_not @user.highlights.attached?
     assert_not ActiveStorage::Blob.service.exist?(highlight_keys.first)
diff --git a/test/blob_test.rb b/test/blob_test.rb
@@ -13,16 +13,16 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
   end
 
   test "download yields chunks" do
-    blob   = create_blob data: 'a' * 75.kilobytes
+    blob   = create_blob data: "a" * 75.kilobytes
     chunks = []
 
     blob.download do |chunk|
       chunks << chunk
     end
 
     assert_equal 2, chunks.size
-    assert_equal 'a' * 64.kilobytes, chunks.first
-    assert_equal 'a' * 11.kilobytes, chunks.second
+    assert_equal "a" * 64.kilobytes, chunks.first
+    assert_equal "a" * 11.kilobytes, chunks.second
   end
 
   test "urls expiring in 5 minutes" do
diff --git a/test/database/setup.rb b/test/database/setup.rb
@@ -1,6 +1,6 @@
 require "active_storage/migration"
 require_relative "create_users_migration"
 
-ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
+ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
 ActiveStorageCreateTables.migrate(:up)
 ActiveStorageCreateUsers.migrate(:up)
diff --git a/test/filename_test.rb b/test/filename_test.rb
@@ -4,8 +4,8 @@ class ActiveStorage::FilenameTest < ActiveSupport::TestCase
   test "sanitize" do
     "%$|:;/\t\r\n\\".each_char do |character|
       filename = ActiveStorage::Filename.new("foo#{character}bar.pdf")
-      assert_equal 'foo-bar.pdf', filename.sanitized
-      assert_equal 'foo-bar.pdf', filename.to_s
+      assert_equal "foo-bar.pdf", filename.sanitized
+      assert_equal "foo-bar.pdf", filename.to_s
     end
   end
 
@@ -23,14 +23,14 @@ class ActiveStorage::FilenameTest < ActiveSupport::TestCase
   test "strips RTL override chars used to spoof unsafe executables as docs" do
     # Would be displayed in Windows as "evilexe.pdf" due to the right-to-left
     # (RTL) override char!
-    assert_equal 'evil-fdp.exe', ActiveStorage::Filename.new("evil\u{202E}fdp.exe").sanitized
+    assert_equal "evil-fdp.exe", ActiveStorage::Filename.new("evil\u{202E}fdp.exe").sanitized
   end
 
   test "compare case-insensitively" do
-    assert_operator ActiveStorage::Filename.new('foobar.pdf'), :==, ActiveStorage::Filename.new('FooBar.PDF')
+    assert_operator ActiveStorage::Filename.new("foobar.pdf"), :==, ActiveStorage::Filename.new("FooBar.PDF")
   end
 
   test "compare sanitized" do
-    assert_operator ActiveStorage::Filename.new('foo-bar.pdf'), :==, ActiveStorage::Filename.new("foo\tbar.pdf")
+    assert_operator ActiveStorage::Filename.new("foo-bar.pdf"), :==, ActiveStorage::Filename.new("foo\tbar.pdf")
   end
 end
diff --git a/test/service/configurator_test.rb b/test/service/configurator_test.rb
@@ -12,4 +12,3 @@ class ActiveStorage::Service::ConfiguratorTest < ActiveSupport::TestCase
     end
   end
 end
-
diff --git a/test/service/disk_service_test.rb b/test/service/disk_service_test.rb
@@ -7,6 +7,6 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase
 
   test "url generation" do
     assert_match /rails\/active_storage\/disk\/.*\/avatar\.png\?disposition=inline/,
-      @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: "avatar.png")    
+      @service.url(FIXTURE_KEY, expires_in: 5.minutes, disposition: :inline, filename: "avatar.png")
   end
 end
diff --git a/test/service/mirror_service_test.rb b/test/service/mirror_service_test.rb
@@ -8,7 +8,7 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase
   end.to_h
 
   config = mirror_config.merge \
-    mirror:   { service: "Mirror", primary: 'primary', mirrors: mirror_config.keys },
+    mirror:   { service: "Mirror", primary: "primary", mirrors: mirror_config.keys },
     primary:  { service: "Disk", root: Dir.mktmpdir("active_storage_tests_primary") }
 
   SERVICE = ActiveStorage::Service.configure :mirror, config

Original file line number	Diff line number	Diff line change
`@@ -14,19 +14,19 @@ class ActiveStorage::Download`
`14`	`14`	`application/xhtml+xml`
`15`	`15`	`)`
`16`	`16`
`17`		`- BINARY_CONTENT_TYPE = 'application/octet-stream'`
	`17`	`+ BINARY_CONTENT_TYPE = "application/octet-stream"`
`18`	`18`
`19`	`19`	`def initialize(stored_file)`
`20`	`20`	`@stored_file = stored_file`
`21`	`21`	`end`
`22`	`22`
`23`	`23`	`def headers(force_attachment: false)`
`24`	`24`	`{`
`25`		`- x_accel_redirect: '/reproxy',`
	`25`	`+ x_accel_redirect: "/reproxy",`
`26`	`26`	`x_reproxy_url: reproxy_url,`
`27`	`27`	`content_type: content_type,`
`28`	`28`	`content_disposition: content_disposition(force_attachment),`
`29`		`- x_frame_options: 'SAMEORIGIN'`
	`29`	`+ x_frame_options: "SAMEORIGIN"`
`30`	`30`	`}`
`31`	`31`	`end`
`32`	`32`