1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

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

This commit is contained in:
Cristian Bica 2017-07-14 01:09:56 +03:00 committed by David Heinemeier Hansson
parent 14e6386b3c
commit 6dcdc5c9ab
18 changed files with 183 additions and 33 deletions

7
.codeclimate.yml Normal file
View file

@ -0,0 +1,7 @@
engines:
rubocop:
enabled: true
ratings:
paths:
- "**.rb"

125
.rubocop.yml Normal file
View file

@ -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

16
Gemfile
View file

@ -1,12 +1,14 @@
source 'https://rubygems.org' source "https://rubygems.org"
gemspec gemspec
gem 'rake' gem "rake"
gem 'byebug' gem "byebug"
gem 'sqlite3' gem "sqlite3"
gem 'httparty' gem "httparty"
gem 'aws-sdk', '~> 2', require: false gem "aws-sdk", "~> 2", require: false
gem 'google-cloud-storage', '~> 1.3', require: false gem "google-cloud-storage", "~> 1.3", require: false
gem "rubocop", require: false

View file

@ -40,6 +40,7 @@ GEM
addressable (2.5.1) addressable (2.5.1)
public_suffix (~> 2.0, >= 2.0.2) public_suffix (~> 2.0, >= 2.0.2)
arel (8.0.0) arel (8.0.0)
ast (2.3.0)
aws-sdk (2.10.7) aws-sdk (2.10.7)
aws-sdk-resources (= 2.10.7) aws-sdk-resources (= 2.10.7)
aws-sdk-core (2.10.7) aws-sdk-core (2.10.7)
@ -107,6 +108,10 @@ GEM
nokogiri (1.7.2) nokogiri (1.7.2)
mini_portile2 (~> 2.1.0) mini_portile2 (~> 2.1.0)
os (0.9.6) 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) public_suffix (2.0.5)
rack (2.0.3) rack (2.0.3)
rack-test (0.6.3) rack-test (0.6.3)
@ -116,12 +121,22 @@ GEM
nokogiri (>= 1.6) nokogiri (>= 1.6)
rails-html-sanitizer (1.0.3) rails-html-sanitizer (1.0.3)
loofah (~> 2.0) loofah (~> 2.0)
rainbow (2.2.2)
rake
rake (12.0.0) rake (12.0.0)
representable (3.0.4) representable (3.0.4)
declarative (< 0.1.0) declarative (< 0.1.0)
declarative-option (< 0.2.0) declarative-option (< 0.2.0)
uber (< 0.2.0) uber (< 0.2.0)
retriable (3.0.2) 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) signet (0.7.3)
addressable (~> 2.3) addressable (~> 2.3)
faraday (~> 0.9) faraday (~> 0.9)
@ -132,6 +147,7 @@ GEM
tzinfo (1.2.3) tzinfo (1.2.3)
thread_safe (~> 0.1) thread_safe (~> 0.1)
uber (0.1.0) uber (0.1.0)
unicode-display_width (1.3.0)
PLATFORMS PLATFORMS
ruby ruby
@ -144,6 +160,7 @@ DEPENDENCIES
google-cloud-storage (~> 1.3) google-cloud-storage (~> 1.3)
httparty httparty
rake rake
rubocop
sqlite3 sqlite3
BUNDLED WITH BUNDLED WITH

View file

@ -33,6 +33,6 @@ class ActiveStorage::DiskController < ActionController::Base
end end
def disposition_param def disposition_param
params[:disposition].presence_in(%w( inline attachment )) || 'inline' params[:disposition].presence_in(%w( inline attachment )) || "inline"
end end
end end

View file

@ -14,7 +14,7 @@ class ActiveStorage::Download
application/xhtml+xml application/xhtml+xml
) )
BINARY_CONTENT_TYPE = 'application/octet-stream' BINARY_CONTENT_TYPE = "application/octet-stream"
def initialize(stored_file) def initialize(stored_file)
@stored_file = stored_file @stored_file = stored_file
@ -22,11 +22,11 @@ class ActiveStorage::Download
def headers(force_attachment: false) def headers(force_attachment: false)
{ {
x_accel_redirect: '/reproxy', x_accel_redirect: "/reproxy",
x_reproxy_url: reproxy_url, x_reproxy_url: reproxy_url,
content_type: content_type, content_type: content_type,
content_disposition: content_disposition(force_attachment), content_disposition: content_disposition(force_attachment),
x_frame_options: 'SAMEORIGIN' x_frame_options: "SAMEORIGIN"
} }
end end

View file

@ -85,7 +85,7 @@ class ActiveStorage::Service
private private
def instrument(operation, key, payload = {}, &block) def instrument(operation, key, payload = {}, &block)
ActiveSupport::Notifications.instrument( ActiveSupport::Notifications.instrument(
"service_#{operation}.active_storage", "service_#{operation}.active_storage",
payload.merge(key: key, service: service_name), &block) payload.merge(key: key, service: service_name), &block)
end end

View file

@ -20,7 +20,7 @@ class ActiveStorage::Service::DiskService < ActiveStorage::Service
def download(key) def download(key)
if block_given? if block_given?
instrument :streaming_download, key do 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) while data = file.read(64.kilobytes)
yield data yield data
end end

View file

