From bb171f7a00c9e2599f60f604fbfeb3c8bcf33be7 Mon Sep 17 00:00:00 2001 From: Ray Zane Date: Thu, 26 Sep 2024 14:24:13 -0400 Subject: [PATCH] Update `Layout` cops to be more consistent with Standard (#55) 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. --- Gemfile | 1 + Gemfile.lock | 16 +++- betterlint.gemspec | 2 +- config/default.yml | 36 ++++++++- .../cop/betterment/non_standard_actions.rb | 8 +- lib/rubocop/cop/betterment/render_status.rb | 8 +- lib/rubocop/cop/betterment/utils/parser.rb | 36 ++++----- spec/standard_compliance_spec.rb | 80 +++++++++++++++++++ spec/support/betterlint_config.rb | 6 +- 9 files changed, 158 insertions(+), 35 deletions(-) create mode 100644 spec/standard_compliance_spec.rb diff --git a/Gemfile b/Gemfile index 64066a4..d95b235 100644 --- a/Gemfile +++ b/Gemfile @@ -7,4 +7,5 @@ group :development do gem 'bundler' gem 'rspec-rails' gem 'rake', '>= 12.3.3' + gem 'standard' end diff --git a/Gemfile.lock b/Gemfile.lock index 597376b..dc8cf76 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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) @@ -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) @@ -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) @@ -186,6 +199,7 @@ DEPENDENCIES pry rake (>= 12.3.3) rspec-rails + standard BUNDLED WITH 2.4.22 diff --git a/betterlint.gemspec b/betterlint.gemspec index a5757f9..ecbde13 100644 --- a/betterlint.gemspec +++ b/betterlint.gemspec @@ -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" diff --git a/config/default.yml b/config/default.yml index e2bd6bc..de92e2c 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 @@ -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: diff --git a/lib/rubocop/cop/betterment/non_standard_actions.rb b/lib/rubocop/cop/betterment/non_standard_actions.rb index 35a71de..6c20243 100644 --- a/lib/rubocop/cop/betterment/non_standard_actions.rb +++ b/lib/rubocop/cop/betterment/non_standard_actions.rb @@ -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 diff --git a/lib/rubocop/cop/betterment/render_status.rb b/lib/rubocop/cop/betterment/render_status.rb index 3075ba7..5ecc080 100644 --- a/lib/rubocop/cop/betterment/render_status.rb +++ b/lib/rubocop/cop/betterment/render_status.rb @@ -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 diff --git a/lib/rubocop/cop/betterment/utils/parser.rb b/lib/rubocop/cop/betterment/utils/parser.rb index 4b5a4f1..65289ab 100644 --- a/lib/rubocop/cop/betterment/utils/parser.rb +++ b/lib/rubocop/cop/betterment/utils/parser.rb @@ -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 diff --git a/spec/standard_compliance_spec.rb b/spec/standard_compliance_spec.rb new file mode 100644 index 0000000..8e3d18b --- /dev/null +++ b/spec/standard_compliance_spec.rb @@ -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 diff --git a/spec/support/betterlint_config.rb b/spec/support/betterlint_config.rb index 55492fe..ecdeac7 100644 --- a/spec/support/betterlint_config.rb +++ b/spec/support/betterlint_config.rb @@ -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