From 38b6751bc09de7308289bc92f440f787fce2db00 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 12 Jun 2023 21:35:09 +0200 Subject: [PATCH 1/3] Add `AST::NodeVisitor::Category` simplifying code a bit --- src/ameba/ast/visitors/node_visitor.cr | 29 ++++++++++++++++--- src/ameba/rule/lint/not_nil.cr | 7 +---- src/ameba/rule/lint/not_nil_after_no_bang.cr | 7 +---- .../performance/chained_call_with_no_bang.cr | 7 +---- .../rule/performance/compact_after_map.cr | 7 +---- .../performance/first_last_after_filter.cr | 7 +---- .../rule/performance/flatten_after_map.cr | 7 +---- .../rule/performance/map_instead_of_block.cr | 7 +---- .../rule/performance/size_after_filter.cr | 7 +---- src/ameba/rule/style/is_a_filter.cr | 7 +---- 10 files changed, 34 insertions(+), 58 deletions(-) diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr index 315d1b7b..d41e4908 100644 --- a/src/ameba/ast/visitors/node_visitor.cr +++ b/src/ameba/ast/visitors/node_visitor.cr @@ -8,13 +8,16 @@ module Ameba::AST # visitor = Ameba::AST::NodeVisitor.new(rule, source) # ``` class NodeVisitor < BaseVisitor + enum Category + Macro + end + # List of nodes to be visited by Ameba's rules. NODES = { Alias, - IsA, Assign, - Call, Block, + Call, Case, ClassDef, ClassVar, @@ -25,20 +28,38 @@ module Ameba::AST HashLiteral, If, InstanceVar, + IsA, LibDef, ModuleDef, NilLiteral, StringInterpolation, Unless, + Until, Var, When, While, - Until, } @skip : Array(Crystal::ASTNode.class)? - def initialize(@rule, @source, skip = nil) + def self.category_to_node_classes(category : Category) + case category + in .macro? + [ + Crystal::Macro, + Crystal::MacroExpression, + Crystal::MacroIf, + Crystal::MacroFor, + ] + end + end + + def initialize(@rule, @source, skip : Category) + initialize @rule, @source, + skip: NodeVisitor.category_to_node_classes(skip) + end + + def initialize(@rule, @source, skip : Array? = nil) @skip = skip.try &.map(&.as(Crystal::ASTNode.class)) super @rule, @source end diff --git a/src/ameba/rule/lint/not_nil.cr b/src/ameba/rule/lint/not_nil.cr index 46f41c58..be4268dc 100644 --- a/src/ameba/rule/lint/not_nil.cr +++ b/src/ameba/rule/lint/not_nil.cr @@ -36,12 +36,7 @@ module Ameba::Rule::Lint MSG = "Avoid using `not_nil!`" def test(source) - AST::NodeVisitor.new self, source, skip: [ - Crystal::Macro, - Crystal::MacroExpression, - Crystal::MacroIf, - Crystal::MacroFor, - ] + AST::NodeVisitor.new self, source, skip: :macro end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/lint/not_nil_after_no_bang.cr b/src/ameba/rule/lint/not_nil_after_no_bang.cr index bc697647..dbde7ab8 100644 --- a/src/ameba/rule/lint/not_nil_after_no_bang.cr +++ b/src/ameba/rule/lint/not_nil_after_no_bang.cr @@ -34,12 +34,7 @@ module Ameba::Rule::Lint MSG = "Use `%s! {...}` instead of `%s {...}.not_nil!`" def test(source) - AST::NodeVisitor.new self, source, skip: [ - Crystal::Macro, - Crystal::MacroExpression, - Crystal::MacroIf, - Crystal::MacroFor, - ] + AST::NodeVisitor.new self, source, skip: :macro end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/performance/chained_call_with_no_bang.cr b/src/ameba/rule/performance/chained_call_with_no_bang.cr index d20cc46c..d9285726 100644 --- a/src/ameba/rule/performance/chained_call_with_no_bang.cr +++ b/src/ameba/rule/performance/chained_call_with_no_bang.cr @@ -59,12 +59,7 @@ module Ameba::Rule::Performance MSG = "Use bang method variant `%s!` after chained `%s` call" def test(source) - AST::NodeVisitor.new self, source, skip: [ - Crystal::Macro, - Crystal::MacroExpression, - Crystal::MacroIf, - Crystal::MacroFor, - ] + AST::NodeVisitor.new self, source, skip: :macro end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/performance/compact_after_map.cr b/src/ameba/rule/performance/compact_after_map.cr index 2414337a..6d3e0099 100644 --- a/src/ameba/rule/performance/compact_after_map.cr +++ b/src/ameba/rule/performance/compact_after_map.cr @@ -31,12 +31,7 @@ module Ameba::Rule::Performance MSG = "Use `compact_map {...}` instead of `map {...}.compact`" def test(source) - AST::NodeVisitor.new self, source, skip: [ - Crystal::Macro, - Crystal::MacroExpression, - Crystal::MacroIf, - Crystal::MacroFor, - ] + AST::NodeVisitor.new self, source, skip: :macro end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/performance/first_last_after_filter.cr b/src/ameba/rule/performance/first_last_after_filter.cr index 6c437a42..a995fcae 100644 --- a/src/ameba/rule/performance/first_last_after_filter.cr +++ b/src/ameba/rule/performance/first_last_after_filter.cr @@ -36,12 +36,7 @@ module Ameba::Rule::Performance MSG_REVERSE = "Use `reverse_each.find {...}` instead of `%s {...}.%s`" def test(source) - AST::NodeVisitor.new self, source, skip: [ - Crystal::Macro, - Crystal::MacroExpression, - Crystal::MacroIf, - Crystal::MacroFor, - ] + AST::NodeVisitor.new self, source, skip: :macro end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/performance/flatten_after_map.cr b/src/ameba/rule/performance/flatten_after_map.cr index 81441726..3051681c 100644 --- a/src/ameba/rule/performance/flatten_after_map.cr +++ b/src/ameba/rule/performance/flatten_after_map.cr @@ -31,12 +31,7 @@ module Ameba::Rule::Performance MSG = "Use `flat_map {...}` instead of `map {...}.flatten`" def test(source) - AST::NodeVisitor.new self, source, skip: [ - Crystal::Macro, - Crystal::MacroExpression, - Crystal::MacroIf, - Crystal::MacroFor, - ] + AST::NodeVisitor.new self, source, skip: :macro end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/performance/map_instead_of_block.cr b/src/ameba/rule/performance/map_instead_of_block.cr index d03c101f..74cbce8f 100644 --- a/src/ameba/rule/performance/map_instead_of_block.cr +++ b/src/ameba/rule/performance/map_instead_of_block.cr @@ -32,12 +32,7 @@ module Ameba::Rule::Performance MSG = "Use `%s {...}` instead of `map {...}.%s`" def test(source) - AST::NodeVisitor.new self, source, skip: [ - Crystal::Macro, - Crystal::MacroExpression, - Crystal::MacroIf, - Crystal::MacroFor, - ] + AST::NodeVisitor.new self, source, skip: :macro end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/performance/size_after_filter.cr b/src/ameba/rule/performance/size_after_filter.cr index b1a3e044..ba952586 100644 --- a/src/ameba/rule/performance/size_after_filter.cr +++ b/src/ameba/rule/performance/size_after_filter.cr @@ -42,12 +42,7 @@ module Ameba::Rule::Performance MSG = "Use `count {...}` instead of `%s {...}.size`." def test(source) - AST::NodeVisitor.new self, source, skip: [ - Crystal::Macro, - Crystal::MacroExpression, - Crystal::MacroIf, - Crystal::MacroFor, - ] + AST::NodeVisitor.new self, source, skip: :macro end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/style/is_a_filter.cr b/src/ameba/rule/style/is_a_filter.cr index c2a27df8..8b07f3a9 100644 --- a/src/ameba/rule/style/is_a_filter.cr +++ b/src/ameba/rule/style/is_a_filter.cr @@ -49,12 +49,7 @@ module Ameba::Rule::Style OLD = "%s {...}" def test(source) - AST::NodeVisitor.new self, source, skip: [ - Crystal::Macro, - Crystal::MacroExpression, - Crystal::MacroIf, - Crystal::MacroFor, - ] + AST::NodeVisitor.new self, source, skip: :macro end def test(source, node : Crystal::Call) From c09b36799aa8f0ad1835d1c15a6bdcb7f6280c41 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 12 Jun 2023 21:53:51 +0200 Subject: [PATCH 2/3] Make `AST::NodeVisitor::Category` a flag enum --- src/ameba/ast/visitors/node_visitor.cr | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr index d41e4908..3a263338 100644 --- a/src/ameba/ast/visitors/node_visitor.cr +++ b/src/ameba/ast/visitors/node_visitor.cr @@ -8,6 +8,7 @@ module Ameba::AST # visitor = Ameba::AST::NodeVisitor.new(rule, source) # ``` class NodeVisitor < BaseVisitor + @[Flags] enum Category Macro end @@ -43,14 +44,13 @@ module Ameba::AST @skip : Array(Crystal::ASTNode.class)? def self.category_to_node_classes(category : Category) - case category - in .macro? - [ + ([] of Crystal::ASTNode.class).tap do |classes| + classes.push( Crystal::Macro, Crystal::MacroExpression, Crystal::MacroIf, Crystal::MacroFor, - ] + ) if category.macro? end end From 1931a5f4eff232cab3315d48815465a5a89c5869 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 12 Jun 2023 21:56:22 +0200 Subject: [PATCH 3/3] Make `skip` a named argument --- src/ameba/ast/visitors/node_visitor.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr index 3a263338..a950e68b 100644 --- a/src/ameba/ast/visitors/node_visitor.cr +++ b/src/ameba/ast/visitors/node_visitor.cr @@ -54,12 +54,12 @@ module Ameba::AST end end - def initialize(@rule, @source, skip : Category) + def initialize(@rule, @source, *, skip : Category) initialize @rule, @source, skip: NodeVisitor.category_to_node_classes(skip) end - def initialize(@rule, @source, skip : Array? = nil) + def initialize(@rule, @source, *, skip : Array? = nil) @skip = skip.try &.map(&.as(Crystal::ASTNode.class)) super @rule, @source end