Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Rubocop rule to catch .any? without block #5169

Merged
merged 2 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cop/development/context_is_passed_cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

module Cop
module Development
class ContextIsPassedCop < RuboCop::Cop::Cop
class ContextIsPassedCop < RuboCop::Cop::Base
MSG = <<-MSG
This method also accepts `context` as an argument. Pass it so that the returned value will reflect the current query, or use another method that isn't context-dependent.
MSG
Expand Down
2 changes: 1 addition & 1 deletion cop/development/no_focus_cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
module Cop
module Development
# Make sure no tests are focused, from https://github.com/rubocop-hq/rubocop/issues/3773#issuecomment-420662102
class NoFocusCop < RuboCop::Cop::Cop
class NoFocusCop < RuboCop::Cop::Base
MSG = 'Remove `focus` from tests.'

def_node_matcher :focused?, <<-MATCHER
Expand Down
22 changes: 16 additions & 6 deletions cop/development/none_without_block_cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ module Development
# A custom Rubocop rule to catch uses of `.none?` without a block.
#
# @see https://github.com/rmosolgo/graphql-ruby/pull/2090
class NoneWithoutBlockCop < RuboCop::Cop::Cop
class NoneWithoutBlockCop < RuboCop::Cop::Base
MSG = <<-MD
Instead of `.none?` without a block:
Instead of `.none?` or `.any?` without a block:

- Use `.empty?` to check for an empty collection (faster)
- Add a block to explicitly check for `false` (more clear)

Run `-a` to replace this with `.empty?`.
Run `-a` to replace this with `%{bang}.empty?`.
MD
def on_block(node)
# Since this method was called with a block, it can't be
Expand All @@ -22,14 +22,24 @@ def on_block(node)
end

def on_send(node)
if !ignored_node?(node) && node.method_name == :none? && node.arguments.size == 0
add_offense(node)
if !ignored_node?(node) && (node.method_name == :none? || node.method_name == :any?) && node.arguments.size == 0
add_offense(node, message: MSG % { bang: node.method_name == :none? ? "" : "!.." } )
end
end

def autocorrect(node)
lambda do |corrector|
corrector.replace(node.location.selector, "empty?")
if node.method_name == :none?
corrector.replace(node.location.selector, "empty?")
else
# Backtrack to any chained method calls so we can insert `!` before them
full_exp = node
while node.parent.send_type?
full_exp = node.parent
end
new_source = "!" + full_exp.source_range.source.sub("any?", "empty?")
corrector.replace(full_exp, new_source)
end
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/graphql/analysis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ def analyze_query(query, analyzers, multiplex_analyzers: [])
.tap { _1.select!(&:analyze?) }

analyzers_to_run = query_analyzers + multiplex_analyzers
if analyzers_to_run.any?
if !analyzers_to_run.empty?

