Merge pull request #2124 from alphagov/dependabot/bundler/rubocop-gov…

…uk-4.0.0

Bump rubocop-govuk from 3.17.2 to 4.0.0

Gemfile.lock

@@ -64,7 +64,7 @@ GEM
     ansi (1.5.0)
     archive-zip (0.12.0)
       io-like (~> 0.3.0)
-    ast (2.4.1)
+    ast (2.4.2)
     better_errors (2.7.1)
       coderay (>= 1.0.0)
       erubi (>= 1.0.0)
@@ -199,8 +199,8 @@ GEM
       mini_portile2 (~> 2.5.0)
       racc (~> 1.4)
     null_logger (0.0.1)
-    parallel (1.19.2)
-    parser (2.7.2.0)
+    parallel (1.20.1)
+    parser (3.0.1.1)
       ast (~> 2.4.1)
     phantomjs (2.1.1.0)
     plek (4.0.0)
@@ -265,32 +265,33 @@ GEM
     rexml (3.2.5)
     robotex (1.0.0)
     rouge (3.26.0)
-    rubocop (0.87.1)
+    rubocop (1.15.0)
       parallel (~> 1.10)
-      parser (>= 2.7.1.1)
+      parser (>= 3.0.0.0)
       rainbow (>= 2.2.2, < 4.0)
-      regexp_parser (>= 1.7)
+      regexp_parser (>= 1.8, < 3.0)
       rexml
-      rubocop-ast (>= 0.1.0, < 1.0)
+      rubocop-ast (>= 1.5.0, < 2.0)
       ruby-progressbar (~> 1.7)
-      unicode-display_width (>= 1.4.0, < 2.0)
-    rubocop-ast (0.8.0)
-      parser (>= 2.7.1.5)
-    rubocop-govuk (3.17.2)
-      rubocop (= 0.87.1)
-      rubocop-ast (= 0.8.0)
-      rubocop-rails (= 2.8.1)
+      unicode-display_width (>= 1.4.0, < 3.0)
+    rubocop-ast (1.6.0)
+      parser (>= 3.0.1.1)
+    rubocop-govuk (4.0.0)
+      rubocop (~> 1.15.0)
+      rubocop-ast (~> 1.6.0)
+      rubocop-rails (~> 2.10.0)
       rubocop-rake (= 0.5.1)
-      rubocop-rspec (= 1.42.0)
-    rubocop-rails (2.8.1)
+      rubocop-rspec (~> 2.3.0)
+    rubocop-rails (2.10.1)
       activesupport (>= 4.2.0)
       rack (>= 1.1)
-      rubocop (>= 0.87.0)
+      rubocop (>= 1.7.0, < 2.0)
     rubocop-rake (0.5.1)
       rubocop
-    rubocop-rspec (1.42.0)
-      rubocop (>= 0.87.0)
-    ruby-progressbar (1.10.1)
+    rubocop-rspec (2.3.0)
+      rubocop (~> 1.0)
+      rubocop-ast (>= 1.1.0)
+    ruby-progressbar (1.11.0)
     ruby2_keywords (0.0.4)
     rubyzip (2.3.0)
     safe_yaml (1.0.5)
@@ -338,7 +339,7 @@ GEM
     unf (0.1.4)
       unf_ext
     unf_ext (0.0.7.7)
-    unicode-display_width (1.7.0)
+    unicode-display_width (2.0.0)
     unicorn (5.8.0)
       kgio (~> 2.6)
       raindrops (~> 0.7)

app/controllers/application_controller.rb

@@ -18,6 +18,6 @@ def content_item_path
                                  .compact
                                  .join(".")
 
-    "/" + URI.encode_www_form_component(path_and_optional_locale).gsub("%2F", "/")
+    "/#{URI.encode_www_form_component(path_and_optional_locale).gsub('%2F', '/')}"
   end
 end

app/controllers/content_items_controller.rb

@@ -173,8 +173,8 @@ def service_url(original_url)
     url.to_s
   end
 
-  def with_locale
-    I18n.with_locale(@content_item.locale || I18n.default_locale) { yield }
+  def with_locale(&block)
+    I18n.with_locale(@content_item.locale || I18n.default_locale, &block)
   end
 
   def error_403(exception)

