Followup to #185

This commit is contained in:
Sijawusz Pur Rahnama 2021-01-17 13:25:50 +01:00
parent 1a091c1f1a
commit 06852c490b
16 changed files with 34 additions and 40 deletions

View file

@ -49,7 +49,7 @@ module Ameba::AST
# a ||= 1 # a ||= 1
# ``` # ```
def op_assign? def op_assign?
node.is_a? Crystal::OpAssign node.is_a?(Crystal::OpAssign)
end end
# Returns true if this assignment is in a branch, false if not. # Returns true if this assignment is in a branch, false if not.

View file

@ -91,8 +91,8 @@ module Ameba::AST
next if consumed_branches.includes?(assignment.branch) next if consumed_branches.includes?(assignment.branch)
assignment.referenced = true assignment.referenced = true
break unless assignment.branch break unless branch = assignment.branch
consumed_branches << assignment.branch.not_nil! consumed_branches << branch
end end
end end

View file

@ -270,7 +270,7 @@ class Ameba::Config
include YAML::Serializable::Strict include YAML::Serializable::Strict
def self.new(config = nil) def self.new(config = nil)
if (raw = config.try &.raw).is_a? Hash if (raw = config.try &.raw).is_a?(Hash)
yaml = raw[rule_name]?.try &.to_yaml yaml = raw[rule_name]?.try &.to_yaml
end end
from_yaml yaml || "{}" from_yaml yaml || "{}"

View file

@ -41,7 +41,7 @@ module Ameba::Formatter
private def rule_issues_map(issues) private def rule_issues_map(issues)
Hash(Rule::Base, Array(Issue)).new.tap do |h| Hash(Rule::Base, Array(Issue)).new.tap do |h|
issues.each do |issue| issues.each do |issue|
next if issue.disabled? || issue.rule.is_a? Rule::Lint::Syntax next if issue.disabled? || issue.rule.is_a?(Rule::Lint::Syntax)
(h[issue.rule] ||= Array(Issue).new) << issue (h[issue.rule] ||= Array(Issue).new) << issue
end end
end end

View file

@ -30,8 +30,7 @@ module Ameba::Rule::Lint
MSG = "Literal value found in conditional" MSG = "Literal value found in conditional"
def check_node(source, node) def check_node(source, node)
return unless literal?(node.cond) issue_for node, MSG if literal?(node.cond)
issue_for node, MSG
end end
def test(source, node : Crystal::If) def test(source, node : Crystal::If)

View file

