Merge pull request #2418 from alphagov/use-auto-generated-rubocop-config-ready-to-merge

Use auto generated Rubocop config

Author: floehopper

.rubocop.yml

@@ -0,0 +1,5 @@
+inherit_from:
+  - .rubocop_todo.yml
+AllCops:
+  Exclude:
+    - 'tmp/**/*'

.rubocop_todo.yml

@@ -2,6 +2,6 @@
 
 class UkbaCountry < OpenStruct
   def self.all
-    @countries ||= YAML.load_file(Rails.root.join('lib', 'data', 'ukba_additional_countries.yml')).map {|c| self.new(c) }
+    @countries ||= YAML.load_file(Rails.root.join('lib', 'data', 'ukba_additional_countries.yml')).map { |c| self.new(c) }
   end
 end

app/models/ukba_country.rb

@@ -65,6 +65,6 @@ def organisations
   end
 
   def fco_organisation
-    self.organisations.find {|o| o.fco_sponsored? }
+    self.organisations.find { |o| o.fco_sponsored? }
   end
 end

app/models/world_location.rb

@@ -20,7 +20,7 @@ def all_offices
   end
 
   def fco_sponsored?
-    @data.sponsors.any? {|s| s.details.acronym == "FCO" }
+    @data.sponsors.any? { |s| s.details.acronym == "FCO" }
   end
 
   def offices_with_service(service_title)

app/models/worldwide_organisation.rb

@@ -1,6 +1,6 @@
 class CountrySelectQuestionPresenter < QuestionPresenter
   def response_label(value)
-    options.find {|option| option.slug == value}.name
+    options.find { |option| option.slug == value }.name
   end
 
   def options

app/presenters/country_select_question_presenter.rb

@@ -17,7 +17,7 @@ def end_date
     @node.range == false ? 3.years.from_now : @node.range.end
   end
 
-  private
+private
 
   def only_display_day_and_month?(value)
     value.year.zero?

app/presenters/date_question_presenter.rb

@@ -1,5 +1,5 @@
 class MultipleChoiceQuestionPresenter < QuestionPresenter
   def response_label(value)
-    options.find {|option| option.value == value}.label
+    options.find { |option| option.value == value }.label
   end
 end

app/presenters/multiple_choice_question_presenter.rb

@@ -3,7 +3,7 @@
 if Rails.env.production?
   FLOW_REGISTRY_OPTIONS = {}
 else
-  FLOW_REGISTRY_OPTIONS = {show_drafts: true}
+  FLOW_REGISTRY_OPTIONS = { show_drafts: true }
 end
 
 # Uncomment the following to run smartanswers with the test flows instead of the real ones

config/initializers/flow_preview.rb

@@ -1,14 +1,22 @@
 # Rubocop
 
-We're using the govuk-lint Gem to include Rubocop and the GOV.UK styleguide rules in the project.
+We're using the `govuk-lint` Gem to include Rubocop and the GOV.UK styleguide rules in the project.
 
 ### Jenkins
 
-We run Rubocop as part of the test suite executed on Jenkins (see jenkins.sh). Rubocop only tests for violations in files introduced in the branch being tested. This should prevent us from introducing new violations without us having to first fix all existing violations.
+We run `govuk-lint-ruby` as part of the test suite executed on Jenkins (see jenkins.sh). Behind the scenes this runs Rubocop with a set of cops defined in the `govuk-lint` gem. However, because `govuk-lint` was retro-fitted to the application relatively recently, there were a large number of cop violations in the codebase.
+
+We've dealt with this by generating a project-specific `.rubocop_todo.yml` file using `govuk-lint-ruby --auto-gen-config --exclude-limit 99999` which [excludes existing violations on a per-cop basis][1]. The limit was set to `99999` to minimise the number of cops that are disabled entirely - most are only disabled for a specific set of files.
+
+The generated `.rubocop_todo.yml` file is included into the configuration via a local `.rubocop.yml` file using an `inherit_from` key. This means that the exclusions in `.rubocop_todo.yml` are used for all `govuk-lint-ruby` commands by default. Note that the `.rubocop.yml` file does not override the cop definitions in the `govuk-lint` gem - it is merely a way to add extra configuration.
+
+Using this approach should prevent us from introducing new violations into the project without needing to fix all the existing violations first. However, in the long run, the idea is for us to gradually remove these exclusions to bring the project fully in line with the default `govuk-lint-ruby` configuration. Ultimately we will be able to remove the `.rubocop_todo.yml` & `.rubocop.yml` files and just rely on the default `govuk-lint` configuration.
+
+Note that as per the header comment in  `.rubocop_todo.yml`, changes in the inspected code, or installation of new versions of RuboCop, may require `.rubocop_todo.yml` to be generated again.
 
 ### Running locally
 
