From e5f4869c02442b7f11780c8bd033cbc6ccff9028 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2020 20:02:26 +0000 Subject: [PATCH 1/8] Bump rubocop from 0.83.0 to 0.91.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [rubocop](https://github.com/rubocop-hq/rubocop) from 0.83.0 to 0.91.0. - [Release notes](https://github.com/rubocop-hq/rubocop/releases) - [Changelog](https://github.com/rubocop-hq/rubocop/blob/master/CHANGELOG.md) - [Commits](https://github.com/rubocop-hq/rubocop/compare/v0.83.0...v0.91.0) The `ConsiderPunctuation` option in the `OrderedGems` rule now defaults to false. We're changing it to true so we keep the existing behavior and because that's the way programs like vim sort lines. Signed-off-by: dependabot-preview[bot] Co-Authored-By: Javi Martín --- .hound.yml | 2 +- .rubocop.yml | 1 + Gemfile | 2 +- Gemfile.lock | 10 +++++++--- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.hound.yml b/.hound.yml index 0b423860b..c7c92dcfe 100644 --- a/.hound.yml +++ b/.hound.yml @@ -1,6 +1,6 @@ rubocop: config_file: .rubocop.yml - version: 0.83.0 + version: 0.91.0 scss: config_file: .scss-lint.yml erblint: diff --git a/.rubocop.yml b/.rubocop.yml index 9941c7659..b8dd5c66c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -15,6 +15,7 @@ Bundler/DuplicatedGem: Bundler/OrderedGems: Enabled: true + ConsiderPunctuation: true Capybara/CurrentPathExpectation: Enabled: true diff --git a/Gemfile b/Gemfile index f5580c981..6a3827b69 100644 --- a/Gemfile +++ b/Gemfile @@ -102,7 +102,7 @@ group :development do gem "erb_lint", require: false gem "github_changelog_generator", "~> 1.15.2" gem "mdl", "~> 0.11.0", require: false - gem "rubocop", "~> 0.83.0", require: false + gem "rubocop", "~> 0.91.0", require: false gem "rubocop-performance", "~> 1.7.1", require: false gem "rubocop-rails", "~> 2.3.2", require: false gem "rubocop-rspec", "~> 1.35.0", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 35285d5f1..883830f58 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -493,13 +493,17 @@ GEM rspec-mocks (~> 3.9) rspec-support (~> 3.9) rspec-support (3.9.3) - rubocop (0.83.0) + rubocop (0.91.1) parallel (~> 1.10) - parser (>= 2.7.0.1) + parser (>= 2.7.1.1) rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 1.7) rexml + rubocop-ast (>= 0.4.0, < 1.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 2.0) + rubocop-ast (0.7.1) + parser (>= 2.7.1.5) rubocop-performance (1.7.1) rubocop (>= 0.82.0) rubocop-rails (2.3.2) @@ -707,7 +711,7 @@ DEPENDENCIES rinku (~> 2.0.6) rollbar (~> 3.0.0) rspec-rails (~> 4.0) - rubocop (~> 0.83.0) + rubocop (~> 0.91.0) rubocop-performance (~> 1.7.1) rubocop-rails (~> 2.3.2) rubocop-rspec (~> 1.35.0) From 02ecdf6acc8289813ca44c6e927600208727e530 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 10 Oct 2020 20:35:30 +0200 Subject: [PATCH 2/8] Add and apply Style/AccessorGrouping rubocop rule This rule was added in Rubocop 0.87.0. We were already grouping accessors in most places. --- .rubocop.yml | 3 +++ app/models/user.rb | 4 +--- app/models/verification/management/email.rb | 4 +--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index b8dd5c66c..f567ec525 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -368,6 +368,9 @@ Security/JSONLoad: Security/YAMLLoad: Enabled: true +Style/AccessorGrouping: + Enabled: true + Style/AndOr: Enabled: true diff --git a/app/models/user.rb b/app/models/user.rb index ac530d3a1..5daabdb50 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -89,9 +89,7 @@ class User < ApplicationRecord accepts_nested_attributes_for :organization, update_only: true - attr_accessor :skip_password_validation - attr_accessor :use_redeemable_code - attr_accessor :login + attr_accessor :skip_password_validation, :use_redeemable_code, :login scope :administrators, -> { joins(:administrator) } scope :moderators, -> { joins(:moderator) } diff --git a/app/models/verification/management/email.rb b/app/models/verification/management/email.rb index ed76e74d8..5348c97ba 100644 --- a/app/models/verification/management/email.rb +++ b/app/models/verification/management/email.rb @@ -1,9 +1,7 @@ class Verification::Management::Email include ActiveModel::Model - attr_accessor :document_type - attr_accessor :document_number - attr_accessor :email + attr_accessor :document_type, :document_number, :email validates :document_type, :document_number, :email, presence: true validates :email, format: { with: Devise.email_regexp }, allow_blank: true From 7275fc9aa2ed569521248ebc82f97a6cf3cd5f8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 10 Oct 2020 20:48:07 +0200 Subject: [PATCH 3/8] Add and apply RedundantFileExtensionInRequire rule This rule was added in Rubocop 0.88.0. --- .rubocop.yml | 3 +++ config/application.rb | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index f567ec525..a4f10b1dd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -410,6 +410,9 @@ Style/PercentLiteralDelimiters: Style/Proc: Enabled: true +Style/RedundantFileExtensionInRequire: + Enabled: true + Style/RedundantFreeze: Enabled: true diff --git a/config/application.rb b/config/application.rb index 935dffa93..e151e7cbd 100644 --- a/config/application.rb +++ b/config/application.rb @@ -129,4 +129,4 @@ class Rails::Engine end end -require "./config/application_custom.rb" +require "./config/application_custom" From efd8f475961dc4a81370c82b1b701eda0f41639c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 10 Oct 2020 21:09:27 +0200 Subject: [PATCH 4/8] Add and apply ArrayCoercion rubocop rule This rule was added in Rubocop 0.88.0. --- .rubocop.yml | 3 +++ app/controllers/concerns/translatable.rb | 4 ++-- db/dev_seeds/users.rb | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a4f10b1dd..ff1fc9fdd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -374,6 +374,9 @@ Style/AccessorGrouping: Style/AndOr: Enabled: true +Style/ArrayCoercion: + Enabled: true + Style/BlockDelimiters: Enabled: true diff --git a/app/controllers/concerns/translatable.rb b/app/controllers/concerns/translatable.rb index 533b8db22..8bff3b08d 100644 --- a/app/controllers/concerns/translatable.rb +++ b/app/controllers/concerns/translatable.rb @@ -6,10 +6,10 @@ module Translatable def translation_params(resource_model, options = {}) attributes = [:id, :locale, :_destroy] if options[:only] - attributes += [*options[:only]] + attributes += Array(options[:only]) else attributes += resource_model.translated_attribute_names end - { translations_attributes: attributes - [*options[:except]] } + { translations_attributes: attributes - Array(options[:except]) } end end diff --git a/db/dev_seeds/users.rb b/db/dev_seeds/users.rb index 954f53282..a8b5ee429 100644 --- a/db/dev_seeds/users.rb +++ b/db/dev_seeds/users.rb @@ -17,7 +17,7 @@ section "Creating Users" do def unique_document_number @document_number ||= 12345678 @document_number += 1 - "#{@document_number}#{[*"A".."Z"].sample}" + "#{@document_number}#{("A".."Z").to_a.sample}" end admin = create_user("admin@consul.dev", "admin") From 66759d2dc05ced7a9f88524974e1b2acff39a8cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 10 Oct 2020 22:00:55 +0200 Subject: [PATCH 5/8] Apply StringConcatenation rule in some places This rule was added in Rubocop 0.89.0. However, there are some false positives when we don't use interpolation but simply concatenate in order to avoid long lines. Even if there weren't false positives, there are places where we concatenate to emphasize the point that we're adding a certain character to a text. We might reconsider this rule in the future, since we generally prefer interpolation over concatenation. --- app/helpers/followables_helper.rb | 2 +- app/helpers/tracks_helper.rb | 2 +- app/helpers/verification_helper.rb | 2 +- app/helpers/votes_helper.rb | 4 ++-- db/dev_seeds/admin_notifications.rb | 2 +- spec/system/admin/homepage/homepage_spec.rb | 2 +- spec/system/admin/site_customization/documents_spec.rb | 2 +- spec/system/social_media_meta_tags_spec.rb | 3 +-- 8 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app/helpers/followables_helper.rb b/app/helpers/followables_helper.rb index ee01d9aee..52a90f156 100644 --- a/app/helpers/followables_helper.rb +++ b/app/helpers/followables_helper.rb @@ -14,7 +14,7 @@ module FollowablesHelper return unless follow.followable.present? followable = follow.followable - partial = followable_class_name(followable) + "_follow" + partial = "#{followable_class_name(followable)}_follow" locals = { followable_class_name(followable).to_sym => followable } render partial, locals diff --git a/app/helpers/tracks_helper.rb b/app/helpers/tracks_helper.rb index 305876915..8236751d5 100644 --- a/app/helpers/tracks_helper.rb +++ b/app/helpers/tracks_helper.rb @@ -3,7 +3,7 @@ module TracksHelper track_data = "" prefix = " data-track-event-" data.each do |key, value| - track_data = track_data + prefix + key.to_s + "=" + value + " " + track_data = "#{track_data}#{prefix}#{key}=#{value} " end content_for :track_event do track_data diff --git a/app/helpers/verification_helper.rb b/app/helpers/verification_helper.rb index 587e17859..3dd9123c9 100644 --- a/app/helpers/verification_helper.rb +++ b/app/helpers/verification_helper.rb @@ -21,6 +21,6 @@ module VerificationHelper data_to_mask = match[2] email_provider = match[3] - data_to_display + "*" * data_to_mask.size + "@" + email_provider + "#{data_to_display}#{"*" * data_to_mask.size}@#{email_provider}" end end diff --git a/app/helpers/votes_helper.rb b/app/helpers/votes_helper.rb index dd436ea4e..8739c2323 100644 --- a/app/helpers/votes_helper.rb +++ b/app/helpers/votes_helper.rb @@ -7,9 +7,9 @@ module VotesHelper return "0%" if debate.total_votes == 0 if vote == "likes" - debate_percentage_of_likes(debate).to_s + "%" + "#{debate_percentage_of_likes(debate)}%" elsif vote == "dislikes" - (100 - debate_percentage_of_likes(debate)).to_s + "%" + "#{100 - debate_percentage_of_likes(debate)}%" end end diff --git a/db/dev_seeds/admin_notifications.rb b/db/dev_seeds/admin_notifications.rb index 308928d53..8823407df 100644 --- a/db/dev_seeds/admin_notifications.rb +++ b/db/dev_seeds/admin_notifications.rb @@ -7,7 +7,7 @@ section "Creating Admin Notifications & Templates" do "people will discuss & support it.", body_es: "Recuerda que puedes crear propuestas y los ciudadanos las debatirán y apoyarán.", - link: Setting["url"] + "/proposals", + link: "#{Setting["url"]}/proposals", segment_recipient: "administrators" ).deliver diff --git a/spec/system/admin/homepage/homepage_spec.rb b/spec/system/admin/homepage/homepage_spec.rb index e2ba36f7a..9a44cda7d 100644 --- a/spec/system/admin/homepage/homepage_spec.rb +++ b/spec/system/admin/homepage/homepage_spec.rb @@ -22,7 +22,7 @@ describe "Homepage" do scenario "Admin menu links to homepage path" do visit new_admin_widget_card_path(header_card: true) - click_link Setting["org_name"] + " Administration" + click_link "#{Setting["org_name"]} Administration" expect(page).to have_current_path(admin_root_path) end diff --git a/spec/system/admin/site_customization/documents_spec.rb b/spec/system/admin/site_customization/documents_spec.rb index 1b0ac771b..51d0a3f2b 100644 --- a/spec/system/admin/site_customization/documents_spec.rb +++ b/spec/system/admin/site_customization/documents_spec.rb @@ -58,7 +58,7 @@ describe "Documents" do scenario "Create" do visit new_admin_site_customization_document_path - attach_file("document_attachment", Rails.root + "spec/fixtures/files/logo.pdf") + attach_file("document_attachment", "#{Rails.root}/spec/fixtures/files/logo.pdf") click_button "Upload" expect(page).to have_content "Document uploaded succesfully" diff --git a/spec/system/social_media_meta_tags_spec.rb b/spec/system/social_media_meta_tags_spec.rb index f114c204f..448776b9d 100644 --- a/spec/system/social_media_meta_tags_spec.rb +++ b/spec/system/social_media_meta_tags_spec.rb @@ -33,8 +33,7 @@ describe "Social media meta tags" do expect(page).to have_property "og:title", with: meta_title expect(page).to have_property "article:publisher", with: url - expect(page).to have_property "article:author", with: "https://www.facebook.com/" + - facebook_handle + expect(page).to have_property "article:author", with: "https://www.facebook.com/#{facebook_handle}" expect(page).to have_property "og:url", with: "#{Capybara.app_host}/" expect(page).to have_property "og:image", with: "#{Capybara.app_host}/social_media_icon.png" expect(page).to have_property "og:site_name", with: org_name From f632c5bfca8a6ef4c212f68f0ca017259a71bffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 11 Oct 2020 13:09:58 +0200 Subject: [PATCH 6/8] Add and apply EmptyFile rubocop rule This rule was added in Rubocop 0.90.0. --- .rubocop.yml | 3 +++ spec/controllers/welcome_controller_spec.rb | 0 2 files changed, 3 insertions(+) delete mode 100644 spec/controllers/welcome_controller_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index ff1fc9fdd..ac924d8bf 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -169,6 +169,9 @@ Lint/AmbiguousRegexpLiteral: Lint/DuplicateMethods: Enabled: true +Lint/EmptyFile: + Enabled: true + Lint/LiteralAsCondition: Enabled: true diff --git a/spec/controllers/welcome_controller_spec.rb b/spec/controllers/welcome_controller_spec.rb deleted file mode 100644 index e69de29bb..000000000 From eb02bc756af0a8e6e7c9fe205d35db5b30791264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 14 Oct 2020 11:54:05 +0200 Subject: [PATCH 7/8] Remove unnecessary constant declarations The `controller` method already creates an anonymous class inheriting from the class we pass it, so there's no need to create yet another subclass. --- spec/controllers/concerns/has_filters_spec.rb | 4 +--- spec/controllers/concerns/has_orders_spec.rb | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/spec/controllers/concerns/has_filters_spec.rb b/spec/controllers/concerns/has_filters_spec.rb index f4cade052..c95531265 100644 --- a/spec/controllers/concerns/has_filters_spec.rb +++ b/spec/controllers/concerns/has_filters_spec.rb @@ -1,9 +1,7 @@ require "rails_helper" describe HasFilters do - class FakeController < ActionController::Base; end - - controller(FakeController) do + controller(ActionController::Base) do include HasFilters has_filters ["all", "pending", "reviewed"], only: :index diff --git a/spec/controllers/concerns/has_orders_spec.rb b/spec/controllers/concerns/has_orders_spec.rb index f37b7c59a..4248e5cd5 100644 --- a/spec/controllers/concerns/has_orders_spec.rb +++ b/spec/controllers/concerns/has_orders_spec.rb @@ -1,9 +1,7 @@ require "rails_helper" describe HasOrders do - class FakeController < ActionController::Base; end - - controller(FakeController) do + controller(ActionController::Base) do include HasOrders has_orders ["created_at", "votes_count", "flags_count", "relevance"], only: :index has_orders ->(c) { ["votes_count", "flags_count"] }, only: :new From b27d0a8e9229054edd3af9def8d10b0807a8cb98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 14 Oct 2020 12:03:26 +0200 Subject: [PATCH 8/8] Add and apply ConstantDefinitionInBlock rule This rule was added in Rubocop 0.91.0. A similar rule named LeakyConstantDeclaration was added in rubocop-rspec 1.34.0. Note using the FILENAMES constant did not result in an offense using the ConstantDefinitionInBlock rule but did result in an offense using the LeakyConstantDeclaration rule. I've simplified the code to get rid of the constant; not sure why we were adding a constant with `||=` in the middle of a spec. --- .rubocop.yml | 3 +++ spec/lib/consul_form_builder_spec.rb | 12 ++++++++---- spec/models/statisticable_spec.rb | 12 ++++++++---- spec/shared/system/nested_documentable.rb | 5 ++--- spec/views/shared/errors_spec.rb | 16 ++++++++++------ 5 files changed, 31 insertions(+), 17 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index ac924d8bf..b74689518 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -166,6 +166,9 @@ Layout/TrailingWhitespace: Lint/AmbiguousRegexpLiteral: Enabled: true +Lint/ConstantDefinitionInBlock: + Enabled: true + Lint/DuplicateMethods: Enabled: true diff --git a/spec/lib/consul_form_builder_spec.rb b/spec/lib/consul_form_builder_spec.rb index 91d22ab89..ed73b6b62 100644 --- a/spec/lib/consul_form_builder_spec.rb +++ b/spec/lib/consul_form_builder_spec.rb @@ -1,10 +1,14 @@ require "rails_helper" describe ConsulFormBuilder do - class DummyModel - include ActiveModel::Model - OPTIONS = %w[Good Bad Ugly].freeze - attr_accessor :title, :quality + before do + dummy_model = Class.new do + include ActiveModel::Model + attr_accessor :title, :quality + end + + stub_const("DummyModel", dummy_model) + stub_const("DummyModel::OPTIONS", %w[Good Bad Ugly].freeze) end let(:builder) { ConsulFormBuilder.new(:dummy, DummyModel.new, ActionView::Base.new, {}) } diff --git a/spec/models/statisticable_spec.rb b/spec/models/statisticable_spec.rb index 3757d3db8..1cc57ca2a 100644 --- a/spec/models/statisticable_spec.rb +++ b/spec/models/statisticable_spec.rb @@ -1,12 +1,16 @@ require "rails_helper" describe Statisticable do - class DummyStats - include Statisticable + before do + dummy_stats = Class.new do + include Statisticable - def participants - User.all + def participants + User.all + end end + + stub_const("DummyStats", dummy_stats) end let(:stats) { DummyStats.new(nil) } diff --git a/spec/shared/system/nested_documentable.rb b/spec/shared/system/nested_documentable.rb index 9c6482d84..a44096f06 100644 --- a/spec/shared/system/nested_documentable.rb +++ b/spec/shared/system/nested_documentable.rb @@ -235,12 +235,11 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na end login_as user_to_login visit send(path, arguments) - FILENAMES ||= %w[clippy empty logo].freeze send(fill_resource_method_name) if fill_resource_method_name - documentable.class.max_documents_allowed.times.zip(FILENAMES).each do |_n, fn| - documentable_attach_new_file(Rails.root.join("spec/fixtures/files/#{fn}.pdf")) + %w[clippy empty logo].take(documentable.class.max_documents_allowed).each do |filename| + documentable_attach_new_file(Rails.root.join("spec/fixtures/files/#{filename}.pdf")) end click_on submit_button diff --git a/spec/views/shared/errors_spec.rb b/spec/views/shared/errors_spec.rb index 6e3c4ec49..1159f58c7 100644 --- a/spec/views/shared/errors_spec.rb +++ b/spec/views/shared/errors_spec.rb @@ -1,13 +1,17 @@ require "rails_helper" describe "shared errors" do - class DummyModel - include ActiveModel::Model - attr_accessor :title, :description, :days + before do + dummy_model = Class.new do + include ActiveModel::Model + attr_accessor :title, :description, :days - validates :title, presence: true - validates :description, presence: true, length: { in: 10..100 } - validates :days, numericality: { greater_than: 10 } + validates :title, presence: true + validates :description, presence: true, length: { in: 10..100 } + validates :days, numericality: { greater_than: 10 } + end + + stub_const("DummyModel", dummy_model) end it "counts the number of fields with errors" do