From 6bd18f9cbf6f920d3979d18d6f98e8e4eac6d2d2 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 22 Nov 2022 19:43:52 +0100 Subject: [PATCH 01/13] Fix typo --- spec/ameba/config_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ameba/config_spec.cr b/spec/ameba/config_spec.cr index c5f82631..03bd26a6 100644 --- a/spec/ameba/config_spec.cr +++ b/spec/ameba/config_spec.cr @@ -132,7 +132,7 @@ module Ameba config.sources.size.should eq(1) end - it "returns a lisf of sources excluding 'Excluded'" do + it "returns a list of sources excluding 'Excluded'" do config.excluded = %w(**/config_spec.cr) config.sources.any?(&.fullpath.==(__FILE__)).should be_false end From 39cc286263c9b4586a64948fed9b5ae9809acff8 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 22 Nov 2022 19:44:38 +0100 Subject: [PATCH 02/13] Use tuple instead of array for `Rule::SPECIAL` --- src/ameba/rule/base.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr index e261a10b..598f5991 100644 --- a/src/ameba/rule/base.cr +++ b/src/ameba/rule/base.cr @@ -1,10 +1,10 @@ module Ameba::Rule # List of names of the special rules, which # behave differently than usual rules. - SPECIAL = [ + SPECIAL = { Lint::Syntax.rule_name, Lint::UnneededDisableDirective.rule_name, - ] + } # Represents a base of all rules. In other words, all rules # inherits from this struct: From 31392046e0425f321b572896b5a19df223f6fdb6 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 22 Nov 2022 19:45:16 +0100 Subject: [PATCH 03/13] Always return `Bool` value from `Rule#excluded?` --- src/ameba/rule/base.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr index 598f5991..bb50d296 100644 --- a/src/ameba/rule/base.cr +++ b/src/ameba/rule/base.cr @@ -88,7 +88,7 @@ module Ameba::Rule # my_rule.excluded?(source) # => true or false # ``` def excluded?(source) - excluded.try &.any? do |path| + !!excluded.try &.any? do |path| source.matches_path?(path) || Dir.glob(path).any? { |glob| source.matches_path?(glob) } end From c7c75ee36a8fa9122e1228748a27827903e8e5f8 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 22 Nov 2022 19:46:38 +0100 Subject: [PATCH 04/13] `Rule#enabled` -> `Rule#enabled?` --- spec/ameba/base_spec.cr | 2 +- spec/ameba/config_spec.cr | 6 +++--- src/ameba/config.cr | 2 +- src/ameba/runner.cr | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/ameba/base_spec.cr b/spec/ameba/base_spec.cr index 508091b5..21ced084 100644 --- a/spec/ameba/base_spec.cr +++ b/spec/ameba/base_spec.cr @@ -25,7 +25,7 @@ module Ameba::Rule subject = DummyRule.new it "is enabled by default" do - subject.enabled.should be_true + subject.enabled?.should be_true end it "has a description property" do diff --git a/spec/ameba/config_spec.cr b/spec/ameba/config_spec.cr index 03bd26a6..d738310f 100644 --- a/spec/ameba/config_spec.cr +++ b/spec/ameba/config_spec.cr @@ -170,7 +170,7 @@ module Ameba name = DummyRule.rule_name config.update_rule name, enabled: false rule = config.rules.find!(&.name.== name) - rule.enabled.should be_false + rule.enabled?.should be_false end it "updates excluded property" do @@ -189,7 +189,7 @@ module Ameba name = DummyRule.rule_name config.update_rules [name], enabled: false rule = config.rules.find!(&.name.== name) - rule.enabled.should be_false + rule.enabled?.should be_false end it "updates multiple rules by excluded property" do @@ -204,7 +204,7 @@ module Ameba group = DummyRule.group_name config.update_rules [group], enabled: false rule = config.rules.find!(&.name.== DummyRule.rule_name) - rule.enabled.should be_false + rule.enabled?.should be_false end it "updates a group by excluded property" do diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 8f5b52a4..515e4fd8 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -247,7 +247,7 @@ class Ameba::Config {% if properties["enabled".id] == nil %} @[YAML::Field(key: "Enabled")] - property enabled = true + property? enabled = true {% end %} {% if properties["severity".id] == nil %} diff --git a/src/ameba/runner.cr b/src/ameba/runner.cr index a987db12..2f8ee72b 100644 --- a/src/ameba/runner.cr +++ b/src/ameba/runner.cr @@ -62,7 +62,7 @@ module Ameba @sources = config.sources @formatter = config.formatter @severity = config.severity - @rules = config.rules.select(&.enabled).reject!(&.special?) + @rules = config.rules.select(&.enabled?).reject!(&.special?) @autocorrect = config.autocorrect? @unneeded_disable_directive_rule = @@ -220,7 +220,7 @@ module Ameba private def check_unneeded_directives(source) return unless rule = @unneeded_disable_directive_rule - return unless rule.enabled + return unless rule.enabled? rule.test(source) end From 2af58cabd4777fc6791de2baa508a55aba741379 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 22 Nov 2022 19:49:16 +0100 Subject: [PATCH 05/13] Use `property?` for defining `Bool`-returning `Rule` properties --- src/ameba/config.cr | 6 +++++- src/ameba/rule/lint/unused_argument.cr | 6 +++--- src/ameba/rule/style/parentheses_around_condition.cr | 6 +++--- src/ameba/rule/style/redundant_next.cr | 4 ++-- src/ameba/rule/style/redundant_return.cr | 4 ++-- src/ameba/rule/style/verbose_block.cr | 11 ++++++----- 6 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 515e4fd8..47a4e49b 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -242,7 +242,11 @@ class Ameba::Config {% properties[name] = {key: key, default: value, type: type, converter: converter} %} @[YAML::Field(key: {{ key }}, converter: {{ converter }}, type: {{ type }})] - property {{ name }} : {{ type }} = {{ value }} + {% if type == Bool %} + property? {{ name }} : {{ type }} = {{ value }} + {% else %} + property {{ name }} : {{ type }} = {{ value }} + {% end %} {% end %} {% if properties["enabled".id] == nil %} diff --git a/src/ameba/rule/lint/unused_argument.cr b/src/ameba/rule/lint/unused_argument.cr index 16d86049..891486e8 100644 --- a/src/ameba/rule/lint/unused_argument.cr +++ b/src/ameba/rule/lint/unused_argument.cr @@ -41,15 +41,15 @@ module Ameba::Rule::Lint end def test(source, node : Crystal::ProcLiteral, scope : AST::Scope) - ignore_procs || find_unused_arguments source, scope + ignore_procs? || find_unused_arguments source, scope end def test(source, node : Crystal::Block, scope : AST::Scope) - ignore_blocks || find_unused_arguments source, scope + ignore_blocks? || find_unused_arguments source, scope end def test(source, node : Crystal::Def, scope : AST::Scope) - ignore_defs || find_unused_arguments source, scope + ignore_defs? || find_unused_arguments source, scope end private def find_unused_arguments(source, scope) diff --git a/src/ameba/rule/style/parentheses_around_condition.cr b/src/ameba/rule/style/parentheses_around_condition.cr index 1b347e2a..5ddc15f4 100644 --- a/src/ameba/rule/style/parentheses_around_condition.cr +++ b/src/ameba/rule/style/parentheses_around_condition.cr @@ -46,7 +46,7 @@ module Ameba::Rule::Style when Crystal::Yield !in_ternary || node.has_parentheses? || node.exps.empty? when Crystal::Assign, Crystal::OpAssign, Crystal::MultiAssign - !in_ternary && !allow_safe_assignment + !in_ternary && !allow_safe_assignment? else true end @@ -55,7 +55,7 @@ module Ameba::Rule::Style def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case | Crystal::While | Crystal::Until) cond = node.cond - if cond.is_a?(Crystal::Assign) && allow_safe_assignment + if cond.is_a?(Crystal::Assign) && allow_safe_assignment? issue_for cond, MSG_MISSING do |corrector| corrector.insert_before(cond, '(') corrector.insert_after(cond, ')') @@ -65,7 +65,7 @@ module Ameba::Rule::Style is_ternary = node.is_a?(Crystal::If) && node.ternary? - return if is_ternary && exclude_ternary + return if is_ternary && exclude_ternary? return unless cond.is_a?(Crystal::Expressions) return unless cond.keyword.paren? diff --git a/src/ameba/rule/style/redundant_next.cr b/src/ameba/rule/style/redundant_next.cr index b0e8b1eb..ce81d080 100644 --- a/src/ameba/rule/style/redundant_next.cr +++ b/src/ameba/rule/style/redundant_next.cr @@ -113,8 +113,8 @@ module Ameba::Rule::Style end def test(source, node : Crystal::Next, visitor : AST::RedundantControlExpressionVisitor) - return if allow_multi_next && node.exp.is_a?(Crystal::TupleLiteral) - return if allow_empty_next && (node.exp.nil? || node.exp.try(&.nop?)) + return if allow_multi_next? && node.exp.is_a?(Crystal::TupleLiteral) + return if allow_empty_next? && (node.exp.nil? || node.exp.try(&.nop?)) if exp_code = control_exp_code(node, source.lines) issue_for node, MSG do |corrector| diff --git a/src/ameba/rule/style/redundant_return.cr b/src/ameba/rule/style/redundant_return.cr index 7a020073..234fd097 100644 --- a/src/ameba/rule/style/redundant_return.cr +++ b/src/ameba/rule/style/redundant_return.cr @@ -110,8 +110,8 @@ module Ameba::Rule::Style end def test(source, node : Crystal::Return, visitor : AST::RedundantControlExpressionVisitor) - return if allow_multi_return && node.exp.is_a?(Crystal::TupleLiteral) - return if allow_empty_return && (node.exp.nil? || node.exp.try(&.nop?)) + return if allow_multi_return? && node.exp.is_a?(Crystal::TupleLiteral) + return if allow_empty_return? && (node.exp.nil? || node.exp.try(&.nop?)) if exp_code = control_exp_code(node, source.lines) issue_for node, MSG do |corrector| diff --git a/src/ameba/rule/style/verbose_block.cr b/src/ameba/rule/style/verbose_block.cr index a97661cb..5e70ee5c 100644 --- a/src/ameba/rule/style/verbose_block.cr +++ b/src/ameba/rule/style/verbose_block.cr @@ -190,11 +190,12 @@ module Ameba::Rule::Style 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_multiple_line_blocks && !same_location_lines?(call, body) - return if exclude_prefix_operators && prefix_operator?(body) - return if exclude_operators && operator?(body.name) - return if exclude_setters && setter?(body.name) + + return if exclude_calls_with_block? && body.block + return if exclude_multiple_line_blocks? && !same_location_lines?(call, body) + return if exclude_prefix_operators? && prefix_operator?(body) + return if exclude_operators? && operator?(body.name) + return if exclude_setters? && setter?(body.name) call_code = call_code(source, call, body) From 9f670c09b55159358e9f9537b848c35c0fe95b56 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 22 Nov 2022 19:51:48 +0100 Subject: [PATCH 06/13] Refactor `TODOFormatter` spec --- spec/ameba/formatter/todo_formatter_spec.cr | 116 +++++++++++--------- spec/spec_helper.cr | 4 +- 2 files changed, 66 insertions(+), 54 deletions(-) diff --git a/spec/ameba/formatter/todo_formatter_spec.cr b/spec/ameba/formatter/todo_formatter_spec.cr index aab58414..bd7497ef 100644 --- a/spec/ameba/formatter/todo_formatter_spec.cr +++ b/spec/ameba/formatter/todo_formatter_spec.cr @@ -2,42 +2,51 @@ require "../../spec_helper" require "file_utils" module Ameba + private def with_formatter + io = IO::Memory.new + formatter = Formatter::TODOFormatter.new(io) + + yield formatter, io + end + private def create_todo - formatter = Formatter::TODOFormatter.new IO::Memory.new - s = Source.new "a = 1", "source.cr" - s.add_issue DummyRule.new, {1, 2}, "message" - file = formatter.finished([s]) - file ? File.read(file.path) : "" + with_formatter do |formatter| + s = Source.new "a = 1", "source.cr" + s.add_issue DummyRule.new, {1, 2}, "message" + file = formatter.finished([s]) + file ? File.read(file.path) : "" + end end describe Formatter::TODOFormatter do ::Spec.after_each do - FileUtils.rm(Ameba::Config::PATH) if File.exists?(Ameba::Config::PATH) + FileUtils.rm_rf(Ameba::Config::PATH) end context "problems not found" do it "does not create file" do - formatter = Formatter::TODOFormatter.new IO::Memory.new - file = formatter.finished [Source.new ""] - file.should be_nil + with_formatter do |formatter| + file = formatter.finished [Source.new ""] + file.should be_nil + end end it "reports a message saying file is not created" do - io = IO::Memory.new - formatter = Formatter::TODOFormatter.new io - formatter.finished [Source.new ""] - io.to_s.should contain "No issues found. File is not generated" + with_formatter do |formatter, io| + formatter.finished [Source.new ""] + io.to_s.should contain "No issues found. File is not generated" + end end end context "problems found" do it "prints a message saying file is created" do - io = IO::Memory.new - formatter = Formatter::TODOFormatter.new io - s = Source.new "a = 1", "source.cr" - s.add_issue DummyRule.new, {1, 2}, "message" - formatter.finished([s]) - io.to_s.should contain "Created .ameba.yml" + with_formatter do |formatter, io| + s = Source.new "a = 1", "source.cr" + s.add_issue DummyRule.new, {1, 2}, "message" + formatter.finished([s]) + io.to_s.should contain "Created .ameba.yml" + end end it "creates a valid YAML document" do @@ -77,49 +86,52 @@ module Ameba end context "with multiple issues" do - formatter = Formatter::TODOFormatter.new IO::Memory.new + it "does generate todo file" do + with_formatter do |formatter| + s1 = Source.new "a = 1", "source1.cr" + s2 = Source.new "a = 1", "source2.cr" + s1.add_issue DummyRule.new, {1, 2}, "message1" + s1.add_issue NamedRule.new, {1, 2}, "message1" + s1.add_issue DummyRule.new, {2, 2}, "message1" + s2.add_issue DummyRule.new, {2, 2}, "message2" - s1 = Source.new "a = 1", "source1.cr" - s2 = Source.new "a = 1", "source2.cr" - s1.add_issue DummyRule.new, {1, 2}, "message1" - s1.add_issue NamedRule.new, {1, 2}, "message1" - s1.add_issue DummyRule.new, {2, 2}, "message1" - s2.add_issue DummyRule.new, {2, 2}, "message2" - - file = formatter.finished([s1, s2]).should_not be_nil - content = File.read(file.path) - content.should contain <<-CONTENT - # Problems found: 3 - # Run `ameba --only Ameba/DummyRule` for details - Ameba/DummyRule: - Description: Dummy rule that does nothing. - Excluded: - - source1.cr - - source2.cr - Enabled: true - Severity: Convention - CONTENT + file = formatter.finished([s1, s2]).should_not be_nil + content = File.read(file.path) + content.should contain <<-CONTENT + # Problems found: 3 + # Run `ameba --only Ameba/DummyRule` for details + Ameba/DummyRule: + Description: Dummy rule that does nothing. + Excluded: + - source1.cr + - source2.cr + Enabled: true + Severity: Convention + CONTENT + end + end end context "when invalid syntax" do it "does generate todo file" do - formatter = Formatter::TODOFormatter.new IO::Memory.new - s = Source.new "def invalid_syntax" - s.add_issue Rule::Lint::Syntax.new, {1, 2}, "message" + with_formatter do |formatter| + s = Source.new "def invalid_syntax" + s.add_issue Rule::Lint::Syntax.new, {1, 2}, "message" - file = formatter.finished [s] - file.should be_nil + file = formatter.finished [s] + file.should be_nil + end end it "prints an error message" do - io = IO::Memory.new - formatter = Formatter::TODOFormatter.new io - s = Source.new "def invalid_syntax" - s.add_issue Rule::Lint::Syntax.new, {1, 2}, "message" + with_formatter do |formatter, io| + s = Source.new "def invalid_syntax" + s.add_issue Rule::Lint::Syntax.new, {1, 2}, "message" - formatter.finished [s] - io.to_s.should contain "Unable to generate TODO file" - io.to_s.should contain "Please fix syntax issues" + formatter.finished [s] + io.to_s.should contain "Unable to generate TODO file" + io.to_s.should contain "Please fix syntax issues" + end end end end diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 46ec4998..bcff74fb 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -74,14 +74,14 @@ module Ameba # A rule that always raises an error class RaiseRule < Rule::Base - property should_raise = false + property? should_raise = false properties do description "Internal rule that always raises" end def test(source) - should_raise && raise "something went wrong" + should_raise? && raise "something went wrong" end end From 60813e48996a8a79d04774e31a48d5e160b1af4e Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 22 Nov 2022 19:53:27 +0100 Subject: [PATCH 07/13] Test generated boolean rule property --- spec/ameba/base_spec.cr | 4 ++++ spec/ameba/formatter/todo_formatter_spec.cr | 1 + spec/spec_helper.cr | 1 + 3 files changed, 6 insertions(+) diff --git a/spec/ameba/base_spec.cr b/spec/ameba/base_spec.cr index 21ced084..c765168b 100644 --- a/spec/ameba/base_spec.cr +++ b/spec/ameba/base_spec.cr @@ -32,6 +32,10 @@ module Ameba::Rule subject.description.should_not be_nil end + it "has a dummy? property" do + subject.dummy?.should be_true + end + it "has excluded property" do subject.excluded.should be_nil end diff --git a/spec/ameba/formatter/todo_formatter_spec.cr b/spec/ameba/formatter/todo_formatter_spec.cr index bd7497ef..3f7a8214 100644 --- a/spec/ameba/formatter/todo_formatter_spec.cr +++ b/spec/ameba/formatter/todo_formatter_spec.cr @@ -102,6 +102,7 @@ module Ameba # Run `ameba --only Ameba/DummyRule` for details Ameba/DummyRule: Description: Dummy rule that does nothing. + Dummy: true Excluded: - source1.cr - source2.cr diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index bcff74fb..32855f95 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -7,6 +7,7 @@ module Ameba class DummyRule < Rule::Base properties do description : String = "Dummy rule that does nothing." + dummy true end def test(source) From 4500181ddbdb087263bafb27b621f5c10c6269c8 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 23 Nov 2022 03:21:15 +0100 Subject: [PATCH 08/13] Remove redundant blank comment lines --- src/ameba/ast/branch.cr | 1 - src/ameba/ast/visitors/node_visitor.cr | 1 - src/ameba/tokenizer.cr | 1 - 3 files changed, 3 deletions(-) diff --git a/src/ameba/ast/branch.cr b/src/ameba/ast/branch.cr index 647ebafd..3954ac55 100644 --- a/src/ameba/ast/branch.cr +++ b/src/ameba/ast/branch.cr @@ -11,7 +11,6 @@ module Ameba::AST # do_something a # --> Branch D # end # ``` - # class Branch # The actual branch node. getter node : Crystal::ASTNode diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr index 2c35adb1..4dca9f7f 100644 --- a/src/ameba/ast/visitors/node_visitor.cr +++ b/src/ameba/ast/visitors/node_visitor.cr @@ -35,7 +35,6 @@ module Ameba::AST # ``` # visitor = Ameba::AST::NodeVisitor.new(rule, source) # ``` - # class NodeVisitor < BaseVisitor @skip : Array(Crystal::ASTNode.class)? diff --git a/src/ameba/tokenizer.cr b/src/ameba/tokenizer.cr index 0c269b26..86f6aa3b 100644 --- a/src/ameba/tokenizer.cr +++ b/src/ameba/tokenizer.cr @@ -10,7 +10,6 @@ module Ameba # puts token # end # ``` - # class Tokenizer # Instantiates Tokenizer using a `source`. # From ffd63ef0282867f649dc064292f0b43394c2e363 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 23 Nov 2022 03:21:49 +0100 Subject: [PATCH 09/13] Fix typo in `AST::Branchable` example --- src/ameba/ast/branchable.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ameba/ast/branchable.cr b/src/ameba/ast/branchable.cr index 4bf9819e..909f8a09 100644 --- a/src/ameba/ast/branchable.cr +++ b/src/ameba/ast/branchable.cr @@ -6,7 +6,7 @@ module Ameba::AST # are branchables. # # ``` - # white a > 100 # Branchable A + # while a > 100 # Branchable A # if b > 2 # Branchable B # a += 1 # end From 8b43a40a65d4d7028e4fb2108e2e844e075ff87f Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 23 Nov 2022 03:22:27 +0100 Subject: [PATCH 10/13] Misc refactors --- src/ameba/ast/branchable.cr | 2 +- src/ameba/ast/variabling/assignment.cr | 3 ++- src/ameba/ast/variabling/variable.cr | 2 +- src/ameba/ast/visitors/counting_visitor.cr | 6 +++--- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ameba/ast/branchable.cr b/src/ameba/ast/branchable.cr index 909f8a09..47eb5972 100644 --- a/src/ameba/ast/branchable.cr +++ b/src/ameba/ast/branchable.cr @@ -37,7 +37,7 @@ module Ameba::AST # Returns true if this node or one of the parent branchables is a loop, false otherwise. def loop? - loop?(node) || parent.try(&.loop?) || false + loop?(node) || !!parent.try(&.loop?) end end end diff --git a/src/ameba/ast/variabling/assignment.cr b/src/ameba/ast/variabling/assignment.cr index 11a7fbbc..f0d80c6f 100644 --- a/src/ameba/ast/variabling/assignment.cr +++ b/src/ameba/ast/variabling/assignment.cr @@ -30,6 +30,7 @@ module Ameba::AST # ``` def initialize(@node, @variable, @scope) return unless scope = @variable.scope + @branch = Branch.of(@node, scope) @referenced = true if @variable.special? || @variable.scope.type_definition? || @@ -37,7 +38,7 @@ module Ameba::AST end def referenced_in_loop? - @variable.referenced? && @branch.try &.in_loop? + @variable.referenced? && !!@branch.try(&.in_loop?) end # Returns true if this assignment is an op assign, false if not. diff --git a/src/ameba/ast/variabling/variable.cr b/src/ameba/ast/variabling/variable.cr index 19bad442..13ba570c 100644 --- a/src/ameba/ast/variabling/variable.cr +++ b/src/ameba/ast/variabling/variable.cr @@ -161,7 +161,7 @@ module Ameba::AST def declared_before?(node) var_location, node_location = location, node.location - return if var_location.nil? || node_location.nil? + return unless var_location && node_location (var_location.line_number < node_location.line_number) || (var_location.line_number == node_location.line_number && diff --git a/src/ameba/ast/visitors/counting_visitor.cr b/src/ameba/ast/visitors/counting_visitor.cr index a9e7dc72..7bb0bb7c 100644 --- a/src/ameba/ast/visitors/counting_visitor.cr +++ b/src/ameba/ast/visitors/counting_visitor.cr @@ -3,7 +3,7 @@ module Ameba::AST class CountingVisitor < Crystal::Visitor DEFAULT_COMPLEXITY = 1 - getter macro_condition = false + getter? macro_condition = false # Creates a new counting visitor def initialize(@scope : Crystal::ASTNode) @@ -27,13 +27,13 @@ module Ameba::AST {% for node in %i(if while until rescue or and) %} # :nodoc: def visit(node : Crystal::{{ node.id.capitalize }}) - @complexity += 1 unless macro_condition + @complexity += 1 unless macro_condition? end {% end %} # :nodoc: def visit(node : Crystal::Case) - return true if macro_condition + return true if macro_condition? # Count the complexity of an exhaustive `Case` as 1 # Otherwise count the number of `When`s From 2c67fe2c3f9d215e0a70c60297c48ffa22e319d8 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 23 Nov 2022 03:23:42 +0100 Subject: [PATCH 11/13] Use tuple instead of an array --- spec/spec_helper.cr | 4 ++-- src/ameba/ast/visitors/node_visitor.cr | 4 ++-- src/ameba/ast/visitors/scope_visitor.cr | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 32855f95..86e992c4 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -241,7 +241,7 @@ module Ameba end class TestNodeVisitor < Crystal::Visitor - NODES = [ + NODES = { Crystal::NilLiteral, Crystal::Var, Crystal::Assign, @@ -255,7 +255,7 @@ module Ameba Crystal::MacroLiteral, Crystal::Expressions, Crystal::ControlExpression, - ] + } def initialize(node) node.accept self diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr index 4dca9f7f..4e6e3618 100644 --- a/src/ameba/ast/visitors/node_visitor.cr +++ b/src/ameba/ast/visitors/node_visitor.cr @@ -2,7 +2,7 @@ require "./base_visitor" module Ameba::AST # List of nodes to be visited by Ameba's rules. - NODES = [ + NODES = { Alias, IsA, Assign, @@ -27,7 +27,7 @@ module Ameba::AST When, While, Until, - ] + } # An AST Visitor that traverses the source and allows all nodes # to be inspected by rules. diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr index 5abf4056..be369b2c 100644 --- a/src/ameba/ast/visitors/scope_visitor.cr +++ b/src/ameba/ast/visitors/scope_visitor.cr @@ -4,7 +4,7 @@ module Ameba::AST # AST Visitor that traverses the source and constructs scopes. class ScopeVisitor < BaseVisitor # Non-exhaustive list of nodes to be visited by Ameba's rules. - NODES = [ + NODES = { ClassDef, ModuleDef, EnumDef, @@ -17,7 +17,7 @@ module Ameba::AST Block, Macro, MacroFor, - ] + } SUPER_NODE_NAME = "super" RECORD_NODE_NAME = "record" From 38eb5d5e5085cfaa4e4a6b6439791fb3fbe94fb1 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 23 Nov 2022 03:26:55 +0100 Subject: [PATCH 12/13] Fix invalid crystal syntax in `Lint/Syntax` rule spec --- spec/ameba/rule/lint/syntax_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ameba/rule/lint/syntax_spec.cr b/spec/ameba/rule/lint/syntax_spec.cr index 2412b0b2..107752ac 100644 --- a/spec/ameba/rule/lint/syntax_spec.cr +++ b/spec/ameba/rule/lint/syntax_spec.cr @@ -8,7 +8,7 @@ module Ameba::Rule::Lint expect_no_issues subject, <<-CRYSTAL def hello puts "totally valid" - rescue e: Exception + rescue e : Exception end CRYSTAL end From 735ec2462a4c12eb1e7b5211673486b8bf2287e9 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 23 Nov 2022 05:36:10 +0100 Subject: [PATCH 13/13] Refactor usages of `\` to sth more readable if possible --- src/ameba/cli/cmd.cr | 9 +++++---- src/ameba/formatter/dot_formatter.cr | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/ameba/cli/cmd.cr b/src/ameba/cli/cmd.cr index 71051724..6cebd608 100644 --- a/src/ameba/cli/cmd.cr +++ b/src/ameba/cli/cmd.cr @@ -184,10 +184,11 @@ module Ameba::Cli private def print_rules(config) config.rules.each do |rule| - puts \ - "#{rule.name.colorize(:white)} " \ - "[#{rule.severity.symbol.to_s.colorize(:green)}] - " \ - "#{rule.description.colorize(:dark_gray)}" + puts "%s [%s] - %s" % { + rule.name.colorize(:white), + rule.severity.symbol.to_s.colorize(:green), + rule.description.colorize(:dark_gray), + } end exit 0 end diff --git a/src/ameba/formatter/dot_formatter.cr b/src/ameba/formatter/dot_formatter.cr index 788d9135..38575e18 100644 --- a/src/ameba/formatter/dot_formatter.cr +++ b/src/ameba/formatter/dot_formatter.cr @@ -45,10 +45,11 @@ module Ameba::Formatter end end output.puts - output.puts \ - "[#{issue.rule.severity.symbol}] " \ - "#{issue.rule.name}: " \ - "#{issue.message}".colorize(:red) + output.puts ("[%s] %s: %s" % { + issue.rule.severity.symbol, + issue.rule.name, + issue.message, + }).colorize(:red) if show_affected_code && (code = affected_code(issue)) output << code.colorize(:default)