-Testing for violations in the entire codebase:
+Testing for violations in the entire codebase (used by `jenkins.sh`):
 
 ```bash
 $ govuk-lint-ruby
@@ -26,4 +34,4 @@ Testing for violations in code staged and committed locally that's not present i
 $ govuk-lint-ruby --diff --cached
 ```
 
-NOTE. This is mostly useful for Jenkins as we first merge, but don't commit, changes from master before running the test suite.
+[1]: https://github.com/bbatsov/rubocop/blob/master/README.md#automatically-generated-configuration

doc/rubocop.md

@@ -26,7 +26,6 @@ export DISPLAY=:99
 
 if [ -z "$RUN_REGRESSION_TESTS" ]; then
   bundle exec govuk-lint-ruby \
-    --diff --cached \
     --format clang
 
   RAILS_ENV=test TEST_COVERAGE=true bundle exec rake test

jenkins.sh

@@ -28,7 +28,7 @@ def find_date
   private
 
   def run(pension_dates)
-    pension_dates.find {|p| p.match?(dob, gender)}
+    pension_dates.find { |p| p.match?(dob, gender) }
   end
 
   # Handle the case where the person's d.o.b. is 29th Feb

lib/data/state_pension_date_query.rb

@@ -4,7 +4,7 @@ def initialize(flow)
   end
 
   def labels
-    Hash[@flow.nodes.map { |node| [node.name, graph_label_text(node)]}]
+    Hash[@flow.nodes.map { |node| [node.name, graph_label_text(node)] }]
   end
 
   def adjacency_list

lib/graph_presenter.rb

@@ -32,7 +32,7 @@ def label_lines
           style: "filled"
         )
       end
-      attribute_clause = attrs.map {|k, v| "#{k}=\"#{v}\""}.join(' ')
+      attribute_clause = attrs.map { |k, v| "#{k}=\"#{v}\"" }.join(' ')
       %Q{#{normalize_name(name)} [#{attribute_clause}]}
     end
   end

lib/graphviz_presenter.rb

@@ -22,7 +22,7 @@ def has_commodity_code?
       commodity_code != 'X'
     end
 
-    private
+  private
 
     def glucose_sucrose_index
       starch_glucose_to_sucrose[@starch_glucose_weight][@sucrose_weight]

lib/smart_answer/calculators/commodity_code_calculator.rb

@@ -12,7 +12,7 @@ class CountryNameFormatter
       "usa" => "the USA"
     }
 
-    def definitive_article(country, capitalized=false)
+    def definitive_article(country, capitalized = false)
       result = country_name(country)
       if requires_definite_article?(country)
         result = capitalized ? "The #{result}" : "the #{result}"
@@ -32,7 +32,7 @@ def friendly_name(country)
       FRIENDLY_COUNTRY_NAME[country]
     end
 
-    private
+  private
 
     def country_name(country)
       WorldLocation.find(country).name

lib/smart_answer/calculators/country_name_formatter.rb

@@ -13,10 +13,10 @@ def included_country?
       postcode_within?(VALID_COUNTRIES, 'country_name')
     end
 
-    private
+  private
 
     def postcode_within?(included_areas, key_name)
-      areas_for_postcode.select {|a| included_areas.include?(a[key_name]) }.any?
+      areas_for_postcode.select { |a| included_areas.include?(a[key_name]) }.any?
     end
 
     def areas_for_postcode

lib/smart_answer/calculators/landlord_immigration_check_calculator.rb

@@ -82,7 +82,7 @@ def ss_alt_fees_table_country?(country_slug, calculator)
       SS_ALT_FEES_TABLE_COUNTRY.include?(country_slug) ||
         (SS_ALT_FEES_TABLE_OR_OUTCOME_GROUP_A.include?(country_slug) && calculator.partner_british?) ||
         (SS_ALT_FEES_TABLE_OR_OUTCOME_GROUP_B.include?(country_slug) && calculator.partner_is_not_national_of_ceremony_country?) &&