app/helpers/typography_helper.rb

@@ -3,10 +3,10 @@ def nbsp_between_last_two_words(text)
     return text if text.nil?
 
     escaped_text = html_escape_once(text.strip)
-    escaped_text.sub(/\s([\w\.\?\!\:]+)$/, ' \1').html_safe
+    escaped_text.sub(/\s([\w.?!:]+)$/, ' \1').html_safe
   end
 
   def strip_trailing_colons(text)
-    text.gsub(/\:$/, "")
+    text.gsub(/:$/, "")
   end
 end

app/presenters/consultation_presenter.rb

@@ -24,7 +24,7 @@ def opening_date_midnight?
   end
 
   def closing_date
-    display_date_and_time(closing_date_time, true)
+    display_date_and_time(closing_date_time, rollback_midnight: true)
   end
 
   def open?
@@ -118,16 +118,16 @@ def add_margin?
 
 private
 
-  def display_date_and_time(date, rollback_midnight = false)
+  def display_date_and_time(date, rollback_midnight: false)
     time = Time.zone.parse(date)
     date_format = "%-e %B %Y"
     time_format = "%l:%M%P"
 
-    if rollback_midnight
+    if rollback_midnight && (time.strftime(time_format) == "12:00am")
       # 12am, 12:00am and "midnight on" can all be misinterpreted
       # Use 11:59pm on the day before to remove ambiguity
       # 12am on 10 January becomes 11:59pm on 9 January
-      time -= 1.second if time.strftime(time_format) == "12:00am"
+      time -= 1.second
     end
     I18n.l(time, format: "#{time_format} on #{date_format}").gsub(":00", "").gsub("12pm", "midday").gsub("12am on ", "").strip
   end

app/presenters/content_item/brexit_hub_page.rb

@@ -20,7 +20,7 @@ def brexit_links
     end
 
     def brexit_link
-      brexit_links[content_item.dig("content_id")]
+      brexit_links[content_item["content_id"]]
     end
   end
 end

app/presenters/content_item/news_image.rb

@@ -13,7 +13,7 @@ def default_news_image
 
     def placeholder_image
       # this image has been uploaded to asset-manager
-      if content_item.dig("document_type") == "world_news_story"
+      if content_item["document_type"] == "world_news_story"
         { "url" => "https://assets.publishing.service.gov.uk/media/5e985599d3bf7f3fc943bbd8/UK_government_logo.jpg" }
       else
         { "url" => "https://assets.publishing.service.gov.uk/media/5e59279b86650c53b2cefbfe/placeholder.jpg" }

app/presenters/content_item_presenter.rb

@@ -66,7 +66,7 @@ def web_url
 
   def canonical_url
     if requesting_a_part?
-      web_url + "/" + part_slug
+      "#{web_url}/#{part_slug}"
     else
       web_url
     end

app/presenters/guide_presenter.rb

@@ -46,8 +46,8 @@ def show_guide_navigation?
   end
 
   def content_title
-    if parts.any?
-      return current_part_title if hide_chapter_navigation?
+    if parts.any? && hide_chapter_navigation?
+      return current_part_title
     end
 
     title

app/presenters/machine_readable/guide_faq_page_schema_presenter.rb

@@ -56,7 +56,7 @@ def part_url(part, index)
       if index.zero?
         guide_url
       else
-        guide_url + "/" + part["slug"]
+        "#{guide_url}/#{part['slug']}"
       end
     end
 

app/presenters/machine_readable/yaml_faq_page_schema_presenter.rb

@@ -7,7 +7,7 @@ def self.configured?(content_item)
     end
 
     def self.config_path(content_item)
-      CONFIG_PATH.join(content_item.slug + ".yml").to_s
+      CONFIG_PATH.join("#{content_item.slug}.yml").to_s
     end
 
     def initialize(content_item)

app/presenters/service_sign_in/paths.rb

@@ -1,7 +1,7 @@
 module ServiceSignIn
   module Paths
     def path
