mirror of
https://gitea.invidious.io/iv-org/shard-ameba.git
synced 2024-08-15 00:53:29 +00:00
Hk cyclomatic complexity (#92)
* Proof of concept for cyclomatic complexity * Enable configurability of rule * Use the same nodes to increment the complexity as rubocop * Fix typo in test description * Properly indent code and simplify macro * Move metric into metrics * Cover a violation supressed by increased threshold * Extract visitor into its own file * Document cyclomatic complexity rule and visitor * Refactor specs to use a macro * Indent code inside macro * Replace array with tuple for string formatting. `Tuple` is stack based, whereas `Array` is allocated on the heap increasing GC pressure. * Fix formatting * Enable cyclomatic complexity rule by default
This commit is contained in:
parent
c4016bd28c
commit
e850bff60f
4 changed files with 149 additions and 0 deletions
39
spec/ameba/ast/visitors/counting_visitor_spec.cr
Normal file
39
spec/ameba/ast/visitors/counting_visitor_spec.cr
Normal file
|
@ -0,0 +1,39 @@
|
|||
require "../../../spec_helper"
|
||||
|
||||
module Ameba::AST
|
||||
describe CountingVisitor do
|
||||
describe "#visit" do
|
||||
it "allow to visit ASTNode" do
|
||||
node = Crystal::Parser.new("").parse
|
||||
visitor = CountingVisitor.new node
|
||||
node.accept visitor
|
||||
end
|
||||
end
|
||||
|
||||
describe "#count" do
|
||||
it "is 1 for an empty method" do
|
||||
node = Crystal::Parser.new("def hello; end").parse
|
||||
visitor = CountingVisitor.new node
|
||||
|
||||
visitor.count.should eq 1
|
||||
end
|
||||
|
||||
{% for pair in [
|
||||
{code: "if true; end", description: "conditional"},
|
||||
{code: "while true; end", description: "while loop"},
|
||||
{code: "until 1 < 2; end", description: "until loop"},
|
||||
{code: "begin; rescue; end", description: "rescue"},
|
||||
{code: "case 1 when 1; end", description: "when"},
|
||||
{code: "true || false", description: "or"},
|
||||
{code: "true && false", description: "and"},
|
||||
] %}
|
||||
it "increases count for every {{ pair[:description].id }}" do
|
||||
node = Crystal::Parser.new("def hello; {{ pair[:code].id }} end").parse
|
||||
visitor = CountingVisitor.new node
|
||||
|
||||
visitor.count.should eq 2
|
||||
end
|
||||
{% end %}
|
||||
end
|
||||
end
|
||||
end
|
47
spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr
Normal file
47
spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr
Normal file
|
@ -0,0 +1,47 @@
|
|||
require "../../../spec_helper"
|
||||
|
||||
module Ameba::Rule::Metrics
|
||||
subject = CyclomaticComplexity.new
|
||||
complex_method = <<-CODE
|
||||
def hello(a, b, c)
|
||||
if a && b && c
|
||||
begin
|
||||
while true
|
||||
return if false && b
|
||||
end
|
||||
""
|
||||
rescue
|
||||
""
|
||||
end
|
||||
end
|
||||
end
|
||||
CODE
|
||||
|
||||
describe CyclomaticComplexity do
|
||||
it "passes for empty methods" do
|
||||
source = Source.new %(
|
||||
def hello
|
||||
end
|
||||
)
|
||||
subject.catch(source).should be_valid
|
||||
end
|
||||
|
||||
it "reports one issue for a complex method" do
|
||||
subject.max_complexity = 5
|
||||
source = Source.new(complex_method, "source.cr")
|
||||
subject.catch(source).should_not be_valid
|
||||
|
||||
issue = source.issues.first
|
||||
issue.rule.should eq subject
|
||||
issue.location.to_s.should eq "source.cr:1:1"
|
||||
issue.end_location.to_s.should eq "source.cr:12:3"
|
||||
issue.message.should eq "Cyclomatic complexity too high [8/5]"
|
||||
end
|
||||
|
||||
it "doesn't report an issue for an increased threshold" do
|
||||
subject.max_complexity = 100
|
||||
source = Source.new(complex_method, "source.cr")
|
||||
subject.catch(source).should be_valid
|
||||
end
|
||||
end
|
||||
end
|
31
src/ameba/ast/visitors/counting_visitor.cr
Normal file
31
src/ameba/ast/visitors/counting_visitor.cr
Normal file
|
@ -0,0 +1,31 @@
|
|||
module Ameba::AST
|
||||
# AST Visitor that counts occurences of certain keywords
|
||||
class CountingVisitor < Crystal::Visitor
|
||||
@complexity = 1
|
||||
|
||||
# Creates a new counting visitor
|
||||
def initialize(@scope : Crystal::ASTNode)
|
||||
end
|
||||
|
||||
# :nodoc:
|
||||
def visit(node : Crystal::ASTNode)
|
||||
true
|
||||
end
|
||||
|
||||
# Returns the number of keywords that were found in the node
|
||||
def count
|
||||
@scope.accept(self)
|
||||
@complexity
|
||||
end
|
||||
|
||||
# Uses the same logic than rubocop. See
|
||||
# https://github.com/rubocop-hq/rubocop/blob/master/lib/rubocop/cop/metrics/cyclomatic_complexity.rb#L21
|
||||
# Except "for", because crystal doesn't have a "for" loop.
|
||||
{% for node in %i(if while until rescue when or and) %}
|
||||
# :nodoc:
|
||||
def visit(node : Crystal::{{ node.id.capitalize }})
|
||||
@complexity += 1
|
||||
end
|
||||
{% end %}
|
||||
end
|
||||
end
|
32
src/ameba/rule/metrics/cyclomatic_complexity.cr
Normal file
32
src/ameba/rule/metrics/cyclomatic_complexity.cr
Normal file
|
@ -0,0 +1,32 @@
|
|||
module Ameba::Rule::Metrics
|
||||
# A rule that disallows methods with a cyclomatic complexity higher than `MaxComplexity`
|
||||
#
|
||||
# YAML configuration example:
|
||||
#
|
||||
# ```
|
||||
# Metrics/CyclomaticComplexity:
|
||||
# Enabled: true
|
||||
# MaxComplexity: 10
|
||||
# ```
|
||||
#
|
||||
struct CyclomaticComplexity < Base
|
||||
properties do
|
||||
description "Disallows methods with a cyclomatic complexity higher than `MaxComplexity`"
|
||||
max_complexity 10
|
||||
end
|
||||
|
||||
MSG = "Cyclomatic complexity too high [%d/%d]"
|
||||
|
||||
def test(source)
|
||||
AST::NodeVisitor.new self, source
|
||||
end
|
||||
|
||||
def test(source, node : Crystal::Def)
|
||||
complexity = AST::CountingVisitor.new(node).count
|
||||
|
||||
if complexity > max_complexity
|
||||
issue_for(node, MSG % {complexity, max_complexity})
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in a new issue