-        (%w(cambodia vietnam).exclude?(country_slug))
+          (%w(cambodia vietnam).exclude?(country_slug))
     end
 
     def ss_marriage_not_possible?(country_slug, calculator)

lib/smart_answer/calculators/marriage_abroad_data_query.rb

@@ -42,7 +42,7 @@ def smp_lel
       end
     end
 
-    private
+  private
 
     def due_date_before_7th_april_2013?
       @due_date < Date.parse("7th April 2013")

lib/smart_answer/calculators/maternity_benefits_calculator.rb

@@ -190,7 +190,7 @@ def any_overtime_hours_worked?
       overtime_hours > 0
     end
 
-    protected
+  protected
 
     def weekly_multiplier
       (@pay_frequency.to_f / 7).round(3)

lib/smart_answer/calculators/minimum_wage_calculator.rb

@@ -60,7 +60,7 @@ def pro_rata_taxable_profit
         (profit_per_day * award_period.number_of_days).floor
       end
 
-      private
+    private
 
       def accounting_year_end_date_in_the_tax_year_that_tax_credits_award_ends
         accounting_date = accounts_end_month_and_day.change(year: tax_year.begins_on.year)

lib/smart_answer/calculators/part_year_profit_tax_credits_calculator.rb

@@ -92,7 +92,7 @@ def in_2015_2016_fin_year?(date)
       (Date.new(2015, 05, 06)..Date.new(2016, 05, 05)).cover?(date)
     end
 
-    private
+  private
 
     def saturday_before(date)
       (date - date.wday) - 1.day

lib/smart_answer/calculators/pay_leave_for_parents_calculator.rb

@@ -59,7 +59,7 @@ def period_of_additional_leave
       period_of_ordinary_leave && weeks_later(period_of_ordinary_leave, 26)
     end
 
-    private
+  private
       def weeks_later(range, weeks)
         (range.first + weeks * 7) .. (range.last + weeks * 7)
       end

lib/smart_answer/calculators/plan_adoption_leave.rb

@@ -54,7 +54,7 @@ def period_of_additional_leave
       period_of_ordinary_leave && weeks_later(period_of_ordinary_leave, 26)
     end
 
-    private
+  private
       def weeks_later(range, weeks)
         (range.first + weeks * 7) .. (range.last + weeks * 7)
       end

lib/smart_answer/calculators/plan_maternity_leave.rb

@@ -80,7 +80,7 @@ def late_payment_penalty
       end
     end
 
-    private
+  private
     def overdue_filing_days
       (filing_date - filing_deadline).to_i
     end

lib/smart_answer/calculators/self_assessment_penalties.rb

@@ -15,7 +15,7 @@ def lump_sum_and_age(dob, weekly_amount, gender)
       age = age_at_date(dob, TOPUP_START_DATE)
       (TOPUP_START_DATE.year..TOPUP_END_DATE.year).each do |_|
         break if birthday_after_topup_end?(dob, age)
-        rows << {amount: lump_sum_amount(age, weekly_amount), age: age} if age >= retirement_age(gender)
+        rows << { amount: lump_sum_amount(age, weekly_amount), age: age } if age >= retirement_age(gender)
         age += 1
       end
       rows

lib/smart_answer/calculators/state_pension_topup_calculator.rb

@@ -51,7 +51,7 @@ def funds_received_by
       end
     end
 
-    private
+  private
 
     def end_of_month_after(date)
       1.month.since(date).end_of_month

lib/smart_answer/calculators/vat_payment_deadlines.rb

@@ -38,7 +38,7 @@ def erb_template_path
       @template_directory.join(erb_template_name)
     end
 
-    private
+  private
 
     def erb_template_name
       "#{@template_name}.govspeak.erb"

lib/smart_answer/erb_renderer.rb

@@ -95,11 +95,11 @@ def questions
     end
 
     def node_exists?(node_or_name)
-      @nodes.any? {|n| n.name == node_or_name.to_sym }
+      @nodes.any? { |n| n.name == node_or_name.to_sym }
     end
 
     def node(node_or_name)
-      @nodes.find {|n| n.name == node_or_name.to_sym } or raise "Node '#{node_or_name}' does not exist"
+      @nodes.find { |n| n.name == node_or_name.to_sym } or raise "Node '#{node_or_name}' does not exist"
     end
 
     def start_state