@ -49,9 +49,9 @@ class ActiveStorage::Service::S3Service < ActiveStorage::Service
instrument :url, key do |payload| instrument :url, key do |payload|
generated_url = object_for(key).presigned_url :get, expires_in: expires_in, generated_url = object_for(key).presigned_url :get, expires_in: expires_in,
response_content_disposition: "#{disposition}; filename=\"#{filename}\"" response_content_disposition: "#{disposition}; filename=\"#{filename}\""
payload[:url] = generated_url payload[:url] = generated_url
generated_url generated_url
end end
end end
@ -60,9 +60,9 @@ class ActiveStorage::Service::S3Service < ActiveStorage::Service
instrument :url, key do |payload| instrument :url, key do |payload|
generated_url = object_for(key).presigned_url :put, expires_in: expires_in, generated_url = object_for(key).presigned_url :put, expires_in: expires_in,
content_type: content_type, content_length: content_length content_type: content_type, content_length: content_length
payload[:url] = generated_url payload[:url] = generated_url
generated_url generated_url
end end
end end

View file

@ -1,5 +1,5 @@
class ActiveStorage::VerifiedKeyWithExpiration 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 class << self
def encode(key, expires_in: nil) def encode(key, expires_in: nil)

View file

@ -7,7 +7,7 @@ namespace :activestorage do
FileUtils.mkdir_p Rails.root.join("tmp/storage") FileUtils.mkdir_p Rails.root.join("tmp/storage")
puts "Made storage and tmp/storage directories for development and testing" 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" 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" migration_file_path = "db/migrate/#{Time.now.utc.strftime("%Y%m%d%H%M%S")}_active_storage_create_tables.rb"

View file

@ -66,7 +66,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
test "attach new blobs" do test "attach new blobs" do
@user.highlights.attach( @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" }) { io: StringIO.new("IT"), filename: "country.jpg", content_type: "image/jpg" })
assert_equal "town.jpg", @user.highlights.first.filename.to_s assert_equal "town.jpg", @user.highlights.first.filename.to_s
@ -76,7 +76,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase
test "purge attached blobs" do test "purge attached blobs" do
@user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg") @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg")
highlight_keys = @user.highlights.collect(&:key) highlight_keys = @user.highlights.collect(&:key)
@user.highlights.purge @user.highlights.purge
assert_not @user.highlights.attached? assert_not @user.highlights.attached?
assert_not ActiveStorage::Blob.service.exist?(highlight_keys.first) assert_not ActiveStorage::Blob.service.exist?(highlight_keys.first)

View file

@ -13,7 +13,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
end end
test "download yields chunks" do test "download yields chunks" do
blob = create_blob data: 'a' * 75.kilobytes blob = create_blob data: "a" * 75.kilobytes
chunks = [] chunks = []
blob.download do |chunk| blob.download do |chunk|
@ -21,8 +21,8 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
end end
assert_equal 2, chunks.size assert_equal 2, chunks.size
assert_equal 'a' * 64.kilobytes, chunks.first assert_equal "a" * 64.kilobytes, chunks.first
assert_equal 'a' * 11.kilobytes, chunks.second assert_equal "a" * 11.kilobytes, chunks.second
end end
test "urls expiring in 5 minutes" do test "urls expiring in 5 minutes" do

View file

@ -1,6 +1,6 @@
require "active_storage/migration" require "active_storage/migration"
require_relative "create_users_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) ActiveStorageCreateTables.migrate(:up)
ActiveStorageCreateUsers.migrate(:up) ActiveStorageCreateUsers.migrate(:up)

View file

@ -4,8 +4,8 @@ class ActiveStorage::FilenameTest < ActiveSupport::TestCase
test "sanitize" do test "sanitize" do
"%$|:;/\t\r\n\\".each_char do |character| "%$|:;/\t\r\n\\".each_char do |character|
filename = ActiveStorage::Filename.new("foo#{character}bar.pdf") filename = ActiveStorage::Filename.new("foo#{character}bar.pdf")
assert_equal 'foo-bar.pdf', filename.sanitized assert_equal "foo-bar.pdf", filename.sanitized
assert_equal 'foo-bar.pdf', filename.to_s assert_equal "foo-bar.pdf", filename.to_s
end end
end end
@ -23,14 +23,14 @@ class ActiveStorage::FilenameTest < ActiveSupport::TestCase
test "strips RTL override chars used to spoof unsafe executables as docs" do 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 # Would be displayed in Windows as "evilexe.pdf" due to the right-to-left
# (RTL) override char! # (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 end
test "compare case-insensitively" do 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 end
test "compare sanitized" do 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
end end

View file

@ -12,4 +12,3 @@ class ActiveStorage::Service::ConfiguratorTest < ActiveSupport::TestCase
end end
end end
end end

View file

@ -7,6 +7,6 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase
test "url generation" do test "url generation" do
assert_match /rails\/active_storage\/disk\/.*\/avatar\.png\?disposition=inline/, 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
end end

View file

@ -8,7 +8,7 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase
end.to_h end.to_h
config = mirror_config.merge \ 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") } primary: { service: "Disk", root: Dir.mktmpdir("active_storage_tests_primary") }
SERVICE = ActiveStorage::Service.configure :mirror, config SERVICE = ActiveStorage::Service.configure :mirror, config