analyzers_to_run.select!(&:visit?)
if analyzers_to_run.any?
if !analyzers_to_run.empty?
visitor = GraphQL::Analysis::Visitor.new(
query: query,
analyzers: analyzers_to_run
Expand All @@ -69,7 +69,7 @@ def analyze_query(query, analyzers, multiplex_analyzers: [])
visitor.visit
end

if visitor.rescued_errors.any?
if !visitor.rescued_errors.empty?
return visitor.rescued_errors
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/analysis/visitor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ def leave_fragment_spread_inline(_fragment_spread)

def skip?(ast_node)
dir = ast_node.directives
dir.any? && !GraphQL::Execution::DirectiveChecks.include?(dir, query)
!dir.empty? && !GraphQL::Execution::DirectiveChecks.include?(dir, query)
end

def on_fragment_with_type(node)
Expand Down
10 changes: 5 additions & 5 deletions lib/graphql/dataloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ def run
next_source_fibers = []
first_pass = true
manager = spawn_fiber do
while first_pass || job_fibers.any?
while first_pass || !job_fibers.empty?
first_pass = false

while (f = (job_fibers.shift || (((next_job_fibers.size + job_fibers.size) < jobs_fiber_limit) && spawn_job_fiber)))
Expand All @@ -207,7 +207,7 @@ def run
end
join_queues(job_fibers, next_job_fibers)

while (source_fibers.any? || @source_cache.each_value.any? { |group_sources| group_sources.each_value.any?(&:pending?) })
while (!source_fibers.empty? || @source_cache.each_value.any? { |group_sources| group_sources.each_value.any?(&:pending?) })
while (f = source_fibers.shift || (((job_fibers.size + source_fibers.size + next_source_fibers.size + next_job_fibers.size) < total_fiber_limit) && spawn_source_fiber))
if f.alive?
finished = run_fiber(f)
Expand All @@ -227,10 +227,10 @@ def run
raise "Invariant: Manager fiber didn't terminate properly."
end

if job_fibers.any?
if !job_fibers.empty?
raise "Invariant: job fibers should have exited but #{job_fibers.size} remained"
end
if source_fibers.any?
if !source_fibers.empty?
raise "Invariant: source fibers should have exited but #{source_fibers.size} remained"
end
rescue UncaughtThrowError => e
Expand Down Expand Up @@ -270,7 +270,7 @@ def join_queues(prev_queue, new_queue)
end

def spawn_job_fiber
if @pending_jobs.any?
if !@pending_jobs.empty?
spawn_fiber do
while job = @pending_jobs.shift
job.call
Expand Down
4 changes: 2 additions & 2 deletions lib/graphql/dataloader/async_dataloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def run
first_pass = true
sources_condition = Async::Condition.new
manager = spawn_fiber do
while first_pass || job_fibers.any?
while first_pass || !job_fibers.empty?
first_pass = false
fiber_vars = get_fiber_variables

Expand All @@ -37,7 +37,7 @@ def run

Sync do |root_task|
set_fiber_variables(fiber_vars)
while source_tasks.any? || @source_cache.each_value.any? { |group_sources| group_sources.each_value.any?(&:pending?) }
while !source_tasks.empty? || @source_cache.each_value.any? { |group_sources| group_sources.each_value.any?(&:pending?) }
while (task = (source_tasks.shift || (((job_fibers.size + next_job_fibers.size + source_tasks.size + next_source_tasks.size) < total_fiber_limit) && spawn_source_task(root_task, sources_condition))))
if task.alive?
root_task.yield # give the source task a chance to run
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/dataloader/source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def load_all(values)
end
}

if pending_keys.size.positive?
if !pending_keys.empty?
sync(pending_keys)
end