@@ -131,7 +131,7 @@ def normalize_responses(responses)
 
     class InvalidStatus < StandardError; end
 
-    private
+  private
 
     def add_question(klass, name, options = {}, &block)
       add_node klass.new(self, name, options, &block)

lib/smart_answer/flow.rb

@@ -31,7 +31,7 @@ def parse_input(raw_input)
       end
 
       def to_response(input)
-        input.split(',').reject {|v| v == NONE_OPTION }
+        input.split(',').reject { |v| v == NONE_OPTION }
       end
     end
   end

lib/smart_answer/question/checkbox.rb

@@ -21,7 +21,7 @@ def country_in(countries)
       end
 
       def valid_option?(option)
-        options.map {|v| v.slug}.include? (option.to_s)
+        options.map { |v| v.slug }.include? (option.to_s)
       end
 
       def parse_input(raw_input)
@@ -40,7 +40,7 @@ def load_countries
           countries = countries.reject { |c| @exclude_countries.include?(c.slug) }
         end
         if @additional_countries
-          countries = (countries + @additional_countries).sort {|a, b| a.name <=> b.name }
+          countries = (countries + @additional_countries).sort { |a, b| a.name <=> b.name }
         end
         countries
       end

lib/smart_answer/question/country_select.rb

@@ -52,23 +52,25 @@ def range
       end
 
       def parse_input(input)
-        date = case input
+        date = begin
+          case input
           when Hash, ActiveSupport::HashWithIndifferentAccess
-           input = input.symbolize_keys
-           year_month_and_day = [
-             default_year || input[:year],
-             default_month || input[:month],
-             default_day || input[:day],
-           ]
-           raise InvalidResponse unless year_month_and_day.all?(&:present?)
-           ::Date.new(*year_month_and_day.map(&:to_i))
+            input = input.symbolize_keys
+            year_month_and_day = [
+              default_year || input[:year],
+              default_month || input[:month],
+              default_day || input[:day],
+            ]
+            raise InvalidResponse unless year_month_and_day.all?(&:present?)
+            ::Date.new(*year_month_and_day.map(&:to_i))
           when String
-           ::Date.parse(input)
+            ::Date.parse(input)
           when ::Date
-           input
+            input
           else
-           raise InvalidResponse, "Bad date", caller
+            raise InvalidResponse, "Bad date", caller
           end
+        end
         validate_input(date) if @validate_in_range
         date
       rescue ArgumentError => e

lib/smart_answer/question/date.rb

@@ -36,7 +36,7 @@ def save_input_as(name)
       __send__ "#{name}=", responses.last
     end
 
-    private
+  private
 
     def initialize_copy(orig)
       super

lib/smart_answer/state.rb

@@ -18,7 +18,7 @@ def empty?
     existing_paths.empty?
   end
 
-  private
+private
 
   def relative_paths
     @relative_paths ||= all_paths.collect do |path|

lib/smart_answer_files.rb

@@ -72,7 +72,7 @@ def define
         validate(:error_perm_prop_house) { |r| ! r.include?('permission,property,social_housing') }
         validate(:error_prop_house) { |r| ! r.include?('property,social_housing') }
         validate(:error_perm_prop) { |r| ! r.include?('permission,property') }
-        validate(:error_perm_house) { |r| ! r.include?('permission,social_housing')}
+        validate(:error_perm_house) { |r| ! r.include?('permission,social_housing') }
 
         next_node do
           question :date_of_birth? # Q3
@@ -163,9 +163,9 @@ def define
             response == 'esa' ||
             response == 'working_tax_credit' ||
             response.include?('universal_credit') ||
-            %w{child_tax_credit esa income_support jsa pension_credit}.all? {|key| response.include? key} ||
-            %w{child_tax_credit esa income_support pension_credit}.all? {|key| response.include? key} ||
-            %w{child_tax_credit esa jsa pension_credit}.all? {|key| response.include? key}
+            %w{child_tax_credit esa income_support jsa pension_credit}.all? { |key| response.include? key } ||
+            %w{child_tax_credit esa income_support pension_credit}.all? { |key| response.include? key } ||
+            %w{child_tax_credit esa jsa pension_credit}.all? { |key| response.include? key }
         end
 
         next_node do |response|