From 69285b955c11504e7dcd7c9b6875b518c18014cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Mon, 16 Mar 2015 12:45:44 +0100 Subject: [PATCH] Enable Ruby linting * Add Rubocop * Add guard-rubocop * Enable Ruby linting on Hound --- .hound.yml | 3 +- .rubocop.yml | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++ Gemfile | 4 +- Gemfile.lock | 18 +++++++ Guardfile | 47 ++++++++++++------- 5 files changed, 184 insertions(+), 18 deletions(-) create mode 100644 .rubocop.yml diff --git a/.hound.yml b/.hound.yml index da013757b..9c0f49ddb 100644 --- a/.hound.yml +++ b/.hound.yml @@ -3,6 +3,7 @@ java_script: config_file: config/.jshint.json ignore_file: config/.jshint_ignore ruby: - enabled: false + enabled: true + config_file: .rubocop.yml scss: enabled: false diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 000000000..2148bb5d0 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,130 @@ +AllCops: + RunRailsCops: true + +# Commonly used screens these days easily fit more than 80 characters. +Metrics/LineLength: + Max: 120 + +# Too short methods lead to extraction of single-use methods, which can make +# the code easier to read (by naming things), but can also clutter the class +Metrics/MethodLength: + Max: 20 + +# The guiding principle of classes is SRP, SRP can't be accurately measured by LoC +Metrics/ClassLength: + Max: 1500 + +# No space makes the method definition shorter and differentiates +# from a regular assignment. +Style/SpaceAroundEqualsInParameterDefault: + EnforcedStyle: no_space + +# Single quotes being faster is hardly measurable and only affects parse time. +# Enforcing double quotes reduces the times where you need to change them +# when introducing an interpolation. Use single quotes only if their semantics +# are needed. +Style/StringLiterals: + EnforcedStyle: double_quotes + +# We do not need to support Ruby 1.9, so this is good to use. +Style/SymbolArray: + Enabled: true + +# Most readable form. +Style/AlignHash: + EnforcedHashRocketStyle: table + EnforcedColonStyle: table + +# Mixing the styles looks just silly. +# REVIEW: Enable once https://github.com/bbatsov/rubocop/commit/760ce1ed2cf10beda5e163f934c03a6fb6daa38e +# is released. +#Style/HashSyntax: +# EnforcedStyle: ruby19_no_mixed_keys + +# has_key? and has_value? are far more readable than key? and value? +Style/DeprecatedHashMethods: + Enabled: false + +# String#% is by far the least verbose and only object oriented variant. +Style/FormatString: + EnforcedStyle: percent + +Style/CollectionMethods: + Enabled: true + PreferredMethods: + # inject seems more common in the community. + reduce: "inject" + + +# Either allow this style or don't. Marking it as safe with parenthesis +# is silly. Let's try to live without them for now. +Style/ParenthesesAroundCondition: + AllowSafeAssignment: false +Lint/AssignmentInCondition: + AllowSafeAssignment: false + +# A specialized exception class will take one or more arguments and construct the message from it. +# So both variants make sense. +Style/RaiseArgs: + Enabled: false + +# Fail is an alias of raise. Avoid aliases, it's more cognitive load for no gain. +# The argument that fail should be used to abort the program is wrong too, +# there's Kernel#abort for that. +Style/SignalException: + EnforcedStyle: only_raise + +# Suppressing exceptions can be perfectly fine, and be it to avoid to +# explicitly type nil into the rescue since that's what you want to return, +# or suppressing LoadError for optional dependencies +Lint/HandleExceptions: + Enabled: false + +Style/SpaceInsideBlockBraces: + # The space here provides no real gain in readability while consuming + # horizontal space that could be used for a better parameter name. + # Also {| differentiates better from a hash than { | does. + SpaceBeforeBlockParameters: false + +# No trailing space differentiates better from the block: +# foo} means hash, foo } means block. +Style/SpaceInsideHashLiteralBraces: + EnforcedStyle: no_space + +# { ... } for multi-line blocks is okay, follow Weirichs rule instead: +# https://web.archive.org/web/20140221124509/http://onestepback.org/index.cgi/Tech/Ruby/BraceVsDoEnd.rdoc +Style/Blocks: + Enabled: false + +# do / end blocks should be used for side effects, +# methods that run a block for side effects and have +# a useful return value are rare, assign the return +# value to a local variable for those cases. +Style/MethodCalledOnDoEndBlock: + Enabled: true + +# Enforcing the names of variables? To single letter ones? Just no. +Style/SingleLineBlockParams: + Enabled: false + +# Shadowing outer local variables with block parameters is often useful +# to not reinvent a new name for the same thing, it highlights the relation +# between the outer variable and the parameter. The cases where it's actually +# confusing are rare, and usually bad for other reasons already, for example +# because the method is too long. +Lint/ShadowingOuterLocalVariable: + Enabled: false + +# Check with yard instead. +Style/Documentation: + Enabled: false + +# This is just silly. Calling the argument `other` in all cases makes no sense. +Style/OpMethod: + Enabled: false + +# There are valid cases, for example debugging Cucumber steps, +# also they'll fail CI anyway +Lint/Debugger: + Enabled: false + diff --git a/Gemfile b/Gemfile index 4ee3b3a0c..2439921ca 100644 --- a/Gemfile +++ b/Gemfile @@ -216,12 +216,14 @@ group :development do # Automatic test runs gem 'guard-cucumber', '1.5.3' gem 'guard-rspec', '4.5.0' + gem 'guard-rubocop', '1.2.0' gem 'guard', '2.12.4', :require => false gem 'rb-fsevent', '0.9.4', :require => false gem 'rb-inotify', '0.9.5', :require => false # Linters - gem 'jshint', '1.3.1' + gem 'jshint', '1.3.1' + gem 'rubocop', '0.29.1' # Preloading environment diff --git a/Gemfile.lock b/Gemfile.lock index 22d5c73b0..209f38680 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -62,6 +62,9 @@ GEM activemodel fog (>= 1.8.0) unf + ast (2.0.0) + astrolabe (1.3.0) + parser (>= 2.2.0.pre.3, < 3.0) autoprefixer-rails (5.1.7) execjs json @@ -318,6 +321,9 @@ GEM guard (~> 2.1) guard-compat (~> 1.1) rspec (>= 2.99.0, < 4.0) + guard-rubocop (1.2.0) + guard (~> 2.0) + rubocop (~> 0.20) haml (4.0.6) tilt handlebars_assets (0.20.1) @@ -439,7 +445,10 @@ GEM faraday (~> 0.9.0) nokogiri (~> 1.6) orm_adapter (0.5.0) + parser (2.2.0.3) + ast (>= 1.1, < 3.0) phantomjs (1.9.8.0) + powerpack (0.1.0) pry (0.10.1) coderay (~> 1.1.0) method_source (~> 0.8.1) @@ -547,6 +556,7 @@ GEM activesupport (= 4.2.0) rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) + rainbow (2.0.0) raindrops (0.13.0) rake (10.4.2) rb-fsevent (0.9.4) @@ -586,6 +596,12 @@ GEM rspec-mocks (~> 3.1.0) rspec-support (~> 3.1.0) rspec-support (3.1.2) + rubocop (0.29.1) + astrolabe (~> 1.3) + parser (>= 2.2.0.1, < 3.0) + powerpack (~> 0.1) + rainbow (>= 1.99.1, < 3.0) + ruby-progressbar (~> 1.4) ruby-oembed (0.8.12) ruby-progressbar (1.7.1) rubyzip (1.1.7) @@ -721,6 +737,7 @@ DEPENDENCIES guard (= 2.12.4) guard-cucumber (= 1.5.3) guard-rspec (= 4.5.0) + guard-rubocop (= 1.2.0) haml (= 4.0.6) handlebars_assets (= 0.20.1) http_accept_language (= 2.0.5) @@ -782,6 +799,7 @@ DEPENDENCIES roxml (= 3.1.6) rspec-instafail (= 0.2.6) rspec-rails (= 3.1.0) + rubocop (= 0.29.1) ruby-oembed (= 0.8.12) sass-rails (= 5.0.1) selenium-webdriver (= 2.45.0) diff --git a/Guardfile b/Guardfile index f366958ed..df294bb7f 100644 --- a/Guardfile +++ b/Guardfile @@ -1,24 +1,39 @@ -guard :rspec, cmd: 'bin/spring rspec', all_on_start: false, all_after_pass: false do - watch(%r{^spec/.+_spec\.rb$}) - watch(%r{^lib/(.+)\.rb$}) { |m| "spec/lib/#{m[1]}_spec.rb" } - watch('spec/spec_helper.rb') { "spec" } +guard :rspec, cmd: "bin/spring rspec", all_on_start: false, all_after_pass: false do + watch(/^spec\/.+_spec\.rb$/) + watch(/^lib\/(.+)\.rb$/) {|m| "spec/lib/#{m[1]}_spec.rb" } + watch(/spec\/spec_helper.rb/) { "spec" } # Rails example - watch(%r{^spec/.+_spec\.rb$}) - watch(%r{^app/(.+)\.rb$}) { |m| "spec/#{m[1]}_spec.rb" } - watch(%r{^lib/(.+)\.rb$}) { |m| "spec/lib/#{m[1]}_spec.rb" } - watch(%r{^app/controllers/(.+)_(controller)\.rb$}) { |m| ["spec/routing/#{m[1]}_routing_spec.rb", "spec/#{m[2]}s/#{m[1]}_#{m[2]}_spec.rb", "spec/acceptance/#{m[1]}_spec.rb"] } + watch(/^spec\/.+_spec\.rb$/) + watch(/^app\/(.+)\.rb$/) {|m| "spec/#{m[1]}_spec.rb" } + watch(/^lib\/(.+)\.rb$/) {|m| "spec/lib/#{m[1]}_spec.rb" } + watch(%r{^app/controllers/(.+)_(controller)\.rb$}) {|m| + ["spec/routing/#{m[1]}_routing_spec.rb", + "spec/#{m[2]}s/#{m[1]}_#{m[2]}_spec.rb", + "spec/acceptance/#{m[1]}_spec.rb"] + } watch(%r{^spec/support/(.+)\.rb$}) { "spec" } - watch('spec/spec_helper.rb') { "spec" } - watch('config/routes.rb') { "spec/routing" } - watch('app/controllers/application_controller.rb') { "spec/controllers" } + watch("spec/spec_helper.rb") { "spec" } + watch("config/routes.rb") { "spec/routing" } + watch("app/controllers/application_controller.rb") { "spec/controllers" } # Capybara request specs - watch(%r{^app/views/(.+)/.*\.(erb|haml)$}) { |m| "spec/requests/#{m[1]}_spec.rb" } + watch(%r{^app/views/(.+)/.*\.(erb|haml)$}) {|m| "spec/requests/#{m[1]}_spec.rb" } end -guard :cucumber, command_prefix: 'bin/spring', bundler: false, all_on_start: false, all_after_pass: false do - watch(%r{^features/.+\.feature$}) - watch(%r{^features/support/.+$}) { 'features' } - watch(%r{^features/step_definitions/(.+)_steps\.rb$}) { |m| Dir[File.join("**/#{m[1]}.feature")][0] || 'features' } +guard(:cucumber, + command_prefix: "bin/spring", + bundler: false, + all_on_start: false, + all_after_pass: false) do + watch(/^features\/.+\.feature$/) + watch(%r{^features/support/.+$}) { "features" } + watch(%r{^features/step_definitions/(.+)_steps\.rb$}) {|m| + Dir[File.join("**/#{m[1]}.feature")][0] || "features" + } +end + +guard :rubocop, all_on_start: false, keep_failed: false do + watch(/(?:app|config|db|lib|features|spec)\/.+\.rb$/) + watch(/(config.ru|Gemfile|Guardfile|Rakefile)$/) end