Skip to content

Commit

Permalink
Update Layout cops to be more consistent with Standard (#55)
Browse files Browse the repository at this point in the history
In an effort to more closely adhere to
https://github.com/standardrb/standard, I've added a spec to detect the
introduction of new rules that don't comply with Standard.

I've updated the layout cops to more closely match the rules in
standard.
  • Loading branch information
rzane authored Sep 26, 2024
1 parent 4a773b0 commit bb171f7
Show file tree
Hide file tree
Showing 9 changed files with 158 additions and 35 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ group :development do
gem 'bundler'
gem 'rspec-rails'
gem 'rake', '>= 12.3.3'
gem 'standard'
end
16 changes: 15 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
betterlint (1.13.0)
betterlint (1.14.0)
rubocop (~> 1.62.0)
rubocop-graphql (~> 1.5.0)
rubocop-performance (~> 1.21.0)
Expand Down Expand Up @@ -57,6 +57,7 @@ GEM
reline (>= 0.3.8)
json (2.7.1)
language_server-protocol (3.17.0.3)
lint_roller (1.1.0)
loofah (2.22.0)
crass (~> 1.0.2)
nokogiri (>= 1.12.0)
Expand Down Expand Up @@ -165,6 +166,18 @@ GEM
rubocop-rspec_rails (2.28.2)
rubocop (~> 1.40)
ruby-progressbar (1.13.0)
standard (1.35.1)
language_server-protocol (~> 3.17.0.2)
lint_roller (~> 1.0)
rubocop (~> 1.62.0)
standard-custom (~> 1.0.0)
standard-performance (~> 1.3)
standard-custom (1.0.2)
lint_roller (~> 1.0)
rubocop (~> 1.50)
standard-performance (1.4.0)
lint_roller (~> 1.1)
rubocop-performance (~> 1.21.0)
stringio (3.1.0)
strscan (3.1.0)
thor (1.3.0)
Expand All @@ -186,6 +199,7 @@ DEPENDENCIES
pry
rake (>= 12.3.3)
rspec-rails
standard

BUNDLED WITH
2.4.22
2 changes: 1 addition & 1 deletion betterlint.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)

Gem::Specification.new do |s|
s.name = "betterlint"
s.version = "1.13.0"
s.version = "1.14.0"
s.authors = ["Development"]
s.email = ["development@betterment.com"]
s.summary = "Betterment rubocop configuration"
Expand Down
36 changes: 32 additions & 4 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,43 @@ FactoryBot/ConsistentParenthesesStyle:
FactoryBot/SyntaxMethods:
Enabled: false

Layout/ArgumentAlignment:
EnforcedStyle: with_fixed_indentation

Layout/ArrayAlignment:
EnforcedStyle: with_fixed_indentation

Layout/CaseIndentation:
IndentOneStep: true
EnforcedStyle: end
IndentOneStep: false

Layout/ClosingParenthesisIndentation:
Enabled: true

Layout/EndAlignment:
EnforcedStyleAlignWith: variable

Layout/FirstArgumentIndentation:
EnforcedStyle: consistent

Layout/FirstArrayElementIndentation:
EnforcedStyle: consistent

Layout/FirstHashElementIndentation:
EnforcedStyle: consistent

Layout/FirstParameterIndentation:
Enabled: false

Layout/LineContinuationLeadingSpace:
Enabled: false

Layout/LineContinuationSpacing:
Enabled: true

Layout/LineEndStringConcatenationIndentation:
Enabled: false

Layout/LineLength:
Max: 140

Expand All @@ -120,11 +148,11 @@ Layout/MultilineOperationIndentation:
EnforcedStyle: indented

Layout/ParameterAlignment:
Enabled: false
Enabled: true
EnforcedStyle: with_fixed_indentation

# Disabling because of a bug in rubocop: https://github.com/rubocop-hq/rubocop/issues/6918
Layout/RescueEnsureAlignment:
Enabled: false
Enabled: true

