mirror of
https://gitea.invidious.io/iv-org/shard-ameba.git
synced 2024-08-15 00:53:29 +00:00
Merge pull request #197 from crystal-ameba/feature/any-instead-of-empty
Add Performance/AnyInsteadOfEmpty rule
This commit is contained in:
commit
fac8072ec1
9 changed files with 98 additions and 8 deletions
|
@ -51,7 +51,7 @@ module Ameba::AST
|
||||||
it "adds a new variable to the scope" do
|
it "adds a new variable to the scope" do
|
||||||
scope = Scope.new as_node("")
|
scope = Scope.new as_node("")
|
||||||
scope.add_variable(Crystal::Var.new "foo")
|
scope.add_variable(Crystal::Var.new "foo")
|
||||||
scope.variables.any?.should be_true
|
scope.variables.empty?.should be_false
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -48,7 +48,7 @@ module Ameba::AST
|
||||||
it "assigns the variable (creates a new assignment)" do
|
it "assigns the variable (creates a new assignment)" do
|
||||||
variable = Variable.new(var_node, scope)
|
variable = Variable.new(var_node, scope)
|
||||||
variable.assign(assign_node, scope)
|
variable.assign(assign_node, scope)
|
||||||
variable.assignments.any?.should be_true
|
variable.assignments.empty?.should be_false
|
||||||
end
|
end
|
||||||
|
|
||||||
it "can create multiple assignments" do
|
it "can create multiple assignments" do
|
||||||
|
@ -64,7 +64,7 @@ module Ameba::AST
|
||||||
variable = Variable.new(var_node, scope)
|
variable = Variable.new(var_node, scope)
|
||||||
variable.assign(as_node("foo=1"), scope)
|
variable.assign(as_node("foo=1"), scope)
|
||||||
variable.reference(var_node, scope)
|
variable.reference(var_node, scope)
|
||||||
variable.references.any?.should be_true
|
variable.references.empty?.should be_false
|
||||||
end
|
end
|
||||||
|
|
||||||
it "adds a reference to the scope" do
|
it "adds a reference to the scope" do
|
||||||
|
|
46
spec/ameba/rule/performance/any_instead_of_empty_spec.cr
Normal file
46
spec/ameba/rule/performance/any_instead_of_empty_spec.cr
Normal file
|
@ -0,0 +1,46 @@
|
||||||
|
require "../../../spec_helper"
|
||||||
|
|
||||||
|
module Ameba::Rule::Performance
|
||||||
|
subject = AnyInsteadOfEmpty.new
|
||||||
|
|
||||||
|
describe AnyInsteadOfEmpty do
|
||||||
|
it "passes if there is no potential performance improvements" do
|
||||||
|
source = Source.new %(
|
||||||
|
[1, 2, 3].any?(&.zero?)
|
||||||
|
[1, 2, 3].any?(String)
|
||||||
|
[1, 2, 3].any?(1..3)
|
||||||
|
[1, 2, 3].any? { |e| e > 1 }
|
||||||
|
)
|
||||||
|
subject.catch(source).should be_valid
|
||||||
|
end
|
||||||
|
|
||||||
|
it "reports if there is any? call without a block nor argument" do
|
||||||
|
source = Source.new %(
|
||||||
|
[1, 2, 3].any?
|
||||||
|
)
|
||||||
|
subject.catch(source).should_not be_valid
|
||||||
|
end
|
||||||
|
|
||||||
|
context "macro" do
|
||||||
|
it "reports in macro scope" do
|
||||||
|
source = Source.new %(
|
||||||
|
{{ [1, 2, 3].any? }}
|
||||||
|
)
|
||||||
|
subject.catch(source).should_not be_valid
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
it "reports rule, pos and message" do
|
||||||
|
source = Source.new path: "source.cr", code: %(
|
||||||
|
[1, 2, 3].any?
|
||||||
|
)
|
||||||
|
subject.catch(source).should_not be_valid
|
||||||
|
issue = source.issues.first
|
||||||
|
|
||||||
|
issue.rule.should_not be_nil
|
||||||
|
issue.location.to_s.should eq "source.cr:1:11"
|
||||||
|
issue.end_location.to_s.should eq "source.cr:1:15"
|
||||||
|
issue.message.should eq "Use `!{...}.empty?` instead of `{...}.any?`"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -58,7 +58,7 @@ module Ameba::AST
|
||||||
# variable.referenced? # => true
|
# variable.referenced? # => true
|
||||||
# ```
|
# ```
|
||||||
def referenced?
|
def referenced?
|
||||||
references.any?
|
!references.empty?
|
||||||
end
|
end
|
||||||
|
|
||||||
# Creates a reference to this variable in some scope.
|
# Creates a reference to this variable in some scope.
|
||||||
|
|
|
@ -60,7 +60,7 @@ module Ameba::AST
|
||||||
end
|
end
|
||||||
|
|
||||||
private def in_loop?
|
private def in_loop?
|
||||||
@loop_stack.any?
|
!@loop_stack.empty?
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -61,7 +61,7 @@ module Ameba::Cli
|
||||||
if f.size == 1 && f.first =~ /.+:\d+:\d+/
|
if f.size == 1 && f.first =~ /.+:\d+:\d+/
|
||||||
configure_explain_opts(f.first, opts)
|
configure_explain_opts(f.first, opts)
|
||||||
else
|
else
|
||||||
opts.globs = f if f.any?
|
opts.globs = f unless f.empty?
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -36,7 +36,7 @@ module Ameba::Rule::Lint
|
||||||
next unless token.type == :COMMENT
|
next unless token.type == :COMMENT
|
||||||
next unless directive = source.parse_inline_directive(token.value.to_s)
|
next unless directive = source.parse_inline_directive(token.value.to_s)
|
||||||
next unless names = unneeded_disables(source, directive, token.location)
|
next unless names = unneeded_disables(source, directive, token.location)
|
||||||
next unless names.any?
|
next if names.empty?
|
||||||
|
|
||||||
issue_for token, MSG % names.join(", ")
|
issue_for token, MSG % names.join(", ")
|
||||||
end
|
end
|
||||||
|
|
44
src/ameba/rule/performance/any_instead_of_empty.cr
Normal file
44
src/ameba/rule/performance/any_instead_of_empty.cr
Normal file
|
@ -0,0 +1,44 @@
|
||||||
|
module Ameba::Rule::Performance
|
||||||
|
# This rule is used to identify usage of arg-less `Enumerable#any?` calls.
|
||||||
|
#
|
||||||
|
# Using `Enumerable#any?` instead of `Enumerable#empty?` might lead to an
|
||||||
|
# unexpected results (like `[nil, false].any? # => false`). In some cases
|
||||||
|
# it also might be less efficient, since it iterates until the block will
|
||||||
|
# return a _truthy_ value, instead of just checking if there's at least
|
||||||
|
# one value present.
|
||||||
|
#
|
||||||
|
# For example, this is considered invalid:
|
||||||
|
#
|
||||||
|
# ```
|
||||||
|
# [1, 2, 3].any?
|
||||||
|
# ```
|
||||||
|
#
|
||||||
|
# And it should be written as this:
|
||||||
|
#
|
||||||
|
# ```
|
||||||
|
# ![1, 2, 3].empty?
|
||||||
|
# ```
|
||||||
|
#
|
||||||
|
# YAML configuration example:
|
||||||
|
#
|
||||||
|
# ```
|
||||||
|
# Performance/AnyInsteadOfEmpty:
|
||||||
|
# Enabled: true
|
||||||
|
# ```
|
||||||
|
class AnyInsteadOfEmpty < Base
|
||||||
|
properties do
|
||||||
|
description "Identifies usage of arg-less `any?` calls."
|
||||||
|
end
|
||||||
|
|
||||||
|
ANY_NAME = "any?"
|
||||||
|
MSG = "Use `!{...}.empty?` instead of `{...}.any?`"
|
||||||
|
|
||||||
|
def test(source, node : Crystal::Call)
|
||||||
|
return unless node.name == ANY_NAME
|
||||||
|
return unless node.block.nil? && node.args.empty?
|
||||||
|
return unless node.obj
|
||||||
|
|
||||||
|
issue_for node.name_location, node.name_end_location, MSG
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -45,7 +45,7 @@ module Ameba::Rule::Performance
|
||||||
def test(source, node : Crystal::Call)
|
def test(source, node : Crystal::Call)
|
||||||
return unless node.name.in?(CALL_NAMES) && (obj = node.obj)
|
return unless node.name.in?(CALL_NAMES) && (obj = node.obj)
|
||||||
return unless obj.is_a?(Crystal::Call) && obj.block
|
return unless obj.is_a?(Crystal::Call) && obj.block
|
||||||
return if !node.block.nil? || node.args.any?
|
return unless node.block.nil? && node.args.empty?
|
||||||
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
|
||||||
|
|
Loading…
Reference in a new issue