-      content_item["base_path"] + "/" + content_item["details"][page_type]["slug"]
+      "#{content_item['base_path']}/#{content_item['details'][page_type]['slug']}"
     end
 
     def has_valid_path?

app/services/presenter_builder.rb

@@ -38,7 +38,7 @@ def presenter_name
       return service_sign_in_presenter_name
     end
 
-    content_item["schema_name"].classify + "Presenter"
+    "#{content_item['schema_name'].classify}Presenter"
   end
 
   def service_sign_in_format?
@@ -66,6 +66,8 @@ def initialize(content_item)
       @content_item = content_item
     end
   end
+
   class SpecialRouteReturned < StandardError; end
+
   class GovernmentReturned < StandardError; end
 end

config/environments/production.rb

@@ -75,7 +75,7 @@
   # config.logger = ActiveSupport::TaggedLogging.new(Syslog::Logger.new 'app-name')
 
   if ENV["RAILS_LOG_TO_STDOUT"].present?
-    logger           = ActiveSupport::Logger.new(STDOUT)
+    logger           = ActiveSupport::Logger.new($stdout)
     logger.formatter = config.log_formatter
     config.logger    = ActiveSupport::TaggedLogging.new(logger)
   end

test/controllers/content_items_controller_test.rb

@@ -294,7 +294,7 @@ class ContentItemsControllerTest < ActionController::TestCase
   test "returns 404 for invalid url" do
     path = "foreign-travel-advice/egypt]"
 
-    stub_content_store_does_not_have_item("/" + path)
+    stub_content_store_does_not_have_item("/#{path}")
 
     get :show, params: { path: path }
     assert_response :not_found
@@ -303,7 +303,7 @@ class ContentItemsControllerTest < ActionController::TestCase
   test "returns 404 for item not in content store" do
     path = "government/case-studies/boost-chocolate-production"
 
-    stub_content_store_does_not_have_item("/" + path)
+    stub_content_store_does_not_have_item("/#{path}")
 
     get :show, params: { path: path }
     assert_response :not_found
@@ -323,7 +323,7 @@ class ContentItemsControllerTest < ActionController::TestCase
 
   test "returns 403 for access-limited item" do
     path = "government/case-studies/super-sekrit-document"
-    url = content_store_endpoint + "/content/" + path
+    url = "#{content_store_endpoint}/content/#{path}"
     stub_request(:get, url).to_return(status: 403, headers: {})
 
     get :show, params: { path: path }

test/controllers/step_navigation_controller_test.rb

@@ -7,7 +7,7 @@ class ContentItemsControllerTest < ActionController::TestCase
     test "#{schema_name} shows step by step navigation where relevant" do
       content_item = content_store_has_schema_example(schema_name, "#{schema_name}-with-step-navs")
       content_item["base_path"] = "/pass-plus"
-      path = content_item["base_path"][1..-1]
+      path = content_item["base_path"][1..]
 
       stub_content_store_has_item(content_item["base_path"], content_item)
 
@@ -22,7 +22,7 @@ class ContentItemsControllerTest < ActionController::TestCase
     test "#{schema_name} does not show step by step navigation where relevant" do
       content_item = content_store_has_schema_example(schema_name, schema_name)
       content_item["base_path"] = "/not-part-of-a-step-by-step"
-      path = content_item["base_path"][1..-1]
+      path = content_item["base_path"][1..]
 
       stub_content_store_has_item(content_item["base_path"], content_item)
 

test/integration/answer_test.rb

@@ -17,7 +17,7 @@ class AnswerTest < ActionDispatch::IntegrationTest
     first_related_link = @content_item["details"]["external_related_links"].first
 
     within(".gem-c-related-navigation") do
-      assert page.has_css?('.gem-c-related-navigation__section-link--other[href="' + first_related_link["url"] + '"]', text: first_related_link["title"])
+      assert page.has_css?(".gem-c-related-navigation__section-link--other[href=\"#{first_related_link['url']}\"]", text: first_related_link["title"])
     end
   end
 

test/integration/service_sign_in/create_new_account_test.rb

@@ -36,7 +36,7 @@ class CreateNewAccount < ActionDispatch::IntegrationTest
 
     def setup_and_visit_create_new_account_page
       content_item = get_content_example("service_sign_in")