Lint/AmbiguousBlockAssociation:
Exclude:
Expand Down
8 changes: 4 additions & 4 deletions lib/rubocop/cop/betterment/non_standard_actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ def check_raw_route(node)
if route
(path, param, value) = route
action = case param
when :to then value.first.split('#').last
when :action then value.first
else path
end
when :to then value.first.split('#').last
when :action then value.first
else path
end
add_offense(node, message: MSG_ROUTE_TO) unless allowed_action?(action)
true
end
Expand Down
8 changes: 4 additions & 4 deletions lib/rubocop/cop/betterment/render_status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ def on_def(node)

def infer_status(responder)
case extract_template(responder).to_s
when 'new', 'edit'
:unprocessable_entity
else
:ok
when 'new', 'edit'
:unprocessable_entity
else
:ok
end
end

Expand Down
36 changes: 18 additions & 18 deletions lib/rubocop/cop/betterment/utils/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,25 @@ def self.get_return_values(node)
return [node] if node.literal? || node.variable?

case node.type
when :begin
get_return_values(node.children.last)
when :block
get_return_values(node.body)
when :if
if_rets = get_return_values(node.if_branch)
else_rets = get_return_values(node.else_branch)
if_rets + else_rets
when :case
cases = []
node.each_when do |block|
cases += get_return_values(block.body)
end
when :begin
get_return_values(node.children.last)
when :block
get_return_values(node.body)
when :if
if_rets = get_return_values(node.if_branch)
else_rets = get_return_values(node.else_branch)
if_rets + else_rets
when :case
cases = []
node.each_when do |block|
cases += get_return_values(block.body)
end

cases + get_return_values(node.else_branch)
when :send
[node]
else
[]
cases + get_return_values(node.else_branch)
when :send
[node]
else
[]
end
end

Expand Down
80 changes: 80 additions & 0 deletions spec/standard_compliance_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe 'standard compliance' do
let(:exceptions) do
%w(
Layout/LineLength
Lint/AmbiguousBlockAssociation
Lint/AmbiguousOperator
Lint/AmbiguousRegexpLiteral
Lint/BooleanSymbol
Metrics/AbcSize
Metrics/ClassLength
Metrics/CyclomaticComplexity
Metrics/ModuleLength
Metrics/ParameterLists
Metrics/PerceivedComplexity
Naming/PredicateName
Naming/VariableNumber
Style/BlockDelimiters
Style/FrozenStringLiteralComment
Style/LambdaCall
Style/MissingElse
Style/NumberedParameters
Style/PercentLiteralDelimiters
Style/SafeNavigation
Style/StringLiterals
Style/TrailingCommaInArguments
Style/TrailingCommaInArrayLiteral
Style/TrailingCommaInHashLiteral
Style/YodaCondition
)
end

let(:default_config) do
root = Gem.loaded_specs['rubocop'].full_gem_path
path = File.expand_path('config/default.yml', root)
config = YAML.load_file(path, permitted_classes: [Regexp, Symbol])
config.delete('AllCops')
config
end

let(:standard_config) do
root = Gem.loaded_specs['standard'].full_gem_path
path = File.expand_path('config/base.yml', root)
YAML.load_file(path)
end

let(:betterlint_config) do
YAML.load_file('config/default.yml')
end

it 'complies with standardrb with notable exceptions' do
violations = betterlint_config.filter_map do |name, betterlint|
default = default_config[name] or next
standard = standard_config.fetch(name, {})

betterlint = default.merge(betterlint)
standard = default.merge(standard)

name if betterlint != standard
end

new_violations = (violations - exceptions).sort
old_exceptions = (exceptions - violations).sort

expect(new_violations).to be_empty, <<~MSG
All new rules should match Standard. These following rules are invalid:
#{new_violations.join("\n")}
MSG

expect(old_exceptions).to be_empty, <<~MSG
The following rules are now compliant with Standard and can be removed from the exceptions:
#{old_exceptions.join("\n")}
MSG
end
end
6 changes: 3 additions & 3 deletions spec/support/betterlint_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
.default_configuration.for_cop(cop_class)
.merge(betterlint_config.for_cop(cop_class))
.merge({
'Enabled' => true, # in case it is 'pending'
'AutoCorrect' => true, # in case defaults set it to false
})
'Enabled' => true, # in case it is 'pending'
'AutoCorrect' => true, # in case defaults set it to false
})
.merge(cop_config)
end
end

0 comments on commit bb171f7

Please sign in to comment.