diff --git a/src/ameba/formatter/disabled_formatter.cr b/src/ameba/formatter/disabled_formatter.cr index cf777ee3e..ecab4aa74 100644 --- a/src/ameba/formatter/disabled_formatter.cr +++ b/src/ameba/formatter/disabled_formatter.cr @@ -5,7 +5,8 @@ module Ameba::Formatter output << "Disabled rules using inline directives:\n\n" sources.each do |source| - source.issues.select(&.disabled?).each do |issue| + source.issues.each do |issue| + next unless issue.disabled? next unless loc = issue.location output << "#{source.path}:#{loc.line_number}".colorize(:cyan) diff --git a/src/ameba/rule/lint/literal_in_interpolation.cr b/src/ameba/rule/lint/literal_in_interpolation.cr index fce215019..e5bbbd3f3 100644 --- a/src/ameba/rule/lint/literal_in_interpolation.cr +++ b/src/ameba/rule/lint/literal_in_interpolation.cr @@ -25,9 +25,13 @@ module Ameba::Rule::Lint MSG = "Literal value found in interpolation" def test(source, node : Crystal::StringInterpolation) - node.expressions - .select { |exp| !exp.is_a?(Crystal::StringLiteral) && literal?(exp) } - .each { |exp| issue_for exp, MSG } + each_literal_node(node) { |exp| issue_for exp, MSG } + end + + private def each_literal_node(node, &) + node.expressions.each do |exp| + yield exp if !exp.is_a?(Crystal::StringLiteral) && literal?(exp) + end end end end diff --git a/src/ameba/rule/lint/redundant_string_coercion.cr b/src/ameba/rule/lint/redundant_string_coercion.cr index 0a486084d..2dc4254be 100644 --- a/src/ameba/rule/lint/redundant_string_coercion.cr +++ b/src/ameba/rule/lint/redundant_string_coercion.cr @@ -30,18 +30,18 @@ module Ameba::Rule::Lint MSG = "Redundant use of `Object#to_s` in interpolation" def test(source, node : Crystal::StringInterpolation) - string_coercion_nodes(node).each do |expr| + each_string_coercion_node(node) do |expr| issue_for name_location(expr), expr.end_location, MSG end end - private def string_coercion_nodes(node) - node.expressions.select do |exp| - exp.is_a?(Crystal::Call) && - exp.name == "to_s" && - exp.args.size.zero? && - exp.named_args.nil? && - exp.obj + private def each_string_coercion_node(node, &) + node.expressions.each do |exp| + yield exp if exp.is_a?(Crystal::Call) && + exp.name == "to_s" && + exp.args.size.zero? && + exp.named_args.nil? && + exp.obj end end end diff --git a/src/ameba/rule/lint/shadowing_outer_local_var.cr b/src/ameba/rule/lint/shadowing_outer_local_var.cr index 1ac1c2393..672e0579b 100644 --- a/src/ameba/rule/lint/shadowing_outer_local_var.cr +++ b/src/ameba/rule/lint/shadowing_outer_local_var.cr @@ -52,7 +52,7 @@ module Ameba::Rule::Lint private def find_shadowing(source, scope) return unless outer_scope = scope.outer_scope - scope.arguments.reject(&.ignored?).each do |arg| + each_argument_node(scope) do |arg| # TODO: handle unpacked variables from `Block#unpacks` next unless name = arg.name.presence @@ -65,5 +65,11 @@ module Ameba::Rule::Lint issue_for arg.node, MSG % name end end + + private def each_argument_node(scope, &) + scope.arguments.each do |arg| + yield arg unless arg.ignored? + end + end end end diff --git a/src/ameba/rule/lint/shared_var_in_fiber.cr b/src/ameba/rule/lint/shared_var_in_fiber.cr index 2a2bd16e6..c68c2d4b3 100644 --- a/src/ameba/rule/lint/shared_var_in_fiber.cr +++ b/src/ameba/rule/lint/shared_var_in_fiber.cr @@ -75,11 +75,11 @@ module Ameba::Rule::Lint private def mutated_in_loop?(variable) declared_in = variable.assignments.first?.try &.branch - variable.assignments - .reject(&.scope.spawn_block?) - .any? do |assign| - assign.branch.try(&.in_loop?) && assign.branch != declared_in - end + variable.assignments.any? do |assign| + !assign.scope.spawn_block? && + assign.branch.try(&.in_loop?) && + assign.branch != declared_in + end end end end diff --git a/src/ameba/rule/naming/accessor_method_name.cr b/src/ameba/rule/naming/accessor_method_name.cr index 55b9cbd92..fc082737e 100644 --- a/src/ameba/rule/naming/accessor_method_name.cr +++ b/src/ameba/rule/naming/accessor_method_name.cr @@ -43,15 +43,7 @@ module Ameba::Rule::Naming MSG = "Favour method name '%s' over '%s'" def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) - defs = - case body = node.body - when Crystal::Def - [body] - when Crystal::Expressions - body.expressions.select(Crystal::Def) - end - - defs.try &.each do |def_node| + each_def_node(node) do |def_node| # skip defs with explicit receiver, as they'll be handled # by the `test(source, node : Crystal::Def)` overload check_issue(source, def_node) unless def_node.receiver @@ -63,6 +55,17 @@ module Ameba::Rule::Naming check_issue(source, node) if node.receiver end + private def each_def_node(node, &) + case body = node.body + when Crystal::Def + yield body + when Crystal::Expressions + body.expressions.each do |exp| + yield exp if exp.is_a?(Crystal::Def) + end + end + end + private def check_issue(source, node : Crystal::Def) case node.name when /^get_([a-z]\w*)$/ diff --git a/src/ameba/rule/naming/query_bool_methods.cr b/src/ameba/rule/naming/query_bool_methods.cr index a0ecaebb0..236e62211 100644 --- a/src/ameba/rule/naming/query_bool_methods.cr +++ b/src/ameba/rule/naming/query_bool_methods.cr @@ -38,17 +38,9 @@ module Ameba::Rule::Naming CALL_NAMES = %w[getter class_getter property class_property] def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) - calls = - case body = node.body - when Crystal::Call - [body] if body.name.in?(CALL_NAMES) - when Crystal::Expressions - body.expressions - .select(Crystal::Call) - .select!(&.name.in?(CALL_NAMES)) - end + each_call_node(node) do |exp| + next unless exp.name.in?(CALL_NAMES) - calls.try &.each do |exp| exp.args.each do |arg| name_node, is_bool = case arg @@ -66,5 +58,16 @@ module Ameba::Rule::Naming end end end + + private def each_call_node(node, &) + case body = node.body + when Crystal::Call + yield body + when Crystal::Expressions + body.expressions.each do |exp| + yield exp if exp.is_a?(Crystal::Call) + end + end + end end end diff --git a/src/ameba/runner.cr b/src/ameba/runner.cr index 5a526fac0..1923e0a01 100644 --- a/src/ameba/runner.cr +++ b/src/ameba/runner.cr @@ -170,10 +170,8 @@ module Ameba # runner.success? # => true or false # ``` def success? - @sources.all? do |source| - source.issues - .reject(&.disabled?) - .none?(&.rule.severity.<=(@severity)) + @sources.all? &.issues.none? do |issue| + issue.enabled? && issue.rule.severity <= @severity end end