mirror of
https://gitea.invidious.io/iv-org/shard-ameba.git
synced 2024-08-15 00:53:29 +00:00
Readability refactors
This commit is contained in:
parent
784e3ac616
commit
07ce595ef2
19 changed files with 33 additions and 30 deletions
|
@ -134,7 +134,7 @@ module Ameba::AST
|
||||||
node.is_a?(Crystal::CStructOrUnionDef)
|
node.is_a?(Crystal::CStructOrUnionDef)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if current scope (or any of inner scopes) references variable,
|
# Returns `true` if current scope (or any of inner scopes) references variable,
|
||||||
# `false` otherwise.
|
# `false` otherwise.
|
||||||
def references?(variable : Variable, check_inner_scopes = true)
|
def references?(variable : Variable, check_inner_scopes = true)
|
||||||
variable.references.any? do |reference|
|
variable.references.any? do |reference|
|
||||||
|
|
|
@ -93,8 +93,8 @@ module Ameba::AST::Util
|
||||||
end
|
end
|
||||||
|
|
||||||
return if last_line.size < end_column + 1
|
return if last_line.size < end_column + 1
|
||||||
node_lines[-1] = last_line.sub(end_column + 1...last_line.size, "")
|
|
||||||
|
|
||||||
|
node_lines[-1] = last_line.sub(end_column + 1...last_line.size, "")
|
||||||
node_lines.join('\n')
|
node_lines.join('\n')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -17,7 +17,6 @@ module Ameba::Rule::Lint
|
||||||
#
|
#
|
||||||
# And it should be written as this:
|
# And it should be written as this:
|
||||||
#
|
#
|
||||||
#
|
|
||||||
# ```
|
# ```
|
||||||
# def some_method
|
# def some_method
|
||||||
# do_some_stuff
|
# do_some_stuff
|
||||||
|
|
|
@ -47,9 +47,7 @@ module Ameba::Rule::Lint
|
||||||
MSG = "Empty loop detected"
|
MSG = "Empty loop detected"
|
||||||
|
|
||||||
def test(source, node : Crystal::Call)
|
def test(source, node : Crystal::Call)
|
||||||
return unless loop?(node)
|
check_node(source, node, node.block) if loop?(node)
|
||||||
|
|
||||||
check_node(source, node, node.block)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def test(source, node : Crystal::While | Crystal::Until)
|
def test(source, node : Crystal::While | Crystal::Until)
|
||||||
|
@ -58,7 +56,9 @@ module Ameba::Rule::Lint
|
||||||
|
|
||||||
private def check_node(source, node, loop_body)
|
private def check_node(source, node, loop_body)
|
||||||
body = loop_body.is_a?(Crystal::Block) ? loop_body.body : loop_body
|
body = loop_body.is_a?(Crystal::Block) ? loop_body.body : loop_body
|
||||||
issue_for node, MSG if body.nil? || body.nop?
|
return unless body.nil? || body.nop?
|
||||||
|
|
||||||
|
issue_for node, MSG
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -7,6 +7,7 @@ module Ameba::Rule::Lint
|
||||||
# replaced with either the body of the construct, or deleted entirely.
|
# replaced with either the body of the construct, or deleted entirely.
|
||||||
#
|
#
|
||||||
# This is considered invalid:
|
# This is considered invalid:
|
||||||
|
#
|
||||||
# ```
|
# ```
|
||||||
# if "something"
|
# if "something"
|
||||||
# :ok
|
# :ok
|
||||||
|
@ -29,12 +30,8 @@ module Ameba::Rule::Lint
|
||||||
|
|
||||||
MSG = "Literal value found in conditional"
|
MSG = "Literal value found in conditional"
|
||||||
|
|
||||||
def check_node(source, node)
|
|
||||||
issue_for node, MSG if static_literal?(node.cond)
|
|
||||||
end
|
|
||||||
|
|
||||||
def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case)
|
def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case)
|
||||||
check_node source, node
|
issue_for node, MSG if static_literal?(node.cond)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -4,7 +4,6 @@ module Ameba::Rule::Lint
|
||||||
# They usually have the same result - except for non-primitive
|
# They usually have the same result - except for non-primitive
|
||||||
# types like containers, range or regex.
|
# types like containers, range or regex.
|
||||||
#
|
#
|
||||||
#
|
|
||||||
# For example, this will be always false:
|
# For example, this will be always false:
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
|
|
|
@ -32,10 +32,8 @@ module Ameba::Rule::Lint
|
||||||
def test(source, node : Crystal::Call)
|
def test(source, node : Crystal::Call)
|
||||||
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).is_a?(Crystal::NumberLiteral) &&
|
||||||
arg.is_a?(Crystal::NumberLiteral) &&
|
arg.value.in?("0", "1")
|
||||||
(value = arg.value) &&
|
|
||||||
value.in?("0", "1")
|
|
||||||
|
|
||||||
issue_for node, MSG % node
|
issue_for node, MSG % node
|
||||||
end
|
end
|
||||||
|
|
|
@ -17,7 +17,7 @@ module Ameba::Rule::Lint
|
||||||
# YAML configuration example:
|
# YAML configuration example:
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
# Lint/RedundantStringCoersion
|
# Lint/RedundantStringCoercion
|
||||||
# Enabled: true
|
# Enabled: true
|
||||||
# ```
|
# ```
|
||||||
class RedundantStringCoercion < Base
|
class RedundantStringCoercion < Base
|
||||||
|
|
|
@ -42,7 +42,6 @@ module Ameba::Rule::Lint
|
||||||
|
|
||||||
def test(source, node : Crystal::ExceptionHandler)
|
def test(source, node : Crystal::ExceptionHandler)
|
||||||
rescues = node.rescues
|
rescues = node.rescues
|
||||||
|
|
||||||
return if rescues.nil?
|
return if rescues.nil?
|
||||||
|
|
||||||
shadowed(rescues).each do |path|
|
shadowed(rescues).each do |path|
|
||||||
|
|
|
@ -63,8 +63,9 @@ module Ameba::Rule::Lint
|
||||||
return unless node.block
|
return unless node.block
|
||||||
|
|
||||||
arg = node.named_args.try &.find(&.name.== "focus")
|
arg = node.named_args.try &.find(&.name.== "focus")
|
||||||
|
return unless arg
|
||||||
|
|
||||||
issue_for arg, MSG if arg
|
issue_for arg, MSG
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -7,6 +7,7 @@ module Ameba::Rule::Lint
|
||||||
# a + b
|
# a + b
|
||||||
# end
|
# end
|
||||||
# ```
|
# ```
|
||||||
|
#
|
||||||
# and should be written as:
|
# and should be written as:
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
|
|
|
@ -41,6 +41,7 @@ module Ameba::Rule::Performance
|
||||||
return unless node.name == ANY_NAME
|
return unless node.name == ANY_NAME
|
||||||
return unless node.block.nil? && node.args.empty?
|
return unless node.block.nil? && node.args.empty?
|
||||||
return unless node.obj
|
return unless node.obj
|
||||||
|
|
||||||
return unless location = node.location
|
return unless location = node.location
|
||||||
return unless name_location = node.name_location
|
return unless name_location = node.name_location
|
||||||
return unless end_location = name_end_location(node)
|
return unless end_location = name_end_location(node)
|
||||||
|
|
|
@ -68,12 +68,13 @@ module Ameba::Rule::Performance
|
||||||
end
|
end
|
||||||
|
|
||||||
def test(source, node : Crystal::Call)
|
def test(source, node : Crystal::Call)
|
||||||
return unless location = node.name_location
|
|
||||||
return unless end_location = name_end_location(node)
|
|
||||||
return unless (obj = node.obj).is_a?(Crystal::Call)
|
return unless (obj = node.obj).is_a?(Crystal::Call)
|
||||||
return unless node.name.in?(call_names)
|
return unless node.name.in?(call_names)
|
||||||
return unless obj.name.in?(call_names) || obj.name.in?(ALLOCATING_METHOD_NAMES)
|
return unless obj.name.in?(call_names) || obj.name.in?(ALLOCATING_METHOD_NAMES)
|
||||||
|
|
||||||
|
return unless location = node.name_location
|
||||||
|
return unless end_location = name_end_location(node)
|
||||||
|
|
||||||
issue_for location, end_location, MSG % {node.name, obj.name} do |corrector|
|
issue_for location, end_location, MSG % {node.name, obj.name} do |corrector|
|
||||||
corrector.insert_after(end_location, '!')
|
corrector.insert_after(end_location, '!')
|
||||||
end
|
end
|
||||||
|
|
|
@ -51,7 +51,9 @@ module Ameba::Rule::Performance
|
||||||
return unless obj.name.in?(filter_names)
|
return unless obj.name.in?(filter_names)
|
||||||
|
|
||||||
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
|
||||||
|
|
|
@ -65,7 +65,7 @@ module Ameba::Rule::Style
|
||||||
|
|
||||||
private def underscored(sign, value, fraction, suffix)
|
private def underscored(sign, value, fraction, suffix)
|
||||||
value = slice_digits(value.reverse).reverse
|
value = slice_digits(value.reverse).reverse
|
||||||
fraction = "." + slice_digits(fraction) if fraction
|
fraction = ".#{slice_digits(fraction)}" if fraction
|
||||||
|
|
||||||
"#{sign}#{value}#{fraction}#{suffix}"
|
"#{sign}#{value}#{fraction}#{suffix}"
|
||||||
end
|
end
|
||||||
|
|
|
@ -48,6 +48,7 @@ module Ameba::Rule::Style
|
||||||
|
|
||||||
def test(source, node : Crystal::Def)
|
def test(source, node : Crystal::Def)
|
||||||
return if (expected = node.name.underscore) == node.name
|
return if (expected = node.name.underscore) == node.name
|
||||||
|
|
||||||
return unless location = name_location(node)
|
return unless location = name_location(node)
|
||||||
return unless end_location = name_end_location(node)
|
return unless end_location = name_end_location(node)
|
||||||
|
|
||||||
|
|
|
@ -188,9 +188,6 @@ module Ameba::Rule::Style
|
||||||
|
|
||||||
# ameba:disable Metrics/CyclomaticComplexity
|
# ameba:disable Metrics/CyclomaticComplexity
|
||||||
protected def issue_for_valid(source, call : Crystal::Call, block : Crystal::Block, body : Crystal::Call)
|
protected def issue_for_valid(source, call : Crystal::Call, block : Crystal::Block, body : Crystal::Call)
|
||||||
return unless location = call.name_location
|
|
||||||
return unless end_location = block.end_location
|
|
||||||
|
|
||||||
return if exclude_calls_with_block? && body.block
|
return if exclude_calls_with_block? && body.block
|
||||||
return if exclude_multiple_line_blocks? && !same_location_lines?(call, body)
|
return if exclude_multiple_line_blocks? && !same_location_lines?(call, body)
|
||||||
return if exclude_prefix_operators? && prefix_operator?(body)
|
return if exclude_prefix_operators? && prefix_operator?(body)
|
||||||
|
@ -203,6 +200,9 @@ module Ameba::Rule::Style
|
||||||
return unless valid_line_length?(call, call_code)
|
return unless valid_line_length?(call, call_code)
|
||||||
return unless valid_length?(call_code)
|
return unless valid_length?(call_code)
|
||||||
|
|
||||||
|
return unless location = call.name_location
|
||||||
|
return unless end_location = block.end_location
|
||||||
|
|
||||||
if call_code.includes?("{...}")
|
if call_code.includes?("{...}")
|
||||||
issue_for location, end_location, MSG % call_code
|
issue_for location, end_location, MSG % call_code
|
||||||
else
|
else
|
||||||
|
|
|
@ -34,6 +34,7 @@ module Ameba::Rule::Style
|
||||||
|
|
||||||
def test(source, node : Crystal::While)
|
def test(source, node : Crystal::While)
|
||||||
return unless node.cond.true_literal?
|
return unless node.cond.true_literal?
|
||||||
|
|
||||||
return unless location = node.location
|
return unless location = node.location
|
||||||
return unless end_location = node.cond.end_location
|
return unless end_location = node.cond.end_location
|
||||||
|
|
||||||
|
|
|
@ -160,8 +160,10 @@ module Ameba::Spec::ExpectIssue
|
||||||
file = __FILE__,
|
file = __FILE__,
|
||||||
line = __LINE__)
|
line = __LINE__)
|
||||||
lines = code.split('\n') # must preserve trailing newline
|
lines = code.split('\n') # must preserve trailing newline
|
||||||
|
|
||||||
_, actual_annotations = actual_annotations(rules, code, path, lines)
|
_, actual_annotations = actual_annotations(rules, code, path, lines)
|
||||||
return if actual_annotations.to_s == code
|
return if actual_annotations.to_s == code
|
||||||
|
|
||||||
fail <<-MSG, file, line
|
fail <<-MSG, file, line
|
||||||
Expected no issues, but got:
|
Expected no issues, but got:
|
||||||
|
|
||||||
|
@ -182,9 +184,10 @@ module Ameba::Spec::ExpectIssue
|
||||||
private def format_issue(code, **replacements)
|
private def format_issue(code, **replacements)
|
||||||
replacements.each do |keyword, value|
|
replacements.each do |keyword, value|
|
||||||
value = value.to_s
|
value = value.to_s
|
||||||
code = code.gsub("%{#{keyword}}", value)
|
code = code
|
||||||
code = code.gsub("^{#{keyword}}", "^" * value.size)
|
.gsub("%{#{keyword}}", value)
|
||||||
code = code.gsub("_{#{keyword}}", " " * value.size)
|
.gsub("^{#{keyword}}", "^" * value.size)
|
||||||
|
.gsub("_{#{keyword}}", " " * value.size)
|
||||||
end
|
end
|
||||||
code
|
code
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue