From 1a091c1f1a5b6c52ee10c047975847c5f81fd198 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sun, 17 Jan 2021 13:16:04 +0100 Subject: [PATCH] Optimize rules (#185) * Replace pointless interpolations with actual values * Rules optimizations * Stylistic refactors * Remove extraneous blank lines * Remove some instances of .not_nil! usage --- spec/ameba/rule/lint/redundant_with_object_spec.cr | 2 +- src/ameba/cli/cmd.cr | 9 +++++++-- src/ameba/rule/layout/line_length.cr | 5 +---- src/ameba/rule/layout/trailing_blank_lines.cr | 1 - src/ameba/rule/layout/trailing_whitespace.cr | 4 +--- src/ameba/rule/lint/bad_directive.cr | 5 +++-- src/ameba/rule/lint/comparison_to_boolean.cr | 5 +---- src/ameba/rule/lint/debugger_statement.cr | 1 - src/ameba/rule/lint/empty_ensure.cr | 1 - src/ameba/rule/lint/empty_expression.cr | 4 +--- src/ameba/rule/lint/hash_duplicated_key.cr | 5 ++--- src/ameba/rule/lint/literal_in_condition.cr | 1 - src/ameba/rule/lint/literal_in_interpolation.cr | 1 - src/ameba/rule/lint/percent_array.cr | 1 - src/ameba/rule/lint/rand_zero.cr | 3 +-- src/ameba/rule/lint/redundant_string_coercion.cr | 1 - src/ameba/rule/lint/redundant_with_index.cr | 4 ++-- src/ameba/rule/lint/redundant_with_object.cr | 7 +++---- src/ameba/rule/lint/shadowed_argument.cr | 1 - src/ameba/rule/lint/shadowed_exception.cr | 1 - src/ameba/rule/lint/shadowing_local_outer_var.cr | 1 - src/ameba/rule/lint/shared_var_in_fiber.cr | 3 +-- src/ameba/rule/lint/syntax.cr | 1 - src/ameba/rule/lint/unneeded_disable_directive.cr | 4 ++-- src/ameba/rule/lint/unreachable_code.cr | 1 - src/ameba/rule/lint/unused_argument.cr | 1 - src/ameba/rule/lint/useless_assign.cr | 1 - src/ameba/rule/lint/useless_condition_in_when.cr | 6 +----- src/ameba/rule/metrics/cyclomatic_complexity.cr | 14 +++++++------- src/ameba/rule/performance/any_after_filter.cr | 8 ++++---- .../rule/performance/first_last_after_filter.cr | 7 +++---- src/ameba/rule/performance/size_after_filter.cr | 8 ++++---- src/ameba/rule/style/constant_names.cr | 3 +-- src/ameba/rule/style/is_a_nil.cr | 5 +++-- src/ameba/rule/style/large_numbers.cr | 5 ++--- src/ameba/rule/style/method_names.cr | 3 +-- .../rule/style/negated_conditions_in_unless.cr | 4 +--- src/ameba/rule/style/predicate_name.cr | 1 - src/ameba/rule/style/redundant_begin.cr | 5 +---- src/ameba/rule/style/type_names.cr | 3 +-- src/ameba/rule/style/unless_else.cr | 4 +--- src/ameba/rule/style/variable_names.cr | 1 - src/ameba/rule/style/while_true.cr | 4 +--- 43 files changed, 57 insertions(+), 98 deletions(-) diff --git a/spec/ameba/rule/lint/redundant_with_object_spec.cr b/spec/ameba/rule/lint/redundant_with_object_spec.cr index f6adc130..bc3a9baf 100644 --- a/spec/ameba/rule/lint/redundant_with_object_spec.cr +++ b/spec/ameba/rule/lint/redundant_with_object_spec.cr @@ -77,7 +77,7 @@ module Ameba::Rule::Lint issue.rule.should_not be_nil issue.location.to_s.should eq "source.cr:2:14" issue.end_location.to_s.should eq "source.cr:2:30" - issue.message.should eq "Use each instead of each_with_object" + issue.message.should eq "Use `each` instead of `each_with_object`" end end end diff --git a/src/ameba/cli/cmd.cr b/src/ameba/cli/cmd.cr index 7b9e6bae..d32543c4 100644 --- a/src/ameba/cli/cmd.cr +++ b/src/ameba/cli/cmd.cr @@ -8,8 +8,13 @@ module Ameba::Cli def run(args = ARGV) opts = parse_args args config = Config.load opts.config, opts.colors? - config.globs = opts.globs.not_nil! if opts.globs - config.severity = opts.fail_level.not_nil! if opts.fail_level + + if globs = opts.globs + config.globs = globs + end + if fail_level = opts.fail_level + config.severity = fail_level + end configure_formatter(config, opts) configure_rules(config, opts) diff --git a/src/ameba/rule/layout/line_length.cr b/src/ameba/rule/layout/line_length.cr index a4c24d37..df843070 100644 --- a/src/ameba/rule/layout/line_length.cr +++ b/src/ameba/rule/layout/line_length.cr @@ -8,7 +8,6 @@ module Ameba::Rule::Layout # Enabled: true # MaxLength: 100 # ``` - # struct LineLength < Base properties do enabled false @@ -20,9 +19,7 @@ module Ameba::Rule::Layout def test(source) source.lines.each_with_index do |line, index| - next unless line.size > max_length - - issue_for({index + 1, max_length + 1}, MSG) + issue_for({index + 1, max_length + 1}, MSG) if line.size > max_length end end end diff --git a/src/ameba/rule/layout/trailing_blank_lines.cr b/src/ameba/rule/layout/trailing_blank_lines.cr index 24499e9c..bf2db9d6 100644 --- a/src/ameba/rule/layout/trailing_blank_lines.cr +++ b/src/ameba/rule/layout/trailing_blank_lines.cr @@ -7,7 +7,6 @@ module Ameba::Rule::Layout # Layout/TrailingBlankLines: # Enabled: true # ``` - # struct TrailingBlankLines < Base properties do description "Disallows trailing blank lines" diff --git a/src/ameba/rule/layout/trailing_whitespace.cr b/src/ameba/rule/layout/trailing_whitespace.cr index a4df797e..77456c1a 100644 --- a/src/ameba/rule/layout/trailing_whitespace.cr +++ b/src/ameba/rule/layout/trailing_whitespace.cr @@ -7,7 +7,6 @@ module Ameba::Rule::Layout # Layout/TrailingWhitespace: # Enabled: true # ``` - # struct TrailingWhitespace < Base properties do description "Disallows trailing whitespaces" @@ -17,8 +16,7 @@ module Ameba::Rule::Layout def test(source) source.lines.each_with_index do |line, index| - next unless line =~ /\s$/ - issue_for({index + 1, line.size}, MSG) + issue_for({index + 1, line.size}, MSG) if line =~ /\s$/ end end end diff --git a/src/ameba/rule/lint/bad_directive.cr b/src/ameba/rule/lint/bad_directive.cr index 76b6d358..1fcfd896 100644 --- a/src/ameba/rule/lint/bad_directive.cr +++ b/src/ameba/rule/lint/bad_directive.cr @@ -17,7 +17,6 @@ module Ameba::Rule::Lint # Lint/BadDirective: # Enabled: true # ``` - # struct BadDirective < Base properties do description "Reports bad comment directives" @@ -48,7 +47,9 @@ module Ameba::Rule::Lint private def check_rules(source, token, rules) bad_names = rules - ALL_RULE_NAMES - ALL_GROUP_NAMES - issue_for token, "Such rules do not exist: %s" % bad_names.join(", ") unless bad_names.empty? + return if bad_names.empty? + + issue_for token, "Such rules do not exist: %s" % bad_names.join(", ") end end end diff --git a/src/ameba/rule/lint/comparison_to_boolean.cr b/src/ameba/rule/lint/comparison_to_boolean.cr index 313db5a2..649d61a5 100644 --- a/src/ameba/rule/lint/comparison_to_boolean.cr +++ b/src/ameba/rule/lint/comparison_to_boolean.cr @@ -19,7 +19,6 @@ module Ameba::Rule::Lint # Lint/ComparisonToBoolean: # Enabled: true # ``` - # struct ComparisonToBoolean < Base properties do enabled false @@ -33,9 +32,7 @@ module Ameba::Rule::Lint to_boolean = node.args.first?.try &.is_a?(Crystal::BoolLiteral) || node.obj.is_a?(Crystal::BoolLiteral) - return unless comparison && to_boolean - - issue_for node, MSG + issue_for node, MSG if comparison && to_boolean end end end diff --git a/src/ameba/rule/lint/debugger_statement.cr b/src/ameba/rule/lint/debugger_statement.cr index 97674490..b5131384 100644 --- a/src/ameba/rule/lint/debugger_statement.cr +++ b/src/ameba/rule/lint/debugger_statement.cr @@ -10,7 +10,6 @@ module Ameba::Rule::Lint # Lint/DebuggerStatement: # Enabled: true # ``` - # struct DebuggerStatement < Base properties do description "Disallows calls to debugger" diff --git a/src/ameba/rule/lint/empty_ensure.cr b/src/ameba/rule/lint/empty_ensure.cr index f8fdeb48..94deeb36 100644 --- a/src/ameba/rule/lint/empty_ensure.cr +++ b/src/ameba/rule/lint/empty_ensure.cr @@ -38,7 +38,6 @@ module Ameba::Rule::Lint # Lint/EmptyEnsure # Enabled: true # ``` - # struct EmptyEnsure < Base properties do description "Disallows empty ensure statement" diff --git a/src/ameba/rule/lint/empty_expression.cr b/src/ameba/rule/lint/empty_expression.cr index 682ed8a3..e3fae35a 100644 --- a/src/ameba/rule/lint/empty_expression.cr +++ b/src/ameba/rule/lint/empty_expression.cr @@ -27,7 +27,6 @@ module Ameba::Rule::Lint # Lint/EmptyExpression: # Enabled: true # ``` - # struct EmptyExpression < Base include AST::Util @@ -41,8 +40,7 @@ module Ameba::Rule::Lint def test(source, node : Crystal::NilLiteral) exp = node_source(node, source.lines).try &.join - - return if exp.nil? || exp == "nil" + return if exp.in?(nil, "nil") issue_for node, MSG % exp end diff --git a/src/ameba/rule/lint/hash_duplicated_key.cr b/src/ameba/rule/lint/hash_duplicated_key.cr index c9de6a78..6a581b4f 100644 --- a/src/ameba/rule/lint/hash_duplicated_key.cr +++ b/src/ameba/rule/lint/hash_duplicated_key.cr @@ -19,7 +19,6 @@ module Ameba::Rule::Lint # Lint/HashDuplicatedKey: # Enabled: true # ``` - # struct HashDuplicatedKey < Base properties do description "Disallows duplicated keys in hash literals" @@ -28,7 +27,7 @@ module Ameba::Rule::Lint MSG = "Duplicated keys in hash literal: %s" def test(source, node : Crystal::HashLiteral) - return unless (keys = duplicated_keys(node.entries)).any? + return if (keys = duplicated_keys(node.entries)).empty? issue_for node, MSG % keys.join(", ") end @@ -36,7 +35,7 @@ module Ameba::Rule::Lint private def duplicated_keys(entries) entries.map(&.key) .group_by(&.itself) - .select { |_, v| v.size > 1 } + .tap(&.select! { |_, v| v.size > 1 }) .map { |k, _| k } end end diff --git a/src/ameba/rule/lint/literal_in_condition.cr b/src/ameba/rule/lint/literal_in_condition.cr index 2dc2e600..15478516 100644 --- a/src/ameba/rule/lint/literal_in_condition.cr +++ b/src/ameba/rule/lint/literal_in_condition.cr @@ -19,7 +19,6 @@ module Ameba::Rule::Lint # Lint/LiteralInCondition: # Enabled: true # ``` - # struct LiteralInCondition < Base include AST::Util diff --git a/src/ameba/rule/lint/literal_in_interpolation.cr b/src/ameba/rule/lint/literal_in_interpolation.cr index df674f74..6c96c1c8 100644 --- a/src/ameba/rule/lint/literal_in_interpolation.cr +++ b/src/ameba/rule/lint/literal_in_interpolation.cr @@ -15,7 +15,6 @@ module Ameba::Rule::Lint # Lint/LiteralInInterpolation # Enabled: true # ``` - # struct LiteralInInterpolation < Base include AST::Util diff --git a/src/ameba/rule/lint/percent_array.cr b/src/ameba/rule/lint/percent_array.cr index afdf0223..b8bb1b54 100644 --- a/src/ameba/rule/lint/percent_array.cr +++ b/src/ameba/rule/lint/percent_array.cr @@ -23,7 +23,6 @@ module Ameba::Rule::Lint # StringArrayUnwantedSymbols: ',"' # SymbolArrayUnwantedSymbols: ',:' # ``` - # struct PercentArrays < Base properties do description "Disallows some unwanted symbols in percent array literals" diff --git a/src/ameba/rule/lint/rand_zero.cr b/src/ameba/rule/lint/rand_zero.cr index 6c2b70f2..efd8ab7b 100644 --- a/src/ameba/rule/lint/rand_zero.cr +++ b/src/ameba/rule/lint/rand_zero.cr @@ -22,7 +22,6 @@ module Ameba::Rule::Lint # Lint/RandZero: # Enabled: true # ``` - # struct RandZero < Base properties do description "Disallows rand zero calls" @@ -36,7 +35,7 @@ module Ameba::Rule::Lint (arg = node.args.first) && (arg.is_a? Crystal::NumberLiteral) && (value = arg.value) && - (value == "0" || value == "1") + value.in?("0", "1") issue_for node, MSG % node end diff --git a/src/ameba/rule/lint/redundant_string_coercion.cr b/src/ameba/rule/lint/redundant_string_coercion.cr index f21484a2..4e85de49 100644 --- a/src/ameba/rule/lint/redundant_string_coercion.cr +++ b/src/ameba/rule/lint/redundant_string_coercion.cr @@ -20,7 +20,6 @@ module Ameba::Rule::Lint # Lint/RedundantStringCoersion # Enabled: true # ``` - # struct RedundantStringCoercion < Base include AST::Util diff --git a/src/ameba/rule/lint/redundant_with_index.cr b/src/ameba/rule/lint/redundant_with_index.cr index d9c646f3..35d3ffc9 100644 --- a/src/ameba/rule/lint/redundant_with_index.cr +++ b/src/ameba/rule/lint/redundant_with_index.cr @@ -26,7 +26,6 @@ module Ameba::Rule::Lint # Lint/RedundantWithIndex: # Enabled: true # ``` - # struct RedundantWithIndex < Base properties do description "Disallows redundant `with_index` calls" @@ -35,7 +34,8 @@ module Ameba::Rule::Lint def test(source, node : Crystal::Call) args, block = node.args, node.block - return if args.size > 1 || block.nil? || with_index_arg?(block.not_nil!) + return if block.nil? || args.size > 1 + return if with_index_arg?(block) case node.name when "with_index" diff --git a/src/ameba/rule/lint/redundant_with_object.cr b/src/ameba/rule/lint/redundant_with_object.cr index 357e45d2..585e6384 100644 --- a/src/ameba/rule/lint/redundant_with_object.cr +++ b/src/ameba/rule/lint/redundant_with_object.cr @@ -27,21 +27,20 @@ module Ameba::Rule::Lint # Lint/RedundantWithObject: # Enabled: true # ``` - # struct RedundantWithObject < Base properties do description "Disallows redundant `with_object` calls" end + MSG = "Use `each` instead of `each_with_object`" + def test(source, node : Crystal::Call) return if node.name != "each_with_object" || node.args.size != 1 || node.block.nil? || with_index_arg?(node.block.not_nil!) - issue_for node.name_location, - node.name_end_location, - "Use each instead of each_with_object" + issue_for node.name_location, node.name_end_location, MSG end private def with_index_arg?(block : Crystal::Block) diff --git a/src/ameba/rule/lint/shadowed_argument.cr b/src/ameba/rule/lint/shadowed_argument.cr index 0c27c25d..444c24e4 100644 --- a/src/ameba/rule/lint/shadowed_argument.cr +++ b/src/ameba/rule/lint/shadowed_argument.cr @@ -35,7 +35,6 @@ module Ameba::Rule::Lint # Lint/ShadowedArgument: # Enabled: true # ``` - # struct ShadowedArgument < Base properties do description "Disallows shadowed arguments" diff --git a/src/ameba/rule/lint/shadowed_exception.cr b/src/ameba/rule/lint/shadowed_exception.cr index 53570340..a937f8bb 100644 --- a/src/ameba/rule/lint/shadowed_exception.cr +++ b/src/ameba/rule/lint/shadowed_exception.cr @@ -33,7 +33,6 @@ module Ameba::Rule::Lint # Lint/ShadowedException: # Enabled: true # ``` - # struct ShadowedException < Base properties do description "Disallows rescued exception that get shadowed" diff --git a/src/ameba/rule/lint/shadowing_local_outer_var.cr b/src/ameba/rule/lint/shadowing_local_outer_var.cr index 846cc484..dbfc4da0 100644 --- a/src/ameba/rule/lint/shadowing_local_outer_var.cr +++ b/src/ameba/rule/lint/shadowing_local_outer_var.cr @@ -30,7 +30,6 @@ module Ameba::Rule::Lint # Lint/ShadowingOuterLocalVar: # Enabled: true # ``` - # struct ShadowingOuterLocalVar < Base properties do description "Disallows the usage of the same name as outer local variables" \ diff --git a/src/ameba/rule/lint/shared_var_in_fiber.cr b/src/ameba/rule/lint/shared_var_in_fiber.cr index 8011fa03..56cc465b 100644 --- a/src/ameba/rule/lint/shared_var_in_fiber.cr +++ b/src/ameba/rule/lint/shared_var_in_fiber.cr @@ -49,7 +49,6 @@ module Ameba::Rule::Lint # Lint/SharedVarInFiber: # Enabled: true # ``` - # struct SharedVarInFiber < Base properties do description "Disallows shared variables in fibers." @@ -77,7 +76,7 @@ module Ameba::Rule::Lint declared_in = variable.assignments.first?.try &.branch variable.assignments - .reject { |assign| assign.scope.spawn_block? } + .reject(&.scope.spawn_block?) .any? do |assign| assign.branch.try(&.in_loop?) && assign.branch != declared_in end diff --git a/src/ameba/rule/lint/syntax.cr b/src/ameba/rule/lint/syntax.cr index 8d135401..6c4efa48 100644 --- a/src/ameba/rule/lint/syntax.cr +++ b/src/ameba/rule/lint/syntax.cr @@ -18,7 +18,6 @@ module Ameba::Rule::Lint # rescue e : Exception # end # ``` - # struct Syntax < Base properties do description "Reports invalid Crystal syntax" diff --git a/src/ameba/rule/lint/unneeded_disable_directive.cr b/src/ameba/rule/lint/unneeded_disable_directive.cr index 6128fbeb..7016f769 100644 --- a/src/ameba/rule/lint/unneeded_disable_directive.cr +++ b/src/ameba/rule/lint/unneeded_disable_directive.cr @@ -24,7 +24,6 @@ module Ameba::Rule::Lint # Lint/UnneededDisableDirective # Enabled: true # ``` - # struct UnneededDisableDirective < Base properties do description "Reports unneeded disable directives in comments" @@ -47,11 +46,12 @@ module Ameba::Rule::Lint return unless directive[:action] == "disable" directive[:rules].reject do |rule_name| + next if rule_name == self.name source.issues.any? do |issue| issue.rule.name == rule_name && issue.disabled? && issue_at_location?(source, issue, location) - end && rule_name != self.name + end end end diff --git a/src/ameba/rule/lint/unreachable_code.cr b/src/ameba/rule/lint/unreachable_code.cr index ded09bf5..69f3731f 100644 --- a/src/ameba/rule/lint/unreachable_code.cr +++ b/src/ameba/rule/lint/unreachable_code.cr @@ -41,7 +41,6 @@ module Ameba::Rule::Lint # Lint/UnreachableCode: # Enabled: true # ``` - # struct UnreachableCode < Base include AST::Util diff --git a/src/ameba/rule/lint/unused_argument.cr b/src/ameba/rule/lint/unused_argument.cr index 6abadca8..9615ec4c 100644 --- a/src/ameba/rule/lint/unused_argument.cr +++ b/src/ameba/rule/lint/unused_argument.cr @@ -24,7 +24,6 @@ module Ameba::Rule::Lint # IgnoreBlocks: false # IgnoreProcs: false # ``` - # struct UnusedArgument < Base properties do description "Disallows unused arguments" diff --git a/src/ameba/rule/lint/useless_assign.cr b/src/ameba/rule/lint/useless_assign.cr index bff18188..23f69d4f 100644 --- a/src/ameba/rule/lint/useless_assign.cr +++ b/src/ameba/rule/lint/useless_assign.cr @@ -25,7 +25,6 @@ module Ameba::Rule::Lint # Lint/UselessAssign: # Enabled: true # ``` - # struct UselessAssign < Base properties do description "Disallows useless variable assignments" diff --git a/src/ameba/rule/lint/useless_condition_in_when.cr b/src/ameba/rule/lint/useless_condition_in_when.cr index 4b2fb36c..f9f40ca6 100644 --- a/src/ameba/rule/lint/useless_condition_in_when.cr +++ b/src/ameba/rule/lint/useless_condition_in_when.cr @@ -30,7 +30,6 @@ module Ameba::Rule::Lint # Lint/UselessConditionInWhen: # Enabled: true # ``` - # struct UselessConditionInWhen < Base properties do description "Disallows useless conditions in when" @@ -43,10 +42,7 @@ module Ameba::Rule::Lint # simple implementation in future. protected def check_node(source, when_node, cond) cond_s = cond.to_s - return if when_node - .conds - .map(&.to_s) - .none? { |c| c == cond_s } + return if when_node.conds.none?(&.to_s.==(cond_s)) issue_for cond, MSG end diff --git a/src/ameba/rule/metrics/cyclomatic_complexity.cr b/src/ameba/rule/metrics/cyclomatic_complexity.cr index fb2e2fe2..167ebfe7 100644 --- a/src/ameba/rule/metrics/cyclomatic_complexity.cr +++ b/src/ameba/rule/metrics/cyclomatic_complexity.cr @@ -8,7 +8,6 @@ module Ameba::Rule::Metrics # Enabled: true # MaxComplexity: 10 # ``` - # struct CyclomaticComplexity < Base properties do description "Disallows methods with a cyclomatic complexity higher than `MaxComplexity`" @@ -21,17 +20,18 @@ module Ameba::Rule::Metrics complexity = AST::CountingVisitor.new(node).count if complexity > max_complexity && (location = node.name_location) - issue_for( - location, - def_name_end_location(node), - MSG % {complexity, max_complexity} - ) + issue_for location, def_name_end_location(node), MSG % { + complexity, max_complexity, + } end end private def def_name_end_location(node) return unless location = node.name_location - line_number, column_number = location.line_number, location.column_number + + line_number, column_number = + location.line_number, location.column_number + Crystal::Location.new(location.filename, line_number, column_number + node.name.size) end end diff --git a/src/ameba/rule/performance/any_after_filter.cr b/src/ameba/rule/performance/any_after_filter.cr index 5e4ae077..b8aa3f34 100644 --- a/src/ameba/rule/performance/any_after_filter.cr +++ b/src/ameba/rule/performance/any_after_filter.cr @@ -24,10 +24,9 @@ module Ameba::Rule::Performance # - select # - reject # ``` - # struct AnyAfterFilter < Base ANY_NAME = "any?" - MSG = "Use `#{ANY_NAME} {...}` instead of `%s {...}.#{ANY_NAME}`" + MSG = "Use `any? {...}` instead of `%s {...}.any?`" properties do filter_names : Array(String) = %w(select reject) @@ -36,9 +35,10 @@ module Ameba::Rule::Performance def test(source, node : Crystal::Call) return unless node.name == ANY_NAME && (obj = node.obj) + return unless obj.is_a?(Crystal::Call) + return if obj.block.nil? || !node.block.nil? - if node.block.nil? && obj.is_a?(Crystal::Call) && - filter_names.includes?(obj.name) && !obj.block.nil? + if filter_names.includes?(obj.name) issue_for obj.name_location, node.name_end_location, MSG % obj.name end end diff --git a/src/ameba/rule/performance/first_last_after_filter.cr b/src/ameba/rule/performance/first_last_after_filter.cr index a3599423..78f3302a 100644 --- a/src/ameba/rule/performance/first_last_after_filter.cr +++ b/src/ameba/rule/performance/first_last_after_filter.cr @@ -23,7 +23,6 @@ module Ameba::Rule::Performance # FilterNames: # - select # ``` - # struct FirstLastAfterFilter < Base CALL_NAMES = %w(first last first? last?) MSG = "Use `find {...}` instead of `%s {...}.%s`" @@ -45,10 +44,10 @@ module Ameba::Rule::Performance def test(source, node : Crystal::Call) return unless CALL_NAMES.includes?(node.name) && (obj = node.obj) - return if node.args.any? + return unless obj.is_a?(Crystal::Call) + return if obj.block.nil? || !node.block.nil? || node.args.any? - if node.block.nil? && obj.is_a?(Crystal::Call) && - filter_names.includes?(obj.name) && !obj.block.nil? + if filter_names.includes?(obj.name) message = node.name.includes?(CALL_NAMES.first) ? MSG : MSG_REVERSE issue_for obj.name_location, node.name_end_location, message % {obj.name, node.name} end diff --git a/src/ameba/rule/performance/size_after_filter.cr b/src/ameba/rule/performance/size_after_filter.cr index 1fe26359..6a281432 100644 --- a/src/ameba/rule/performance/size_after_filter.cr +++ b/src/ameba/rule/performance/size_after_filter.cr @@ -30,10 +30,9 @@ module Ameba::Rule::Performance # - select # - reject # ``` - # struct SizeAfterFilter < Base SIZE_NAME = "size" - MSG = "Use `count {...}` instead of `%s {...}.#{SIZE_NAME}`." + MSG = "Use `count {...}` instead of `%s {...}.size`." properties do filter_names : Array(String) = %w(select reject) @@ -51,9 +50,10 @@ module Ameba::Rule::Performance def test(source, node : Crystal::Call) return unless node.name == SIZE_NAME && (obj = node.obj) + return unless obj.is_a?(Crystal::Call) + return if obj.block.nil? - if obj.is_a?(Crystal::Call) && - filter_names.includes?(obj.name) && !obj.block.nil? + if filter_names.includes?(obj.name) issue_for obj.name_location, node.name_end_location, MSG % obj.name end end diff --git a/src/ameba/rule/style/constant_names.cr b/src/ameba/rule/style/constant_names.cr index 3628006e..fdd7caad 100644 --- a/src/ameba/rule/style/constant_names.cr +++ b/src/ameba/rule/style/constant_names.cr @@ -21,7 +21,6 @@ module Ameba::Rule::Style # Style/ConstantNames: # Enabled: true # ``` - # struct ConstantNames < Base properties do description "Enforces constant names to be in screaming case" @@ -34,7 +33,7 @@ module Ameba::Rule::Style name = target.names.first expected = name.upcase - return if expected == name || name.camelcase == name + return if name.in?(expected, name.camelcase) issue_for target, MSG % {expected, name} end diff --git a/src/ameba/rule/style/is_a_nil.cr b/src/ameba/rule/style/is_a_nil.cr index 18653bbc..cbf4aced 100644 --- a/src/ameba/rule/style/is_a_nil.cr +++ b/src/ameba/rule/style/is_a_nil.cr @@ -19,7 +19,6 @@ module Ameba::Rule::Style # Style/IsANil: # Enabled: true # ``` - # struct IsANil < Base properties do description "Disallows calls to `is_a?(Nil)` in favor of `nil?`" @@ -32,7 +31,9 @@ module Ameba::Rule::Style return if node.nil_check? const = node.const - issue_for const, MSG if const.is_a?(Crystal::Path) && const.names == PATH_NIL_NAMES + return unless const.is_a?(Crystal::Path) && const.names == PATH_NIL_NAMES + + issue_for const, MSG end end end diff --git a/src/ameba/rule/style/large_numbers.cr b/src/ameba/rule/style/large_numbers.cr index a9a2b33d..caba735f 100644 --- a/src/ameba/rule/style/large_numbers.cr +++ b/src/ameba/rule/style/large_numbers.cr @@ -26,7 +26,6 @@ module Ameba::Rule::Style # Enabled: true # IntMinDigits: 5 # i.e. integers higher than 9999 # ``` - # struct LargeNumbers < Base properties do description "Disallows usage of large numbers without underscore" @@ -53,7 +52,7 @@ module Ameba::Rule::Style end private def allowed?(_sign, value, fraction, _suffix) - return true if !fraction.nil? && fraction.size > 3 + return true if fraction && fraction.size > 3 digits = value.chars.select &.to_s.=~ /[0-9]/ digits.size >= int_min_digits @@ -71,7 +70,7 @@ module Ameba::Rule::Style value.chars.reject(&.== '_').each_slice(by) do |slice| slices << (yield slice).join end - end.join("_") + end.join('_') end private def parse_number(value) diff --git a/src/ameba/rule/style/method_names.cr b/src/ameba/rule/style/method_names.cr index 905de373..c53a0f9a 100644 --- a/src/ameba/rule/style/method_names.cr +++ b/src/ameba/rule/style/method_names.cr @@ -37,7 +37,6 @@ module Ameba::Rule::Style # Style/MethodNames: # Enabled: true # ``` - # struct MethodNames < Base properties do description "Enforces method names to be in underscored case" @@ -51,7 +50,7 @@ module Ameba::Rule::Style line_number = node.location.try &.line_number column_number = node.name_location.try &.column_number - return if line_number.nil? || column_number.nil? + return unless line_number && column_number issue_for( {line_number, column_number}, diff --git a/src/ameba/rule/style/negated_conditions_in_unless.cr b/src/ameba/rule/style/negated_conditions_in_unless.cr index 8781b36a..7225c5df 100644 --- a/src/ameba/rule/style/negated_conditions_in_unless.cr +++ b/src/ameba/rule/style/negated_conditions_in_unless.cr @@ -26,7 +26,6 @@ module Ameba::Rule::Style # Style/NegatedConditionsInUnless: # Enabled: true # ``` - # struct NegatedConditionsInUnless < Base properties do description "Disallows negated conditions in unless" @@ -35,8 +34,7 @@ module Ameba::Rule::Style MSG = "Avoid negated conditions in unless blocks" def test(source, node : Crystal::Unless) - return unless negated_condition? node.cond - issue_for node, MSG + issue_for node, MSG if negated_condition?(node.cond) end private def negated_condition?(node) diff --git a/src/ameba/rule/style/predicate_name.cr b/src/ameba/rule/style/predicate_name.cr index b23d938c..a7db3ef6 100644 --- a/src/ameba/rule/style/predicate_name.cr +++ b/src/ameba/rule/style/predicate_name.cr @@ -28,7 +28,6 @@ module Ameba::Rule::Style # Style/PredicateName: # Enabled: true # ``` - # struct PredicateName < Base properties do description "Disallows tautological predicate names" diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr index 1318bb4b..db6902d8 100644 --- a/src/ameba/rule/style/redundant_begin.cr +++ b/src/ameba/rule/style/redundant_begin.cr @@ -55,7 +55,6 @@ module Ameba::Rule::Style # Style/RedundantBegin: # Enabled: true # ``` - # struct RedundantBegin < Base include AST::Util properties do @@ -65,9 +64,7 @@ module Ameba::Rule::Style MSG = "Redundant `begin` block detected" def test(source, node : Crystal::Def) - return unless redundant_begin?(source, node) - - issue_for node, MSG + issue_for node, MSG if redundant_begin?(source, node) end private def redundant_begin?(source, node) diff --git a/src/ameba/rule/style/type_names.cr b/src/ameba/rule/style/type_names.cr index 2daa28f8..8cc8bb0d 100644 --- a/src/ameba/rule/style/type_names.cr +++ b/src/ameba/rule/style/type_names.cr @@ -51,7 +51,6 @@ module Ameba::Rule::Style # Style/TypeNames: # Enabled: true # ``` - # struct TypeNames < Base properties do description "Enforces type names in camelcase manner" @@ -62,7 +61,7 @@ module Ameba::Rule::Style private def check_node(source, node) name = node.name.to_s expected = name.camelcase - return if expected == name + return if name == expected issue_for node, MSG % {expected, name} end diff --git a/src/ameba/rule/style/unless_else.cr b/src/ameba/rule/style/unless_else.cr index 55909cdc..3583c79c 100644 --- a/src/ameba/rule/style/unless_else.cr +++ b/src/ameba/rule/style/unless_else.cr @@ -42,7 +42,6 @@ module Ameba::Rule::Style # Style/UnlessElse: # Enabled: true # ``` - # struct UnlessElse < Base properties do description "Disallows the use of an `else` block with the `unless`" @@ -51,8 +50,7 @@ module Ameba::Rule::Style MSG = "Favour if over unless with else" def test(source, node : Crystal::Unless) - return if node.else.nop? - issue_for node, MSG + issue_for node, MSG unless node.else.nop? end end end diff --git a/src/ameba/rule/style/variable_names.cr b/src/ameba/rule/style/variable_names.cr index 108c54d0..3d0efa34 100644 --- a/src/ameba/rule/style/variable_names.cr +++ b/src/ameba/rule/style/variable_names.cr @@ -22,7 +22,6 @@ module Ameba::Rule::Style # Style/VariableNames: # Enabled: true # ``` - # struct VariableNames < Base properties do description "Enforces variable names to be in underscored case" diff --git a/src/ameba/rule/style/while_true.cr b/src/ameba/rule/style/while_true.cr index f57289e0..06e9f148 100644 --- a/src/ameba/rule/style/while_true.cr +++ b/src/ameba/rule/style/while_true.cr @@ -25,7 +25,6 @@ module Ameba::Rule::Style # Style/WhileTrue: # Enabled: true # ``` - # struct WhileTrue < Base properties do description "Disallows while statements with a true literal as condition" @@ -34,8 +33,7 @@ module Ameba::Rule::Style MSG = "While statement using true literal as condition" def test(source, node : Crystal::While) - return unless node.cond.true_literal? - issue_for node, MSG + issue_for node, MSG if node.cond.true_literal? end end end