Skip to content

Commit

Permalink
Autocorrect cops (#2459)
Browse files Browse the repository at this point in the history
* Autocorrect rubocop whenever possible

Fix Lint/AmbiguousBlockAssociation:
Fix Lint/DuplicateBranch
Fix Lint/EmptyClass:
Fix Naming/MemoizedInstanceVariableName
Fix Naming/MethodParameterName
Fix RSpec/NoExpectationExample
Fix RSpec/ScatteredSetup:
Fix Style/RedundantConstantBase
Fix Style/Semicolon
Fix Style/SuperArguments
Fix Style/SymbolProc
Fix Style/YodaCondition
Fix Style/ZeroLengthPredicate

* Add CHANGELOG.md
  • Loading branch information
ericproulx authored Jun 25, 2024
1 parent cc948bd commit 69d14ee
Show file tree
Hide file tree
Showing 30 changed files with 176 additions and 259 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,6 @@ RSpec/MultipleMemoizedHelpers:

RSpec/ContextWording:
Enabled: false

RSpec/MessageSpies:
EnforcedStyle: receive
131 changes: 5 additions & 126 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2024-06-15 15:12:21 UTC using RuboCop version 1.64.1.
# on 2024-06-22 17:26:10 UTC using RuboCop version 1.64.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
Expand All @@ -14,57 +14,19 @@ Gemspec/RequireMFA:
Exclude:
- 'grape.gemspec'

# Offense count: 1
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AllowedMethods, AllowedPatterns.
Lint/AmbiguousBlockAssociation:
Exclude:
- 'spec/grape/dsl/routing_spec.rb'

# Offense count: 1
# Configuration parameters: AllowedMethods.
# AllowedMethods: enums
Lint/ConstantDefinitionInBlock:
Exclude:
- 'spec/grape/validations/validators/except_values_spec.rb'

# Offense count: 3
# Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches.
Lint/DuplicateBranch:
Exclude:
- 'spec/support/versioned_helpers.rb'

# Offense count: 1
# Configuration parameters: AllowComments.
Lint/EmptyClass:
Exclude:
- 'lib/grape/dsl/parameters.rb'

# Offense count: 1
# Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns.
Metrics/MethodLength:
Exclude:
- 'lib/grape/endpoint.rb'

# Offense count: 2
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyleForLeadingUnderscores.
# SupportedStylesForLeadingUnderscores: disallowed, required, optional
Naming/MemoizedInstanceVariableName:
Exclude:
- 'lib/grape/api/instance.rb'
- 'lib/grape/middleware/base.rb'

# Offense count: 5
# Configuration parameters: MinNameLength, AllowNamesEndingInNumbers, AllowedNames, ForbiddenNames.
# AllowedNames: as, at, by, cc, db, id, if, in, io, ip, of, on, os, pp, to
Naming/MethodParameterName:
Exclude:
- 'lib/grape/endpoint.rb'
- 'lib/grape/middleware/error.rb'
- 'lib/grape/middleware/stack.rb'
- 'spec/grape/api_spec.rb'

# Offense count: 18
# Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns.
# SupportedStyles: snake_case, normalcase, non_integer
Expand Down Expand Up @@ -117,10 +79,9 @@ RSpec/ExpectActual:
- 'spec/grape/endpoint/declared_spec.rb'
- 'spec/grape/middleware/exception_spec.rb'

# Offense count: 3
# Offense count: 1
RSpec/ExpectInHook:
Exclude:
- 'spec/grape/api_spec.rb'
- 'spec/grape/validations/validators/values_spec.rb'

# Offense count: 6
Expand All @@ -147,31 +108,16 @@ RSpec/LeakyConstantDeclaration:
Exclude:
- 'spec/grape/validations/validators/except_values_spec.rb'

# Offense count: 2
# Offense count: 1
RSpec/MessageChain:
Exclude:
- 'spec/grape/middleware/formatter_spec.rb'

# Offense count: 144
# Configuration parameters: .
# SupportedStyles: have_received, receive
RSpec/MessageSpies:
EnforcedStyle: receive

# Offense count: 12
RSpec/MissingExampleGroupArgument:
Exclude:
- 'spec/grape/middleware/exception_spec.rb'

# Offense count: 17
# Configuration parameters: AllowedPatterns.
# AllowedPatterns: ^expect_, ^assert_
RSpec/NoExpectationExample:
Exclude:
- 'spec/grape/api_remount_spec.rb'
- 'spec/grape/api_spec.rb'
- 'spec/grape/validations_spec.rb'

# Offense count: 12
RSpec/RepeatedDescription:
Exclude:
Expand All @@ -180,10 +126,9 @@ RSpec/RepeatedDescription:
- 'spec/grape/validations/validators/allow_blank_spec.rb'
- 'spec/grape/validations/validators/values_spec.rb'

# Offense count: 8
# Offense count: 6
RSpec/RepeatedExample:
Exclude:
- 'spec/grape/api_spec.rb'
- 'spec/grape/middleware/versioner/accept_version_header_spec.rb'
- 'spec/grape/validations/validators/allow_blank_spec.rb'

Expand All @@ -195,13 +140,6 @@ RSpec/RepeatedExampleGroupDescription:
- 'spec/grape/util/inheritable_setting_spec.rb'
- 'spec/grape/validations/validators/values_spec.rb'

# Offense count: 2
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AutoCorrect.
RSpec/ScatteredSetup:
Exclude:
- 'spec/grape/util/inheritable_setting_spec.rb'

# Offense count: 5
RSpec/StubbedMock:
Exclude:
Expand All @@ -228,7 +166,7 @@ RSpec/SubjectStub:
- 'spec/grape/middleware/stack_spec.rb'
- 'spec/grape/parser_spec.rb'

# Offense count: 24
# Offense count: 23
# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
RSpec/VerifiedDoubles:
Exclude:
Expand All @@ -251,13 +189,6 @@ Style/CombinableLoops:
Exclude:
- 'spec/grape/endpoint_spec.rb'

# Offense count: 2
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: MaxUnannotatedPlaceholdersAllowed, AllowedMethods, AllowedPatterns.
# SupportedStyles: annotated, template, unannotated
Style/FormatStringToken:
EnforcedStyle: template

# Offense count: 12
# Configuration parameters: AllowedMethods.
# AllowedMethods: respond_to_missing?
Expand All @@ -274,55 +205,3 @@ Style/OptionalBooleanParameter:
- 'lib/grape/validations/types/dry_type_coercer.rb'
- 'lib/grape/validations/types/primitive_coercer.rb'
- 'lib/grape/validations/types/set_coercer.rb'

# Offense count: 29
# This cop supports safe autocorrection (--autocorrect).
Style/RedundantConstantBase:
Exclude:
- 'spec/grape/api/invalid_format_spec.rb'
- 'spec/grape/api_spec.rb'
- 'spec/grape/dsl/logger_spec.rb'
- 'spec/grape/endpoint/declared_spec.rb'
- 'spec/grape/endpoint_spec.rb'
- 'spec/grape/middleware/formatter_spec.rb'
- 'spec/grape/validations/validators/coerce_spec.rb'
- 'spec/grape/validations/validators/default_spec.rb'
- 'spec/integration/multi_json/json_spec.rb'
- 'spec/integration/multi_xml/xml_spec.rb'

# Offense count: 2
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: AllowAsExpressionSeparator.
Style/Semicolon:
Exclude:
- 'spec/grape/api_spec.rb'

# Offense count: 2
# This cop supports safe autocorrection (--autocorrect).
Style/SuperArguments:
Exclude:
- 'lib/grape/api.rb'
- 'spec/support/deprecated_warning_handlers.rb'

# Offense count: 2
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: AllowMethodsWithArguments, AllowedMethods, AllowedPatterns, AllowComments.
# AllowedMethods: define_method
Style/SymbolProc:
Exclude:
- 'benchmark/large_model.rb'
- 'spec/grape/validations/params_scope_spec.rb'

# Offense count: 1
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: forbid_for_all_comparison_operators, forbid_for_equality_operators_only, require_for_all_comparison_operators, require_for_equality_operators_only
Style/YodaCondition:
Exclude:
- 'lib/grape/api.rb'

# Offense count: 1
# This cop supports unsafe autocorrection (--autocorrect-all).
Style/ZeroLengthPredicate:
Exclude:
- 'lib/grape/validations/validators/exactly_one_of_validator.rb'
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#### Fixes

* [#2459](https://github.com/ruby-grape/grape/pull/2459): Autocorrect cops - [@ericproulx](https://github.com/ericproulx).
* [#3458](https://github.com/ruby-grape/grape/pull/2458): Remove unused Grape::Util::Accept::Header - [@ericproulx](https://github.com/ericproulx).
* Your contribution here.

Expand Down
2 changes: 1 addition & 1 deletion benchmark/large_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def self.vrp_request_configuration(this)
def self.vrp_request_partition(this)
this.requires(:method, type: String, values: %w[hierarchical_tree balanced_kmeans])
this.optional(:metric, type: Symbol)
this.optional(:entity, type: Symbol, values: %i[vehicle work_day], coerce_with: ->(value) { value.to_sym })
this.optional(:entity, type: Symbol, values: %i[vehicle work_day], coerce_with: lambda(&:to_sym))
this.optional(:threshold, type: Integer)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/grape/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def new(...)
def inherited(api)
super

api.initial_setup(Grape::API == self ? Grape::API::Instance : @base_instance)
api.initial_setup(self == Grape::API ? Grape::API::Instance : @base_instance)
api.override_all_methods!
end

Expand Down Expand Up @@ -108,7 +108,7 @@ def replay_setup_on(instance)
end

def respond_to?(method, include_private = false)
super(method, include_private) || base_instance.respond_to?(method, include_private)
super || base_instance.respond_to?(method, include_private)
end

def respond_to_missing?(method, include_private = false)
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/api/instance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def reset!
# Parses the API's definition and compiles it into an instance of
# Grape::API.
def compile
@instance ||= new
@instance ||= new # rubocop:disable Naming/MemoizedInstanceVariableName
end

# Wipe the compiled API so we can recompile after changes were made.
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/dsl/parameters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def declared_param?(param)

alias group requires

class EmptyOptionalValue; end
class EmptyOptionalValue; end # rubocop:disable Lint/EmptyClass

def map_params(params, element, is_array = false)
if params.is_a?(Array)
Expand Down
4 changes: 2 additions & 2 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ def endpoints
options[:app].endpoints if options[:app].respond_to?(:endpoints)
end

def equals?(e)
(options == e.options) && (inheritable_setting.to_hash == e.inheritable_setting.to_hash)
def equals?(endpoint)
(options == endpoint.options) && (inheritable_setting.to_hash == endpoint.inheritable_setting.to_hash)
end

protected
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/exceptions/validation_errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Grape
module Exceptions
class ValidationErrors < Grape::Exceptions::Base
ERRORS_FORMAT_KEY = 'grape.errors.format'
DEFAULT_ERRORS_FORMAT = '%{attributes} %{message}'
DEFAULT_ERRORS_FORMAT = '%<attributes>s %<message>s'

include Enumerable

Expand Down
2 changes: 1 addition & 1 deletion lib/grape/middleware/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def content_type
end

def mime_types
@mime_type ||= content_types.each_pair.with_object({}) do |(k, v), types_without_params|
@mime_types ||= content_types.each_pair.with_object({}) do |(k, v), types_without_params|
types_without_params[v.split(';').first] = k
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ def error_response(error = {})
rack_response(status, headers, format_message(message, backtrace, original_exception))
end

def default_rescue_handler(e)
error_response(message: e.message, backtrace: e.backtrace, original_exception: e)
def default_rescue_handler(exception)
error_response(message: exception.message, backtrace: exception.backtrace, original_exception: exception)
end

def rescue_handler_for_base_only_class(klass)
Expand Down
4 changes: 2 additions & 2 deletions lib/grape/middleware/stack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ def last
middlewares.last
end

def [](i)
middlewares[i]
def [](index)
middlewares[index]
end

def insert(index, *args, &block)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class ExactlyOneOfValidator < MultipleParamsBase
def validate_params!(params)
keys = keys_in_common(params)
return if keys.length == 1
raise Grape::Exceptions::Validation.new(params: all_keys, message: message(:exactly_one)) if keys.length.zero?
raise Grape::Exceptions::Validation.new(params: all_keys, message: message(:exactly_one)) if keys.empty?

raise Grape::Exceptions::Validation.new(params: keys, message: message(:mutual_exclusion))
end
Expand Down
6 changes: 3 additions & 3 deletions spec/grape/api/invalid_format_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,19 @@ def app
it 'no format' do
get '/foo'
expect(last_response.status).to eq 200
expect(last_response.body).to eq(::Grape::Json.dump(id: 'foo', format: nil))
expect(last_response.body).to eq(Grape::Json.dump(id: 'foo', format: nil))
end

it 'json format' do
get '/foo.json'
expect(last_response.status).to eq 200
expect(last_response.body).to eq(::Grape::Json.dump(id: 'foo', format: 'json'))
expect(last_response.body).to eq(Grape::Json.dump(id: 'foo', format: 'json'))
end

it 'invalid format' do
get '/foo.invalid'
expect(last_response.status).to eq 200
expect(last_response.body).to eq(::Grape::Json.dump(id: 'foo', format: 'invalid'))
expect(last_response.body).to eq(Grape::Json.dump(id: 'foo', format: 'invalid'))
end
end
end
4 changes: 3 additions & 1 deletion spec/grape/api_remount_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,13 +306,15 @@
tags ['not_configurable_tag', configuration[:a_configurable_tag]]
end
get 'location' do
'success'
route.tags
end
end
end

it 'mounts the endpoint with the appropiate tags' do
root_api.mount({ a_remounted_api => 'integer' }, with: { a_configurable_tag: 'a configured tag' })
get '/integer/location', param_key: 'a'
expect(JSON.parse(last_response.body)).to eq ['not_configurable_tag', 'a configured tag']
end
end

Expand Down
Loading

0 comments on commit 69d14ee

Please sign in to comment.