Expand Down
8 changes: 4 additions & 4 deletions lib/graphql/execution/interpreter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def run_all(schema, query_options, context: {}, max_complexity: schema.max_compl
end
multiplex.dataloader.append_job {
operation = query.selected_operation
result = if operation.nil? || !query.valid? || query.context.errors.any?
result = if operation.nil? || !query.valid? || !query.context.errors.empty?
NO_OPERATION
else
begin
Expand Down Expand Up @@ -100,12 +100,12 @@ def run_all(schema, query_options, context: {}, max_complexity: schema.max_compl
# Then, find all errors and assign the result to the query object
results.each_with_index do |data_result, idx|
query = queries[idx]
if (events = query.context.namespace(:subscriptions)[:events]) && events.any?
if (events = query.context.namespace(:subscriptions)[:events]) && !events.empty?
schema.subscriptions.write_subscription(query, events)
end
# Assign the result so that it can be accessed in instrumentation
query.result_values = if data_result.equal?(NO_OPERATION)
if !query.valid? || query.context.errors.any?
if !query.valid? || !query.context.errors.empty?
# A bit weird, but `Query#static_errors` _includes_ `query.context.errors`
{ "errors" => query.static_errors.map(&:to_h) }
else
Expand All @@ -114,7 +114,7 @@ def run_all(schema, query_options, context: {}, max_complexity: schema.max_compl
else
result = {}

if query.context.errors.any?
if !query.context.errors.empty?
error_result = query.context.errors.map(&:to_h)
result["errors"] = error_result
end
Expand Down
6 changes: 3 additions & 3 deletions lib/graphql/execution/interpreter/resolve.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def self.resolve_each_depth(lazies_at_depth, dataloader)

if smallest_depth
lazies = lazies_at_depth.delete(smallest_depth)
if lazies.any?
if !lazies.empty?
dataloader.append_job {
lazies.each(&:value) # resolve these Lazy instances
}
Expand Down Expand Up @@ -55,7 +55,7 @@ def self.resolve(results, dataloader)
# these approaches.
dataloader.run
next_results = []
while results.any?
while !results.empty?
result_value = results.shift
if result_value.is_a?(Runtime::GraphQLResultHash) || result_value.is_a?(Hash)
results.concat(result_value.values)
Expand All @@ -81,7 +81,7 @@ def self.resolve(results, dataloader)
end
end

if next_results.any?
if !next_results.empty?
# Any pending data loader jobs may populate the
# resutl arrays or result hashes accumulated in
# `next_results``. Run those **to completion**
Expand Down
8 changes: 4 additions & 4 deletions lib/graphql/execution/interpreter/runtime.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def gather_selections(owner_object, owner_type, selections, selections_to_run =
end
else
# This is an InlineFragment or a FragmentSpread
if @runtime_directive_names.any? && node.directives.any? { |d| @runtime_directive_names.include?(d.name) }
if !@runtime_directive_names.empty? && node.directives.any? { |d| @runtime_directive_names.include?(d.name) }
next_selections = {}
next_selections[:graphql_directives] = node.directives
if selections_to_run
Expand Down Expand Up @@ -332,7 +332,7 @@ def evaluate_selection_with_args(arguments, field_defn, ast_node, field_ast_node
extra_args[extra] = field_defn.fetch_extra(extra, context)
end
end
if extra_args.any?
if !extra_args.empty?
resolved_arguments = resolved_arguments.merge_extras(extra_args)
end
resolved_arguments.keyword_arguments
Expand Down Expand Up @@ -361,7 +361,7 @@ def evaluate_selection_with_resolved_keyword_args(kwarg_arguments, resolved_argu
end

field_result = call_method_on_directives(:resolve, object, directives) do
if directives.any?
if !directives.empty?
# This might be executed in a different context; reset this info
runtime_state = get_current_runtime_state
runtime_state.current_field = field_defn
Expand Down Expand Up @@ -525,7 +525,7 @@ def continue_value(value, field, is_non_null, ast_node, result_name, selection_r
end
when Array
# It's an array full of execution errors; add them all.
if value.any? && value.all?(GraphQL::ExecutionError)
if !value.empty? && value.all?(GraphQL::ExecutionError)
list_type_at_all = (field && (field.type.list?))
if selection_result.nil? || !selection_result.graphql_dead
value.each_with_index do |error, index|
Expand Down
6 changes: 3 additions & 3 deletions lib/graphql/language/document_from_schema_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def build_schema_node

def build_object_type_node(object_type)
ints = @types.interfaces(object_type)
if ints.any?
if !ints.empty?
ints.sort_by!(&:graphql_name)
ints.map! { |iface| build_type_name_node(iface) }
end
Expand Down Expand Up @@ -247,7 +247,7 @@ def build_type_definition_node(type)
end

def build_argument_nodes(arguments)
if arguments.any?
if !arguments.empty?
nodes = arguments.map { |arg| build_argument_node(arg) }
nodes.sort_by!(&:name)
nodes
Expand All @@ -271,7 +271,7 @@ def build_definition_nodes
all_types = @types.all_types
type_nodes = build_type_definition_nodes(all_types)

if (ex_t = schema.extra_types).any?
if !(ex_t = schema.extra_types).empty?
dummy_query = Class.new(GraphQL::Schema::Object) do
graphql_name "DummyQuery"
(all_types + ex_t).each_with_index do |type, idx|
Expand Down
16 changes: 8 additions & 8 deletions lib/graphql/language/printer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def print_directive(directive)
print_string("@")
print_string(directive.name)

if directive.arguments.any?
if !directive.arguments.empty?
print_string("(")
directive.arguments.each_with_index do |a, i|
print_argument(a)
Expand All @@ -117,7 +117,7 @@ def print_field(field, indent: "")
print_string(": ")
end
print_string(field.name)
if field.arguments.any?
if !field.arguments.empty?
print_string("(")
field.arguments.each_with_index do |a, i|
print_argument(a)
Expand Down Expand Up @@ -182,7 +182,7 @@ def print_operation_definition(operation_definition, indent: "")
print_string(operation_definition.name)
end

if operation_definition.variables.any?
if !operation_definition.variables.empty?
print_string("(")
operation_definition.variables.each_with_index do |v, i|
print_variable_definition(v)
Expand Down Expand Up @@ -230,7 +230,7 @@ def print_schema_definition(schema, extension: false)

extension ? print_string("extend schema") : print_string("schema")

if schema.directives.any?
if !schema.directives.empty?
schema.directives.each do |dir|
print_string("\n ")
print_node(dir)
Expand Down Expand Up @@ -332,7 +332,7 @@ def print_interface_type_definition(interface_type, extension: false)
extension ? print_string("extend ") : print_description_and_comment(interface_type)
print_string("interface ")
print_string(interface_type.name)
print_implements(interface_type) if interface_type.interfaces.any?
print_implements(interface_type) if !interface_type.interfaces.empty?
print_directives(interface_type.directives)
print_field_definitions(interface_type.fields)
end
Expand All @@ -342,7 +342,7 @@ def print_union_type_definition(union_type, extension: false)
print_string("union ")
print_string(union_type.name)
print_directives(union_type.directives)
if union_type.types.any?
if !union_type.types.empty?
print_string(" = ")
i = 0
union_type.types.each do |t|
Expand All @@ -360,7 +360,7 @@ def print_enum_type_definition(enum_type, extension: false)
print_string("enum ")
print_string(enum_type.name)
print_directives(enum_type.directives)
if enum_type.values.any?
if !enum_type.values.empty?
print_string(" {\n")
enum_type.values.each.with_index do |value, i|
print_description(value, indent: " ", first_in_block: i == 0)
Expand Down Expand Up @@ -401,7 +401,7 @@ def print_directive_definition(directive)
print_string("directive @")
print_string(directive.name)

if directive.arguments.any?
if !directive.arguments.empty?
print_arguments(directive.arguments)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/graphql/query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def initialize(schema, query_string = nil, query: nil, document: nil, context: n
end
end

if context_tracers.any? && !(schema.trace_class <= GraphQL::Tracing::CallLegacyTracers)
if !context_tracers.empty? && !(schema.trace_class <= GraphQL::Tracing::CallLegacyTracers)
raise ArgumentError, "context[:tracers] are not supported without `trace_with(GraphQL::Tracing::CallLegacyTracers)` in the schema configuration, please add it."
end

Expand Down Expand Up @@ -479,7 +479,7 @@ def prepare_ast
@mutation = false
@subscription = false
operation_name_error = nil
if @operations.any?
if !@operations.empty?
@selected_operation = find_operation(@operations, @operation_name)
if @selected_operation.nil?
operation_name_error = GraphQL::Query::OperationNameMissingError.new(@operation_name)
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/query/context/scoped_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def dig(key, *other_keys)
each_present_path_ctx do |path_ctx|
if path_ctx.key?(key)
found_value = path_ctx[key]
if other_keys.any?
if !other_keys.empty?
return found_value.dig(*other_keys)
else
return found_value
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/query/variable_validation_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def initialize(variable_ast, type, value, validation_result, msg: nil)

msg ||= "Variable $#{variable_ast.name} of type #{type.to_type_signature} was provided invalid value"

if problem_fields.any?
if !problem_fields.empty?
msg += " for #{problem_fields.join(", ")}"
end

Expand Down
Loading
Loading