From 5537fdb9a14cf17de27f678adf9d406f68c9605e Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Tue, 17 Sep 2024 16:21:20 +0200 Subject: [PATCH] Enable new cops and fix some offenses --- .rubocop_cc.yml | 6 + .rubocop_todo.yml | 123 ++++-------------- .../runtime/update_buildpack_installer.rb | 6 +- .../label_selector_requirement_validator.rb | 2 +- app/presenters/v3/paginated_list_presenter.rb | 2 +- lib/cloud_controller/console.rb | 5 +- spec/api/documentation/stacks_api_spec.rb | 2 +- spec/api/internal/log_access_api_spec.rb | 2 +- .../support/matchers/have_queried_db_times.rb | 1 - .../v3/sidecars_controller_spec.rb | 2 +- .../dependency_locator_spec.rb | 2 +- .../errands/rotate_database_key_spec.rb | 2 +- .../install_buildpacks_spec.rb | 4 +- spec/unit/middleware/request_metrics_spec.rb | 2 +- 14 files changed, 41 insertions(+), 120 deletions(-) diff --git a/.rubocop_cc.yml b/.rubocop_cc.yml index dc820d7b2f6..7265c1a51f8 100644 --- a/.rubocop_cc.yml +++ b/.rubocop_cc.yml @@ -556,6 +556,12 @@ Style/SuperArguments: Enabled: true Rails/WhereRange: Enabled: true +Lint/UselessNumericOperation: + Enabled: true +Style/RedundantInterpolationUnfreeze: + Enabled: true +Rails/EnumSyntax: + Enabled: true ### Broken Cops that break code Lint/ShadowedException: # Breaks "bundle exec rake rubocop" if enabled diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index a34a246a2f8..98d12ff8b8d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2024-09-11 09:08:10 UTC using RuboCop version 1.66.1. +# on 2024-09-30 07:29:13 UTC using RuboCop version 1.66.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -33,7 +33,7 @@ Lint/EmptyConditionalBody: - 'lib/cloud_controller/diego/failure_reason_sanitizer.rb' - 'spec/support/shared_examples/jobs/delayed_job.rb' -# Offense count: 56 +# Offense count: 57 # Configuration parameters: AllowedParentClasses. Lint/MissingSuper: Enabled: false @@ -79,13 +79,6 @@ Lint/UnusedMethodArgument: - 'lib/services/service_brokers/v2/response_parser.rb' - 'spec/support/fakes/fake_service_broker_v2_client.rb' -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: AutoCorrect. -Lint/UselessAssignment: - Exclude: - - 'spec/support/matchers/have_queried_db_times.rb' - # Offense count: 4 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: AutoCorrect. @@ -96,12 +89,7 @@ Lint/UselessMethodDefinition: - 'app/presenters/v3/process_presenter.rb' - 'spec/support/fake_front_controller.rb' -# Offense count: 1 -Lint/UselessRescue: - Exclude: - - 'app/jobs/runtime/update_buildpack_installer.rb' - -# Offense count: 790 +# Offense count: 791 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: Max: 99 @@ -109,14 +97,14 @@ Metrics/AbcSize: # Offense count: 114 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 447 + Max: 446 # Offense count: 62 # Configuration parameters: CountKeywordArgs, MaxOptionalParameters. Metrics/ParameterLists: Max: 9 -# Offense count: 163 +# Offense count: 162 # Configuration parameters: AllowedMethods, AllowedPatterns. Metrics/PerceivedComplexity: Max: 30 @@ -144,14 +132,14 @@ Naming/HeredocDelimiterNaming: Naming/PredicateName: Enabled: false -# Offense count: 860 +# Offense count: 862 # Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns. # SupportedStyles: snake_case, normalcase, non_integer # AllowedIdentifiers: capture3, iso8601, rfc1123_date, rfc822, rfc2822, rfc3339, x86_64 Naming/VariableNumber: Enabled: false -# Offense count: 407 +# Offense count: 409 RSpec/AnyInstance: Enabled: false @@ -180,17 +168,12 @@ RSpec/ChangeByZero: RSpec/ContextWording: Enabled: false -# Offense count: 98 +# Offense count: 99 # Configuration parameters: IgnoredMetadata. RSpec/DescribeClass: Enabled: false -# Offense count: 1 -RSpec/DescribeMethod: - Exclude: - - 'spec/api/internal/log_access_api_spec.rb' - -# Offense count: 4428 +# Offense count: 4434 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: SkipBlocks, EnforcedStyle, OnlyStaticConstants. # SupportedStyles: described_class, explicit @@ -203,19 +186,11 @@ RSpec/DescribedClass: RSpec/EmptyExampleGroup: Enabled: false -# Offense count: 4293 +# Offense count: 4303 # Configuration parameters: CountAsOne. RSpec/ExampleLength: Max: 158 -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: CustomTransform, IgnoredWords, DisallowedExamples. -# DisallowedExamples: works -RSpec/ExampleWording: - Exclude: - - 'spec/unit/controllers/v3/sidecars_controller_spec.rb' - # Offense count: 34 # This cop supports safe autocorrection (--autocorrect). RSpec/ExpectActual: @@ -231,12 +206,7 @@ RSpec/ExpectInHook: RSpec/ExpectInLet: Enabled: false -# Offense count: 1 -RSpec/IdenticalEqualityAssertion: - Exclude: - - 'spec/unit/lib/cloud_controller/dependency_locator_spec.rb' - -# Offense count: 1499 +# Offense count: 1501 # Configuration parameters: Max, AllowedIdentifiers, AllowedPatterns. RSpec/IndexedLet: Enabled: false @@ -268,44 +238,39 @@ RSpec/MessageChain: - 'spec/unit/messages/space_quota_update_message_spec.rb' - 'spec/unit/messages/space_quotas_create_message_spec.rb' -# Offense count: 785 +# Offense count: 798 # Configuration parameters: EnforcedStyle. # SupportedStyles: have_received, receive RSpec/MessageSpies: Enabled: false -# Offense count: 1 -RSpec/MissingExampleGroupArgument: - Exclude: - - 'spec/api/documentation/stacks_api_spec.rb' - # Offense count: 2 RSpec/MultipleDescribes: Exclude: - 'spec/request/knowledge_bombs/verify_old_lrps_can_download_assets_spec.rb' - 'spec/unit/lib/vcap/json_message_spec.rb' -# Offense count: 8089 +# Offense count: 8101 RSpec/MultipleExpectations: Max: 48 -# Offense count: 8923 +# Offense count: 8945 # Configuration parameters: AllowSubject. RSpec/MultipleMemoizedHelpers: Max: 36 -# Offense count: 2549 +# Offense count: 2557 # Configuration parameters: EnforcedStyle, IgnoreSharedExamples. # SupportedStyles: always, named_only RSpec/NamedSubject: Enabled: false -# Offense count: 1598 +# Offense count: 1602 # Configuration parameters: AllowedGroups. RSpec/NestedGroups: Max: 8 -# Offense count: 88 +# Offense count: 89 # Configuration parameters: AllowedPatterns. # AllowedPatterns: ^expect_, ^assert_ RSpec/NoExpectationExample: @@ -327,14 +292,6 @@ RSpec/PendingWithoutReason: Exclude: - 'spec/unit/fetchers/label_selector_query_generator_spec.rb' -# Offense count: 1 -# This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: Strict, EnforcedStyle, AllowedExplicitMatchers. -# SupportedStyles: inflected, explicit -RSpec/PredicateMatcher: - Exclude: - - 'spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb' - # Offense count: 44 RSpec/RepeatedDescription: Exclude: @@ -402,13 +359,13 @@ RSpec/ReturnFromStub: Exclude: - 'spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb' -# Offense count: 204 +# Offense count: 206 # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: AutoCorrect. RSpec/ScatteredLet: Enabled: false -# Offense count: 33 +# Offense count: 35 # Configuration parameters: Include, CustomTransform, IgnoreMethods, IgnoreMetadata. # Include: **/*_spec.rb RSpec/SpecFilePathFormat: @@ -441,11 +398,6 @@ RSpec/SubjectStub: - 'spec/unit/lib/vcap/host_system_spec.rb' - 'spec/unit/support/stepper_spec.rb' -# Offense count: 1 -RSpec/UnspecifiedException: - Exclude: - - 'spec/unit/middleware/request_metrics_spec.rb' - # Offense count: 72 # Configuration parameters: EnforcedStyle, AllowedPatterns. # SupportedStyles: snake_case, camelCase @@ -457,7 +409,7 @@ RSpec/VariableName: - 'spec/request/revisions_spec.rb' - 'spec/request/spaces_spec.rb' -# Offense count: 531 +# Offense count: 536 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. RSpec/VerifiedDoubles: Enabled: false @@ -488,13 +440,6 @@ Rails/ActiveRecordAliases: - 'spec/support/shared_examples/models/operations.rb' - 'spec/unit/models/services/service_instance_operation_spec.rb' -# Offense count: 1 -# Configuration parameters: Include. -# Include: app/**/*.rb, config/**/*.rb, lib/**/*.rb -Rails/Exit: - Exclude: - - 'lib/cloud_controller/console.rb' - # Offense count: 2 # Configuration parameters: Include. # Include: db/**/*.rb @@ -503,7 +448,7 @@ Rails/ThreeStateBooleanColumn: - 'db/migrations/20141022211551_add_updateable_column_to_services.rb' - 'db/migrations/20151217235335_remove_unused_package_cols.rb' -# Offense count: 256 +# Offense count: 260 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: EnforcedStyle. # SupportedStyles: strict, flexible @@ -576,13 +521,7 @@ Style/CombinableLoops: - 'spec/linters/migration/add_constraint_name.rb' - 'spec/unit/models/runtime/organization_spec.rb' -# Offense count: 1 -# This cop supports unsafe autocorrection (--autocorrect-all). -Style/ConcatArrayLiterals: - Exclude: - - 'spec/unit/lib/cloud_controller/install_buildpacks_spec.rb' - -# Offense count: 1486 +# Offense count: 1488 # Configuration parameters: AllowedConstants. Style/Documentation: Enabled: false @@ -599,14 +538,6 @@ Style/DoubleNegation: - 'spec/support/matchers/have_attributes.rb' - 'spec/unit/lib/services/service_brokers/v2/response_parser_spec.rb' -# Offense count: 1 -# This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: EnforcedStyle. -# SupportedStyles: left_coerce, right_coerce, single_coerce, fdiv -Style/FloatDivision: - Exclude: - - 'app/presenters/v3/paginated_list_presenter.rb' - # Offense count: 3279 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: EnforcedStyle. @@ -622,14 +553,6 @@ Style/HashConversion: - 'lib/cloud_controller/controller_factory.rb' - 'spec/support/matchers/sequel_validations.rb' -# Offense count: 1 -# This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: AllowedReceivers. -# AllowedReceivers: Thread.current -Style/HashEachMethods: - Exclude: - - 'app/messages/validators/label_selector_requirement_validator.rb' - # Offense count: 5 # This cop supports unsafe autocorrection (--autocorrect-all). Style/MapIntoArray: @@ -705,7 +628,7 @@ Style/ReturnNilInPredicateMethodDefinition: - 'app/messages/space_delete_unmapped_routes_message.rb' - 'spec/support/legacy_api_dsl.rb' -# Offense count: 111 +# Offense count: 114 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: ConvertCodeThatCanStartToReturnNil, AllowedMethods, MaxChainLength. # AllowedMethods: present?, blank?, presence, try, try! diff --git a/app/jobs/runtime/update_buildpack_installer.rb b/app/jobs/runtime/update_buildpack_installer.rb index de98ed89eb6..1734910510d 100644 --- a/app/jobs/runtime/update_buildpack_installer.rb +++ b/app/jobs/runtime/update_buildpack_installer.rb @@ -12,11 +12,7 @@ def perform return end - begin - buildpack_uploader.upload_buildpack(buildpack, file, File.basename(file)) - rescue StandardError - raise - end + buildpack_uploader.upload_buildpack(buildpack, file, File.basename(file)) buildpack.update(options.merge(stack: stack_name)) logger.info "Buildpack #{name} updated" diff --git a/app/messages/validators/label_selector_requirement_validator.rb b/app/messages/validators/label_selector_requirement_validator.rb index 15b88d490f6..13d03b9d637 100644 --- a/app/messages/validators/label_selector_requirement_validator.rb +++ b/app/messages/validators/label_selector_requirement_validator.rb @@ -34,7 +34,7 @@ def valid_requirement?(requirement) res = MetadataValidatorHelper.new(key: requirement.key).key_error return res unless res.is_valid? - requirement.values.each do |v| + requirement.values.each do |v| # rubocop:disable Style/HashEachMethods res = MetadataValidatorHelper.new(value: v).value_error return res unless res.is_valid? end diff --git a/app/presenters/v3/paginated_list_presenter.rb b/app/presenters/v3/paginated_list_presenter.rb index 1a006ae0952..ff9bcfa1aa2 100644 --- a/app/presenters/v3/paginated_list_presenter.rb +++ b/app/presenters/v3/paginated_list_presenter.rb @@ -28,7 +28,7 @@ def to_hash def present_pagination_hash(filters=nil) pagination_options = @paginated_result.pagination_options - last_page = (@paginated_result.total.to_f / pagination_options.per_page.to_f).ceil + last_page = (@paginated_result.total.to_f / pagination_options.per_page).ceil last_page = 1 if last_page < 1 previous_page = pagination_options.page - 1 next_page = pagination_options.page + 1 diff --git a/lib/cloud_controller/console.rb b/lib/cloud_controller/console.rb index 892fed167de..0eab422f2aa 100755 --- a/lib/cloud_controller/console.rb +++ b/lib/cloud_controller/console.rb @@ -16,10 +16,7 @@ end @config_file = ARGV[0] || ENV['CLOUD_CONTROLLER_NG_CONFIG'] || File.expand_path('../../config/cloud_controller.yml', __dir__) -unless File.exist?(@config_file) - warn "#{@config_file} not found. Try running bin/console ." - exit 1 -end +raise "#{@config_file} not found. Try running bin/console ." unless File.exist?(@config_file) context = ARGV[1].try(:to_sym) || :api config_kwargs = { context: } diff --git a/spec/api/documentation/stacks_api_spec.rb b/spec/api/documentation/stacks_api_spec.rb index b85bf1b6f1a..07575784aa0 100644 --- a/spec/api/documentation/stacks_api_spec.rb +++ b/spec/api/documentation/stacks_api_spec.rb @@ -11,7 +11,7 @@ let(:guid) { VCAP::CloudController::Stack.first.guid } - context do + describe 'Standard endpoints' do field :name, 'The name for the stack.' field :description, 'The description for the stack' diff --git a/spec/api/internal/log_access_api_spec.rb b/spec/api/internal/log_access_api_spec.rb index 1735030418b..a524cd07c0b 100644 --- a/spec/api/internal/log_access_api_spec.rb +++ b/spec/api/internal/log_access_api_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -RSpec.describe 'GET', '/internal/v4/log_access/:guid', type: [:api] do +RSpec.describe 'GET /internal/v4/log_access/:guid', type: [:api] do include RequestSpecHelper context 'when the guid is for a v3 app' do diff --git a/spec/support/matchers/have_queried_db_times.rb b/spec/support/matchers/have_queried_db_times.rb index f4b44f610fc..f31dace911a 100644 --- a/spec/support/matchers/have_queried_db_times.rb +++ b/spec/support/matchers/have_queried_db_times.rb @@ -1,6 +1,5 @@ RSpec::Matchers.define :have_queried_db_times do |query_regex, expected_times| match do |actual_blk| - matched_calls = [] stub = allow(Sequel::Model.db).to receive(:_execute) stub.and_call_original calls = stub.and_record_arguments diff --git a/spec/unit/controllers/v3/sidecars_controller_spec.rb b/spec/unit/controllers/v3/sidecars_controller_spec.rb index ece4004a4a7..28d79e0d2f8 100644 --- a/spec/unit/controllers/v3/sidecars_controller_spec.rb +++ b/spec/unit/controllers/v3/sidecars_controller_spec.rb @@ -39,7 +39,7 @@ set_current_user_as_role(role: :space_developer, org: org, user: user, space: space) end - it 'works' do + it 'succeeds' do get :index_by_app, params: { app_guid: app_model.guid } expect(response.status).to eq(200), response.body end diff --git a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb index e8db8f1e1e7..223d4965f1c 100644 --- a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb +++ b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb @@ -172,7 +172,7 @@ it { is_expected.to be_a(VCAP::CloudController::Repositories::AppEventRepository) } it 'memoizes the instance' do - expect(locator.app_event_repository).to eq(locator.app_event_repository) + expect(subject).to eq(locator.app_event_repository) end end diff --git a/spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb b/spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb index fbaf00e35e6..7c8455c3ed3 100644 --- a/spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb +++ b/spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb @@ -124,7 +124,7 @@ def nilable_columns(entity) entity = encrypted_models[klass] nilable_string_columns = nilable_columns(entity) vals = entity.reload.values.except(*encrypted_columns(entity.class), *nilable_string_columns) - expect(vals.values.all?(&:present?)).to be_truthy, "all fields of #{entity.class} need to have values" + expect(vals.values).to be_all(&:present?), "all fields of #{entity.class} need to have values" RotateDatabaseKey.perform(batch_size: 1) diff --git a/spec/unit/lib/cloud_controller/install_buildpacks_spec.rb b/spec/unit/lib/cloud_controller/install_buildpacks_spec.rb index 9ac3c650497..25e7cf0f63e 100644 --- a/spec/unit/lib/cloud_controller/install_buildpacks_spec.rb +++ b/spec/unit/lib/cloud_controller/install_buildpacks_spec.rb @@ -69,10 +69,10 @@ module VCAP::CloudController expect(File).to receive(:file?).with(buildpack2_file). and_return(true) - TestConfig.config[:install_buildpacks].concat [ + TestConfig.config[:install_buildpacks].push( { 'name' => 'buildpack1', 'package' => 'myotherpkg' }, { 'name' => 'buildpack2', 'package' => 'myotherpkg2' } - ] + ) allow(Buildpacks::StackNameExtractor).to receive(:extract_from_file).with(buildpack1a_file).and_return('cflinuxfs11') allow(Buildpacks::StackNameExtractor).to receive(:extract_from_file).with(buildpack1b_file).and_return('cflinuxfs12') diff --git a/spec/unit/middleware/request_metrics_spec.rb b/spec/unit/middleware/request_metrics_spec.rb index dded675ca5b..2a791079470 100644 --- a/spec/unit/middleware/request_metrics_spec.rb +++ b/spec/unit/middleware/request_metrics_spec.rb @@ -32,7 +32,7 @@ module Middleware end it 'catches the exception and calls complete request on request metrics' do - expect { middleware.call({}) }.to raise_error do + expect { middleware.call({}) }.to raise_error('Unexpected') do expect(request_metrics).to have_received(:complete_request).with(500) end end