-      path = content_item["base_path"] + "/create-new-account"
+      path = "#{content_item['base_path']}/create-new-account"
       stub_content_store_has_item(path, content_item.to_json)
       visit(path)
     end

test/integration/specialist_document_test.rb

@@ -135,7 +135,7 @@ class SpecialistDocumentTest < ActionDispatch::IntegrationTest
   def assert_nested_content_item(heading)
     heading_level = heading["level"]
     selector = "a[href=\"##{heading['id']}\"]"
-    text = heading["text"].gsub(/\:$/, "")
+    text = heading["text"].gsub(/:$/, "")
 
     if heading_level < 4
       assert page.has_css?(selector), "Failed to find an element matching: #{selector}"

test/presenters/content_item/parts_test.rb

@@ -91,7 +91,7 @@ def part_slug
       end
 
       def requested_path
-        base_path + "/second-slug"
+        "#{base_path}/second-slug"
       end
     end
 
@@ -105,7 +105,7 @@ def part_slug
       end
 
       def requested_path
-        base_path + "/" + part_slug
+        "#{base_path}/#{part_slug}"
       end
     end
 
@@ -121,7 +121,7 @@ def part_slug
       end
 
       def requested_path
-        base_path + "/" + part_slug
+        "#{base_path}/#{part_slug}"
       end
     end
 
@@ -136,7 +136,7 @@ def part_slug
       end
 
       def requested_path
-        base_path + "/" + part_slug
+        "#{base_path}/#{part_slug}"
       end
     end
 

test/presenters/content_item/shareable_test.rb

@@ -15,7 +15,7 @@ def initialize
   end
 
   def expected_path
-    url_encode(Plek.current.website_root + "/a/base/path")
+    url_encode("#{Plek.current.website_root}/a/base/path")
   end
 
   test "presents the twitter share url" do

test/presenters/content_item_presenter_test.rb

@@ -33,7 +33,7 @@ class ContentItemPresenterTest < PresenterTestCase
 
   test "#canonical_url with a part" do
     example_with_parts = govuk_content_schema_example("travel_advice", "full-country")
-    request_path = example_with_parts["base_path"] + "/safety-and-security"
+    request_path = "#{example_with_parts['base_path']}/safety-and-security"
     presenter = create_presenter(
       TravelAdvicePresenter,
       content_item: example_with_parts,

test/test_helper.rb

@@ -37,7 +37,7 @@ class ActiveSupport::TestCase
   include GovukContentSchemaExamples
 end
 
-# Note: This is so that slimmer is skipped, preventing network requests for
+# NOTE: This is so that slimmer is skipped, preventing network requests for
 # content from static (i.e. core_layout.html.erb).
 class ActionController::Base
   before_action :set_skip_slimmer_header
@@ -85,7 +85,7 @@ def assert_has_contents_list(contents)
     end
   end
 
-  def assert_has_published_dates(first_published = nil, last_updated = nil, history_link = false)
+  def assert_has_published_dates(first_published = nil, last_updated = nil, history_link: false)
     text = []
     text << first_published if first_published
     text << last_updated if last_updated
@@ -135,7 +135,7 @@ def assert_has_metadata_local(metadata, term_selector, definition_selector)
 
   def assert_has_publisher_metadata(options)
     within(".app-c-publisher-metadata") do
-      assert_has_published_dates(options[:first_published], options[:last_updated], options[:history_link])
+      assert_has_published_dates(options[:first_published], options[:last_updated], history_link: options[:history_link])
       assert_has_publisher_metadata_other(options[:metadata])
     end
   end
@@ -148,8 +148,8 @@ def assert_has_important_metadata(metadata)
     end
   end
 
-  def assert_footer_has_published_dates(first_published = nil, last_updated = nil, history_link = false)
-    assert_has_published_dates(first_published, last_updated, history_link)
+  def assert_footer_has_published_dates(first_published = nil, last_updated = nil, history_link: false)
+    assert_has_published_dates(first_published, last_updated, history_link: history_link)
   end
 
   def setup_and_visit_content_item(name, parameter_string = "")