@ -26,8 +26,8 @@ module Ameba::Rule::Lint
struct PercentArrays < Base struct PercentArrays < Base
properties do properties do
description "Disallows some unwanted symbols in percent array literals" description "Disallows some unwanted symbols in percent array literals"
string_array_unwanted_symbols ",\"" string_array_unwanted_symbols %(,")
symbol_array_unwanted_symbols ",:" symbol_array_unwanted_symbols %(,:)
end end
MSG = "Symbols `%s` may be unwanted in %s array literals" MSG = "Symbols `%s` may be unwanted in %s array literals"
@ -62,8 +62,7 @@ module Ameba::Rule::Lint
end end
private def check_array_entry(entry, symbols, literal) private def check_array_entry(entry, symbols, literal)
return unless entry =~ /[#{symbols}]/ MSG % {symbols, literal} if entry =~ /[#{symbols}]/
MSG % {symbols, literal}
end end
end end
end end

View file

@ -33,7 +33,7 @@ module Ameba::Rule::Lint
return unless node.name == "rand" && return unless node.name == "rand" &&
node.args.size == 1 && node.args.size == 1 &&
(arg = node.args.first) && (arg = node.args.first) &&
(arg.is_a? Crystal::NumberLiteral) && arg.is_a?(Crystal::NumberLiteral) &&
(value = arg.value) && (value = arg.value) &&
value.in?("0", "1") value.in?("0", "1")

View file

@ -30,7 +30,9 @@ module Ameba::Rule::Lint
MSG = "Redundant use of `Object#to_s` in interpolation" MSG = "Redundant use of `Object#to_s` in interpolation"
def test(source, node : Crystal::StringInterpolation) def test(source, node : Crystal::StringInterpolation)
string_coercion_nodes(node).each { |n| issue_for n.name_location, n.end_location, MSG } string_coercion_nodes(node).each do |n|
issue_for n.name_location, n.end_location, MSG
end
end end
private def string_coercion_nodes(node) private def string_coercion_nodes(node)

View file

@ -41,12 +41,11 @@ module Ameba::Rule::Lint
MSG = "Exception handler has shadowed exceptions: %s" MSG = "Exception handler has shadowed exceptions: %s"
def test(source, node : Crystal::ExceptionHandler) def test(source, node : Crystal::ExceptionHandler)
return unless excs = node.rescues return unless excs = node.rescues.try &.map(&.types)
return if (excs = shadowed excs).empty?
if (excs = shadowed excs.map(&.types)).any?
issue_for node, MSG % excs.join(", ") issue_for node, MSG % excs.join(", ")
end end
end
private def shadowed(exceptions, exception_found = false) private def shadowed(exceptions, exception_found = false)
previous_exceptions = [] of String previous_exceptions = [] of String

View file

@ -20,9 +20,8 @@ module Ameba::Rule::Metrics
complexity = AST::CountingVisitor.new(node).count complexity = AST::CountingVisitor.new(node).count
if complexity > max_complexity && (location = node.name_location) if complexity > max_complexity && (location = node.name_location)
issue_for location, def_name_end_location(node), MSG % { issue_for location, def_name_end_location(node),
complexity, max_complexity, MSG % {complexity, max_complexity}
}
end end
end end

View file

@ -35,12 +35,10 @@ module Ameba::Rule::Performance
def test(source, node : Crystal::Call) def test(source, node : Crystal::Call)
return unless node.name == ANY_NAME && (obj = node.obj) return unless node.name == ANY_NAME && (obj = node.obj)
return unless obj.is_a?(Crystal::Call) return unless obj.is_a?(Crystal::Call) && obj.block && node.block.nil?
return if obj.block.nil? || !node.block.nil? return unless filter_names.includes?(obj.name)
if filter_names.includes?(obj.name)
issue_for obj.name_location, node.name_end_location, MSG % obj.name issue_for obj.name_location, node.name_end_location, MSG % obj.name
end end
end end
end end
end

View file

@ -44,13 +44,12 @@ module Ameba::Rule::Performance
def test(source, node : Crystal::Call) def test(source, node : Crystal::Call)
return unless CALL_NAMES.includes?(node.name) && (obj = node.obj) return unless CALL_NAMES.includes?(node.name) && (obj = node.obj)
return unless obj.is_a?(Crystal::Call) return unless obj.is_a?(Crystal::Call) && obj.block
return if obj.block.nil? || !node.block.nil? || node.args.any? return if !node.block.nil? || node.args.any?
return unless filter_names.includes?(obj.name)
if filter_names.includes?(obj.name)
message = node.name.includes?(CALL_NAMES.first) ? MSG : MSG_REVERSE message = node.name.includes?(CALL_NAMES.first) ? MSG : MSG_REVERSE
issue_for obj.name_location, node.name_end_location, message % {obj.name, node.name} issue_for obj.name_location, node.name_end_location, message % {obj.name, node.name}
end end
end end
end end
end

View file

@ -50,12 +50,10 @@ module Ameba::Rule::Performance
def test(source, node : Crystal::Call) def test(source, node : Crystal::Call)
return unless node.name == SIZE_NAME && (obj = node.obj) return unless node.name == SIZE_NAME && (obj = node.obj)
return unless obj.is_a?(Crystal::Call) return unless obj.is_a?(Crystal::Call) && obj.block
return if obj.block.nil? return unless filter_names.includes?(obj.name)
if filter_names.includes?(obj.name)
issue_for obj.name_location, node.name_end_location, MSG % obj.name issue_for obj.name_location, node.name_end_location, MSG % obj.name
end end
end end
end end
end

View file

@ -29,7 +29,7 @@ module Ameba::Rule::Style
MSG = "Constant name should be screaming-cased: %s, not %s" MSG = "Constant name should be screaming-cased: %s, not %s"
def test(source, node : Crystal::Assign) def test(source, node : Crystal::Assign)
if (target = node.target).is_a? Crystal::Path if (target = node.target).is_a?(Crystal::Path)
name = target.names.first name = target.names.first
expected = name.upcase expected = name.upcase

View file

@ -4,7 +4,7 @@ module Ameba::Rule::Style
# This is considered bad: # This is considered bad:
# #
# ``` # ```
# var.is_a? Nil # var.is_a?(Nil)
# ``` # ```
# #
# And needs to be written as: # And needs to be written as:
@ -31,7 +31,8 @@ module Ameba::Rule::Style
return if node.nil_check? return if node.nil_check?
const = node.const const = node.const
return unless const.is_a?(Crystal::Path) && const.names == PATH_NIL_NAMES return unless const.is_a?(Crystal::Path)
return unless const.names == PATH_NIL_NAMES
issue_for const, MSG issue_for const, MSG
end end

View file

@ -42,7 +42,7 @@ module Ameba::Rule::Style
when Crystal::BinaryOp when Crystal::BinaryOp
negated_condition?(node.left) || negated_condition?(node.right) negated_condition?(node.left) || negated_condition?(node.right)
when Crystal::Expressions when Crystal::Expressions
node.expressions.any? { |e| negated_condition? e } node.expressions.any? { |e| negated_condition?(e) }
when Crystal::Not when Crystal::Not
true true
else else