From 03adc20872a36ec020d22fcc32e696bd3eb754f8 Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Fri, 11 Mar 2022 14:15:05 +0200 Subject: [PATCH 1/7] Pass on crystal-nightly --- spec/ameba/tokenizer_spec.cr | 2 +- src/ameba/ast/util.cr | 6 +++--- src/ameba/rule/lint/bad_directive.cr | 2 +- src/ameba/rule/lint/percent_array.cr | 6 +++--- .../rule/lint/unneeded_disable_directive.cr | 2 +- src/ameba/rule/style/large_numbers.cr | 2 +- src/ameba/rule/style/redundant_begin.cr | 14 +++++++------- src/ameba/tokenizer.cr | 18 +++++++++--------- 8 files changed, 26 insertions(+), 26 deletions(-) diff --git a/spec/ameba/tokenizer_spec.cr b/spec/ameba/tokenizer_spec.cr index 2970f9d8..58d1d8e6 100644 --- a/spec/ameba/tokenizer_spec.cr +++ b/spec/ameba/tokenizer_spec.cr @@ -3,7 +3,7 @@ require "../spec_helper" module Ameba private def it_tokenizes(str, expected) it "tokenizes #{str}" do - ([] of Symbol).tap do |token_types| + ([] of Crystal::Token::Kind).tap do |token_types| Tokenizer.new(Source.new str, normalize: false) .run { |token| token_types << token.type } .should be_true diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr index dd577f30..3b210661 100644 --- a/src/ameba/ast/util.cr +++ b/src/ameba/ast/util.cr @@ -194,9 +194,9 @@ module Ameba::AST::Util return 0 unless node.responds_to?(:name) && (name = node.name) case name - when Crystal::ASTNode then name.name_size - when Symbol then name.to_s.size # Crystal::MagicConstant - else name.size + when Crystal::ASTNode then name.name_size + when Crystal::Token::Kind then name.to_s.size # Crystal::MagicConstant + else name.size end end diff --git a/src/ameba/rule/lint/bad_directive.cr b/src/ameba/rule/lint/bad_directive.cr index 3d70c2d5..e0ff30f4 100644 --- a/src/ameba/rule/lint/bad_directive.cr +++ b/src/ameba/rule/lint/bad_directive.cr @@ -28,7 +28,7 @@ module Ameba::Rule::Lint def test(source) Tokenizer.new(source).run do |token| - next unless token.type == :COMMENT + next unless token.type.comment? next unless directive = source.parse_inline_directive(token.value.to_s) check_action source, token, directive[:action] diff --git a/src/ameba/rule/lint/percent_array.cr b/src/ameba/rule/lint/percent_array.cr index 3109fa15..fde05e88 100644 --- a/src/ameba/rule/lint/percent_array.cr +++ b/src/ameba/rule/lint/percent_array.cr @@ -38,13 +38,13 @@ module Ameba::Rule::Lint Tokenizer.new(source).run do |token| case token.type - when :STRING_ARRAY_START, :SYMBOL_ARRAY_START + when .string_array_start?, .symbol_array_start? start_token = token.dup - when :STRING + when .string? if start_token && issue.nil? issue = array_entry_invalid?(token.value, start_token.not_nil!.raw) end - when :STRING_ARRAY_END, :SYMBOL_ARRAY_END + when .string_array_end? if issue issue_for start_token.not_nil!, issue.not_nil! end diff --git a/src/ameba/rule/lint/unneeded_disable_directive.cr b/src/ameba/rule/lint/unneeded_disable_directive.cr index 4a55f446..a32215a8 100644 --- a/src/ameba/rule/lint/unneeded_disable_directive.cr +++ b/src/ameba/rule/lint/unneeded_disable_directive.cr @@ -33,7 +33,7 @@ module Ameba::Rule::Lint def test(source) Tokenizer.new(source).run do |token| - next unless token.type == :COMMENT + next unless token.type.comment? next unless directive = source.parse_inline_directive(token.value.to_s) next unless names = unneeded_disables(source, directive, token.location) next if names.empty? diff --git a/src/ameba/rule/style/large_numbers.cr b/src/ameba/rule/style/large_numbers.cr index 08cb19c4..b8bba3cf 100644 --- a/src/ameba/rule/style/large_numbers.cr +++ b/src/ameba/rule/style/large_numbers.cr @@ -37,7 +37,7 @@ module Ameba::Rule::Style def test(source) Tokenizer.new(source).run do |token| - next unless token.type == :NUMBER && decimal?(token.raw) + next unless token.type.number? && decimal?(token.raw) parsed = parse_number token.raw diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr index cddad1ed..7db7baa0 100644 --- a/src/ameba/rule/style/redundant_begin.cr +++ b/src/ameba/rule/style/redundant_begin.cr @@ -119,19 +119,19 @@ module Ameba::Rule::Style token = lexer.next_token case token.type - when :EOF, :"->" + when .eof?, .op_minus_gt? break - when :IDENT + when .ident? next unless in_body return unless token.value == :begin return token.location - when :"(" + when .op_lparen? in_argument_list = true - when :")" + when .op_rparen? in_argument_list = false - when :NEWLINE + when .newline? in_body = true unless in_argument_list - when :SPACE + when .space? # ignore else return if in_body @@ -142,7 +142,7 @@ module Ameba::Rule::Style private def def_redundant_end_loc(lexer) end_loc = def_end_loc = nil - while (token = lexer.next_token).type != :EOF + while !(token = lexer.next_token).type.eof? next unless token.value == :end end_loc, def_end_loc = def_end_loc, token.location diff --git a/src/ameba/tokenizer.cr b/src/ameba/tokenizer.cr index 5703a95a..5ccd24fc 100644 --- a/src/ameba/tokenizer.cr +++ b/src/ameba/tokenizer.cr @@ -56,13 +56,13 @@ module Ameba block.call token case token.type - when :DELIMITER_START + when .delimiter_start? run_delimiter_state lexer, token, &block - when :STRING_ARRAY_START, :SYMBOL_ARRAY_START + when .string_array_start?, .symbol_array_start? run_array_state lexer, token, &block - when :EOF + when .op_rcurly? break - when :"}" + when .eof? break if break_on_rcurly end end @@ -74,11 +74,11 @@ module Ameba block.call token case token.type - when :DELIMITER_END + when .delimiter_end? break - when :INTERPOLATION_START + when .interpolation_start? run_normal_state lexer, break_on_rcurly: true, &block - when :EOF + when .eof? break end end @@ -90,9 +90,9 @@ module Ameba block.call token case token.type - when :STRING_ARRAY_END + when .string_array_end? break - when :EOF + when .eof? break end end From 1cfc926a284c7200ba6ec75fb3bee6fe9961853f Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Fri, 11 Mar 2022 16:18:33 +0200 Subject: [PATCH 2/7] Fix specs for tokenizer --- spec/ameba/tokenizer_spec.cr | 22 +++++++++++----------- src/ameba/tokenizer.cr | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/spec/ameba/tokenizer_spec.cr b/spec/ameba/tokenizer_spec.cr index 58d1d8e6..674ade29 100644 --- a/spec/ameba/tokenizer_spec.cr +++ b/spec/ameba/tokenizer_spec.cr @@ -3,9 +3,9 @@ require "../spec_helper" module Ameba private def it_tokenizes(str, expected) it "tokenizes #{str}" do - ([] of Crystal::Token::Kind).tap do |token_types| + ([] of String).tap do |token_types| Tokenizer.new(Source.new str, normalize: false) - .run { |token| token_types << token.type } + .run { |token| token_types << token.type.to_s } .should be_true end.should eq expected end @@ -13,20 +13,20 @@ module Ameba describe Tokenizer do describe "#run" do - it_tokenizes %("string"), %i(DELIMITER_START STRING DELIMITER_END EOF) - it_tokenizes %(100), %i(NUMBER EOF) - it_tokenizes %('a'), %i(CHAR EOF) - it_tokenizes %([]), %i([] EOF) - it_tokenizes %([] of String), %i([] SPACE IDENT SPACE CONST EOF) - it_tokenizes %q("str #{3}"), %i( + it_tokenizes %("string"), %w(DELIMITER_START STRING DELIMITER_END EOF) + it_tokenizes %(100), %w(NUMBER EOF) + it_tokenizes %('a'), %w(CHAR EOF) + it_tokenizes %([]), %w([] EOF) + it_tokenizes %([] of String), %w([] SPACE IDENT SPACE CONST EOF) + it_tokenizes %q("str #{3}"), %w( DELIMITER_START STRING INTERPOLATION_START NUMBER } DELIMITER_END EOF ) it_tokenizes %(%w(1 2)), - %i(STRING_ARRAY_START STRING STRING STRING_ARRAY_END EOF) + %w(STRING_ARRAY_START STRING STRING STRING_ARRAY_END EOF) it_tokenizes %(%i(one two)), - %i(SYMBOL_ARRAY_START STRING STRING STRING_ARRAY_END EOF) + %w(SYMBOL_ARRAY_START STRING STRING STRING_ARRAY_END EOF) it_tokenizes %( class A @@ -34,7 +34,7 @@ module Ameba puts "hello" end end - ), %i( + ), %w( NEWLINE SPACE IDENT SPACE CONST NEWLINE SPACE IDENT SPACE IDENT NEWLINE SPACE IDENT SPACE DELIMITER_START STRING DELIMITER_END NEWLINE SPACE IDENT NEWLINE SPACE IDENT NEWLINE SPACE EOF diff --git a/src/ameba/tokenizer.cr b/src/ameba/tokenizer.cr index 5ccd24fc..0c269b26 100644 --- a/src/ameba/tokenizer.cr +++ b/src/ameba/tokenizer.cr @@ -60,9 +60,9 @@ module Ameba run_delimiter_state lexer, token, &block when .string_array_start?, .symbol_array_start? run_array_state lexer, token, &block - when .op_rcurly? - break when .eof? + break + when .op_rcurly? break if break_on_rcurly end end From 7192b64df0bafef4026e29ae736fa6e7088ccf6c Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Fri, 11 Mar 2022 18:00:25 +0200 Subject: [PATCH 3/7] Fix rest of the specs --- spec/ameba/rule/lint/empty_ensure_spec.cr | 2 +- spec/ameba/rule/lint/shadowed_exception_spec.cr | 2 +- ...ble_directive_spec.cr => unneeded_disable_directive_spec.cr} | 0 src/ameba/inline_comments.cr | 2 +- src/ameba/rule/style/redundant_begin.cr | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename spec/ameba/rule/lint/{unneded_disable_directive_spec.cr => unneeded_disable_directive_spec.cr} (100%) diff --git a/spec/ameba/rule/lint/empty_ensure_spec.cr b/spec/ameba/rule/lint/empty_ensure_spec.cr index bba7a6d8..d979306e 100644 --- a/spec/ameba/rule/lint/empty_ensure_spec.cr +++ b/spec/ameba/rule/lint/empty_ensure_spec.cr @@ -61,7 +61,7 @@ module Ameba::Rule::Lint issue = s.issues.first issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:3" + issue.location.to_s.should eq "source.cr:1:1" issue.end_location.to_s.should eq "source.cr:6:3" issue.message.should eq "Empty `ensure` block detected" end diff --git a/spec/ameba/rule/lint/shadowed_exception_spec.cr b/spec/ameba/rule/lint/shadowed_exception_spec.cr index 5b7d4933..9546d7d4 100644 --- a/spec/ameba/rule/lint/shadowed_exception_spec.cr +++ b/spec/ameba/rule/lint/shadowed_exception_spec.cr @@ -166,7 +166,7 @@ module Ameba::Rule::Lint issue = s.issues.first issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:3" + issue.location.to_s.should eq "source.cr:1:1" issue.end_location.to_s.should eq "source.cr:4:3" issue.message.should eq( "Exception handler has shadowed exceptions: IndexError" diff --git a/spec/ameba/rule/lint/unneded_disable_directive_spec.cr b/spec/ameba/rule/lint/unneeded_disable_directive_spec.cr similarity index 100% rename from spec/ameba/rule/lint/unneded_disable_directive_spec.cr rename to spec/ameba/rule/lint/unneeded_disable_directive_spec.cr diff --git a/src/ameba/inline_comments.cr b/src/ameba/inline_comments.cr index 648f1f98..160a9144 100644 --- a/src/ameba/inline_comments.cr +++ b/src/ameba/inline_comments.cr @@ -95,7 +95,7 @@ module Ameba commented = false lexer = Crystal::Lexer.new(line).tap(&.comments_enabled = true) - Tokenizer.new(lexer).run { |t| commented = true if t.type == :COMMENT } + Tokenizer.new(lexer).run { |t| commented = true if t.type.comment? } commented end end diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr index 7db7baa0..9e5609ac 100644 --- a/src/ameba/rule/style/redundant_begin.cr +++ b/src/ameba/rule/style/redundant_begin.cr @@ -90,7 +90,7 @@ module Ameba::Rule::Style end private def redundant_begin_in_expressions?(node) - node.keyword == :begin + node.keyword == Crystal::Expressions::Keyword::Begin end private def inner_handler?(handler) From 087f470f155dbf5346535662b9e7e647ab0a3f73 Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Sun, 3 Apr 2022 19:17:47 +0300 Subject: [PATCH 4/7] Backward compatibility to Crystal 1.3 --- shard.yml | 2 +- spec/ameba/ast/util_spec.cr | 8 +- spec/ameba/rule/lint/empty_ensure_spec.cr | 4 +- .../rule/lint/shadowed_exception_spec.cr | 4 +- src/ameba/ast/util.cr | 5 +- src/ameba/ext/override.cr | 81 +++++++++++++++++++ src/ameba/rule/style/redundant_begin.cr | 2 +- 7 files changed, 94 insertions(+), 12 deletions(-) create mode 100644 src/ameba/ext/override.cr diff --git a/shard.yml b/shard.yml index 688957a4..48a10106 100644 --- a/shard.yml +++ b/shard.yml @@ -15,6 +15,6 @@ scripts: executables: - ameba -crystal: ">= 0.35.0" +crystal: ">= 0.36.0" license: MIT diff --git a/spec/ameba/ast/util_spec.cr b/spec/ameba/ast/util_spec.cr index 9132eeeb..1e83851d 100644 --- a/spec/ameba/ast/util_spec.cr +++ b/spec/ameba/ast/util_spec.cr @@ -1,5 +1,4 @@ require "../../spec_helper" -require "semantic_version" module Ameba::AST struct Test @@ -77,12 +76,7 @@ module Ameba::AST CRYSTAL node = as_nodes(s).nil_literal_nodes.first source = subject.node_source node, s.split("\n") - - if SemanticVersion.parse(Crystal::VERSION) <= SemanticVersion.parse("0.35.1") - source.should be_nil - else - source.should eq "nil" - end + source.should eq "nil" end end diff --git a/spec/ameba/rule/lint/empty_ensure_spec.cr b/spec/ameba/rule/lint/empty_ensure_spec.cr index d979306e..3eeaa2b7 100644 --- a/spec/ameba/rule/lint/empty_ensure_spec.cr +++ b/spec/ameba/rule/lint/empty_ensure_spec.cr @@ -61,7 +61,9 @@ module Ameba::Rule::Lint issue = s.issues.first issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" + # TODO: refactor this in next release + # Crystal 1.4 changes the start location of Crystal::ExceptionHandler + # issue.location.to_s.should eq "source.cr:2:3" issue.end_location.to_s.should eq "source.cr:6:3" issue.message.should eq "Empty `ensure` block detected" end diff --git a/spec/ameba/rule/lint/shadowed_exception_spec.cr b/spec/ameba/rule/lint/shadowed_exception_spec.cr index 9546d7d4..5cad8bd7 100644 --- a/spec/ameba/rule/lint/shadowed_exception_spec.cr +++ b/spec/ameba/rule/lint/shadowed_exception_spec.cr @@ -166,7 +166,9 @@ module Ameba::Rule::Lint issue = s.issues.first issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" + # TODO: refactor this in next release + # Crystal 1.4 changes the start location of Crystal::ExceptionHandler + # issue.location.to_s.should eq "source.cr:2:3" issue.end_location.to_s.should eq "source.cr:4:3" issue.message.should eq( "Exception handler has shadowed exceptions: IndexError" diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr index 3b210661..4f72bda2 100644 --- a/src/ameba/ast/util.cr +++ b/src/ameba/ast/util.cr @@ -194,7 +194,10 @@ module Ameba::AST::Util return 0 unless node.responds_to?(:name) && (name = node.name) case name - when Crystal::ASTNode then name.name_size + when Crystal::ASTNode then name.name_size + # TODO: remove in a next release + # (preserves backward compatibility of crystal <= 1.3.2 ) + when Symbol then name.to_s.size # Crystal::MagicConstant when Crystal::Token::Kind then name.to_s.size # Crystal::MagicConstant else name.size end diff --git a/src/ameba/ext/override.cr b/src/ameba/ext/override.cr new file mode 100644 index 00000000..25419c6f --- /dev/null +++ b/src/ameba/ext/override.cr @@ -0,0 +1,81 @@ +# TODO: remove in a next release +# (preserves backward compatibility of crystal <= 1.3.2 ) +{% if compare_versions(Crystal::VERSION, "1.3.2") < 1 %} + struct Symbol + def comment? + self == :COMMENT + end + + def delimiter_start? + self == :DELIMITER_START + end + + def delimiter_end? + self == :DELIMITER_END + end + + def interpolation_start? + self == :INTERPOLATION_START + end + + def string_array_start? + self == :STRING_ARRAY_START + end + + def string_array_end? + self == :STRING_ARRAY_END + end + + def symbol_array_start? + self == :SYMBOL_ARRAY_START + end + + def eof? + self == :EOF + end + + def op_rcurly? + self == :"}" + end + + def begin? + self == :begin + end + + def op_minus_gt? + self == :"->" + end + + def ident? + self == :IDENT + end + + def op_lparen? + self == :"(" + end + + def op_rparen? + self == :")" + end + + def newline? + self == :NEWLINE + end + + def space? + self == :SPACE + end + + def number? + self == :NUMBER + end + + def string? + self == :STRING + end + end + + struct Crystal::Token::Kind + # + end +{% end %} diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr index 9e5609ac..b20c7091 100644 --- a/src/ameba/rule/style/redundant_begin.cr +++ b/src/ameba/rule/style/redundant_begin.cr @@ -90,7 +90,7 @@ module Ameba::Rule::Style end private def redundant_begin_in_expressions?(node) - node.keyword == Crystal::Expressions::Keyword::Begin + node.keyword.try &.begin? end private def inner_handler?(handler) From 2f29999106fa53eb5a657e7938b8d482a26792e2 Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Sun, 3 Apr 2022 19:43:51 +0300 Subject: [PATCH 5/7] Return boolean --- src/ameba/rule/style/redundant_begin.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr index b20c7091..4b15bee6 100644 --- a/src/ameba/rule/style/redundant_begin.cr +++ b/src/ameba/rule/style/redundant_begin.cr @@ -90,7 +90,7 @@ module Ameba::Rule::Style end private def redundant_begin_in_expressions?(node) - node.keyword.try &.begin? + !!(node.keyword.try &.begin?) end private def inner_handler?(handler) From 1f4dda1c4a5a189f2fcb71336fd8486746e743ca Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Sun, 3 Apr 2022 20:16:00 +0300 Subject: [PATCH 6/7] Add conditional check using semantic version --- spec/ameba/rule/lint/empty_ensure_spec.cr | 7 ++++++- spec/ameba/rule/lint/shadowed_exception_spec.cr | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/spec/ameba/rule/lint/empty_ensure_spec.cr b/spec/ameba/rule/lint/empty_ensure_spec.cr index 3eeaa2b7..3e59b19d 100644 --- a/spec/ameba/rule/lint/empty_ensure_spec.cr +++ b/spec/ameba/rule/lint/empty_ensure_spec.cr @@ -1,4 +1,5 @@ require "../../../spec_helper" +require "semantic_version" module Ameba::Rule::Lint describe EmptyEnsure do @@ -63,7 +64,11 @@ module Ameba::Rule::Lint issue.rule.should_not be_nil # TODO: refactor this in next release # Crystal 1.4 changes the start location of Crystal::ExceptionHandler - # issue.location.to_s.should eq "source.cr:2:3" + if SemanticVersion.parse(Crystal::VERSION) <= SemanticVersion.parse("1.3.2") + issue.location.to_s.should eq "source.cr:2:3" + else + issue.location.to_s.should eq "source.cr:1:1" + end issue.end_location.to_s.should eq "source.cr:6:3" issue.message.should eq "Empty `ensure` block detected" end diff --git a/spec/ameba/rule/lint/shadowed_exception_spec.cr b/spec/ameba/rule/lint/shadowed_exception_spec.cr index 5cad8bd7..852d7c13 100644 --- a/spec/ameba/rule/lint/shadowed_exception_spec.cr +++ b/spec/ameba/rule/lint/shadowed_exception_spec.cr @@ -1,4 +1,5 @@ require "../../../spec_helper" +require "semantic_version" private def check_shadowed(source, exceptions) s = Ameba::Source.new source @@ -168,7 +169,11 @@ module Ameba::Rule::Lint issue.rule.should_not be_nil # TODO: refactor this in next release # Crystal 1.4 changes the start location of Crystal::ExceptionHandler - # issue.location.to_s.should eq "source.cr:2:3" + if SemanticVersion.parse(Crystal::VERSION) <= SemanticVersion.parse("1.3.2") + issue.location.to_s.should eq "source.cr:2:3" + else + issue.location.to_s.should eq "source.cr:1:1" + end issue.end_location.to_s.should eq "source.cr:4:3" issue.message.should eq( "Exception handler has shadowed exceptions: IndexError" From 8d4730182f7600dc142b2a1d5eb4814af9fd3a44 Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Mon, 4 Apr 2022 00:56:11 +0300 Subject: [PATCH 7/7] Rework breaking specs, break backward compatibility --- shard.yml | 2 +- spec/ameba/rule/lint/empty_ensure_spec.cr | 42 ++----- .../rule/lint/shadowed_exception_spec.cr | 105 +++++++++--------- src/ameba/ast/util.cr | 5 +- src/ameba/ext/override.cr | 81 -------------- src/ameba/rule/lint/empty_ensure.cr | 2 +- src/ameba/rule/lint/shadowed_exception.cr | 52 +++++---- src/ameba/rule/style/redundant_begin.cr | 2 +- 8 files changed, 94 insertions(+), 197 deletions(-) delete mode 100644 src/ameba/ext/override.cr diff --git a/shard.yml b/shard.yml index 48a10106..0ac4f66b 100644 --- a/shard.yml +++ b/shard.yml @@ -15,6 +15,6 @@ scripts: executables: - ameba -crystal: ">= 0.36.0" +crystal: ">= 1.4.0" license: MIT diff --git a/spec/ameba/rule/lint/empty_ensure_spec.cr b/spec/ameba/rule/lint/empty_ensure_spec.cr index 3e59b19d..5b7f11b0 100644 --- a/spec/ameba/rule/lint/empty_ensure_spec.cr +++ b/spec/ameba/rule/lint/empty_ensure_spec.cr @@ -1,12 +1,11 @@ require "../../../spec_helper" -require "semantic_version" module Ameba::Rule::Lint describe EmptyEnsure do subject = EmptyEnsure.new it "passes if there is no empty ensure blocks" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def some_method do_some_stuff ensure @@ -24,53 +23,30 @@ module Ameba::Rule::Lint ensure nil end - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if there is an empty ensure in method" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method do_some_stuff ensure + # ^^^^^^ error: Empty `ensure` block detected end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "fails if there is an empty ensure in a block" do - s = Source.new %( - begin - do_some_stuff - ensure - # nothing here - end - ) - subject.catch(s).should_not be_valid - end - - it "reports rule, pos and message" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL begin do_some_stuff rescue do_some_other_stuff ensure + # ^^^^^^ error: Empty `ensure` block detected + # nothing here end - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - # TODO: refactor this in next release - # Crystal 1.4 changes the start location of Crystal::ExceptionHandler - if SemanticVersion.parse(Crystal::VERSION) <= SemanticVersion.parse("1.3.2") - issue.location.to_s.should eq "source.cr:2:3" - else - issue.location.to_s.should eq "source.cr:1:1" - end - issue.end_location.to_s.should eq "source.cr:6:3" - issue.message.should eq "Empty `ensure` block detected" + CRYSTAL end end end diff --git a/spec/ameba/rule/lint/shadowed_exception_spec.cr b/spec/ameba/rule/lint/shadowed_exception_spec.cr index 852d7c13..7a3c97fb 100644 --- a/spec/ameba/rule/lint/shadowed_exception_spec.cr +++ b/spec/ameba/rule/lint/shadowed_exception_spec.cr @@ -1,18 +1,11 @@ require "../../../spec_helper" -require "semantic_version" - -private def check_shadowed(source, exceptions) - s = Ameba::Source.new source - Ameba::Rule::Lint::ShadowedException.new.catch(s).should_not be_valid - s.issues.first.message.should contain exceptions.join(", ") -end module Ameba::Rule::Lint describe ShadowedException do subject = ShadowedException.new it "passes if there isn't shadowed exception" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method do_something rescue ArgumentError @@ -32,152 +25,158 @@ module Ameba::Rule::Lint rescue e : Exception handle_exception end - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if there is a shadowed exception" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin do_something rescue Exception handle_exception rescue ArgumentError + # ^^^^^^^^^^^^^ error: Shadowed exception found: ArgumentError handle_argument_error_exception end - ), %w(ArgumentError) + CRYSTAL end it "fails if there is a custom shadowed exceptions" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin 1 rescue Exception 2 rescue MySuperException + # ^^^^^^^^^^^^^^^^ error: Shadowed exception found: MySuperException 3 end - ), %w(MySuperException) + CRYSTAL end it "fails if there is a shadowed exception in a type list" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue Exception | IndexError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError end - ), %w(IndexError) + CRYSTAL end it "fails if there is a first shadowed exception in a type list" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue IndexError | Exception + # ^^^^^^^^^^ error: Shadowed exception found: IndexError rescue Exception + # ^^^^^^^^^ error: Shadowed exception found: Exception rescue end - ), %w(IndexError) + CRYSTAL end it "fails if there is a shadowed duplicated exception" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue IndexError rescue ArgumentError rescue IndexError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError end - ), %w(IndexError) + CRYSTAL end it "fails if there is a shadowed duplicated exception in a type list" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue IndexError rescue ArgumentError | IndexError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError end - ), %w(IndexError) + CRYSTAL end it "fails if there is only shadowed duplicated exceptions" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue IndexError rescue IndexError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError + rescue Exception end - ), %w(IndexError) + CRYSTAL end it "fails if there is only shadowed duplicated exceptions in a type list" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue IndexError | IndexError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError end - ), %w(IndexError) + CRYSTAL end it "fails if all rescues are shadowed and there is a catch-all rescue" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue Exception rescue ArgumentError + # ^^^^^^^^^^^^^ error: Shadowed exception found: ArgumentError rescue IndexError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError rescue KeyError | IO::Error + # ^^^^^^^^^ error: Shadowed exception found: IO::Error + # ^^^^^^^^ error: Shadowed exception found: KeyError rescue end - ), %w(IndexError KeyError IO::Error) + CRYSTAL end it "fails if there are shadowed exception with args" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue Exception rescue ex : IndexError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError rescue end - ), %w(IndexError) + CRYSTAL end it "fails if there are multiple shadowed exceptions" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue Exception rescue ArgumentError + # ^^^^^^^^^^^^^ error: Shadowed exception found: ArgumentError rescue IndexError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError end - ), %w(ArgumentError IndexError) + CRYSTAL end it "fails if there are multiple shadowed exceptions in a type list" do - check_shadowed %( + expect_issue subject, <<-CRYSTAL begin rescue Exception rescue ArgumentError | IndexError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError + # ^^^^^^^^^^^^^ error: Shadowed exception found: ArgumentError rescue IO::Error + # ^^^^^^^^^ error: Shadowed exception found: IO::Error end - ), %w(ArgumentError IndexError IO::Error) + CRYSTAL end - it "reports rule, location and a message" do - s = Source.new %q( + it "fails if there are multiple shadowed exceptions in a single rescue" do + expect_issue subject, <<-CRYSTAL begin do_something - rescue Exception | IndexError + rescue Exception | IndexError | ArgumentError + # ^^^^^^^^^^^^^ error: Shadowed exception found: ArgumentError + # ^^^^^^^^^^ error: Shadowed exception found: IndexError end - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - # TODO: refactor this in next release - # Crystal 1.4 changes the start location of Crystal::ExceptionHandler - if SemanticVersion.parse(Crystal::VERSION) <= SemanticVersion.parse("1.3.2") - issue.location.to_s.should eq "source.cr:2:3" - else - issue.location.to_s.should eq "source.cr:1:1" - end - issue.end_location.to_s.should eq "source.cr:4:3" - issue.message.should eq( - "Exception handler has shadowed exceptions: IndexError" - ) + CRYSTAL end end end diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr index 4f72bda2..3b210661 100644 --- a/src/ameba/ast/util.cr +++ b/src/ameba/ast/util.cr @@ -194,10 +194,7 @@ module Ameba::AST::Util return 0 unless node.responds_to?(:name) && (name = node.name) case name - when Crystal::ASTNode then name.name_size - # TODO: remove in a next release - # (preserves backward compatibility of crystal <= 1.3.2 ) - when Symbol then name.to_s.size # Crystal::MagicConstant + when Crystal::ASTNode then name.name_size when Crystal::Token::Kind then name.to_s.size # Crystal::MagicConstant else name.size end diff --git a/src/ameba/ext/override.cr b/src/ameba/ext/override.cr deleted file mode 100644 index 25419c6f..00000000 --- a/src/ameba/ext/override.cr +++ /dev/null @@ -1,81 +0,0 @@ -# TODO: remove in a next release -# (preserves backward compatibility of crystal <= 1.3.2 ) -{% if compare_versions(Crystal::VERSION, "1.3.2") < 1 %} - struct Symbol - def comment? - self == :COMMENT - end - - def delimiter_start? - self == :DELIMITER_START - end - - def delimiter_end? - self == :DELIMITER_END - end - - def interpolation_start? - self == :INTERPOLATION_START - end - - def string_array_start? - self == :STRING_ARRAY_START - end - - def string_array_end? - self == :STRING_ARRAY_END - end - - def symbol_array_start? - self == :SYMBOL_ARRAY_START - end - - def eof? - self == :EOF - end - - def op_rcurly? - self == :"}" - end - - def begin? - self == :begin - end - - def op_minus_gt? - self == :"->" - end - - def ident? - self == :IDENT - end - - def op_lparen? - self == :"(" - end - - def op_rparen? - self == :")" - end - - def newline? - self == :NEWLINE - end - - def space? - self == :SPACE - end - - def number? - self == :NUMBER - end - - def string? - self == :STRING - end - end - - struct Crystal::Token::Kind - # - end -{% end %} diff --git a/src/ameba/rule/lint/empty_ensure.cr b/src/ameba/rule/lint/empty_ensure.cr index e363f824..e01a08fa 100644 --- a/src/ameba/rule/lint/empty_ensure.cr +++ b/src/ameba/rule/lint/empty_ensure.cr @@ -49,7 +49,7 @@ module Ameba::Rule::Lint node_ensure = node.ensure return if node_ensure.nil? || !node_ensure.nop? - issue_for node, MSG + issue_for node.ensure_location, node.end_location, MSG end end end diff --git a/src/ameba/rule/lint/shadowed_exception.cr b/src/ameba/rule/lint/shadowed_exception.cr index 6e10de9d..6c779601 100644 --- a/src/ameba/rule/lint/shadowed_exception.cr +++ b/src/ameba/rule/lint/shadowed_exception.cr @@ -38,38 +38,44 @@ module Ameba::Rule::Lint description "Disallows rescued exception that get shadowed" end - MSG = "Exception handler has shadowed exceptions: %s" + MSG = "Shadowed exception found: %s" def test(source, node : Crystal::ExceptionHandler) - return unless excs = node.rescues.try &.map(&.types) - return if (excs = shadowed excs).empty? + rescues = node.rescues - issue_for node, MSG % excs.join(", ") + return if rescues.nil? + + shadowed(rescues).each { |tp| issue_for tp, MSG % tp.names.join("::") } end - private def shadowed(exceptions, exception_found = false) - previous_exceptions = [] of String + private def shadowed(rescues, catch_all = false) + traversed_types = Set(String).new - exceptions.reduce([] of String) do |shadowed, excs| - excs = excs.try(&.map(&.to_s)) || %w[Exception] - - if exception_found - shadowed.concat excs - previous_exceptions.concat excs + filter_rescues(rescues).each_with_object([] of Crystal::Path) do |types, shadowed| + case + when catch_all + shadowed.concat(types) + next + when types.any?(&.single?("Exception")) + nodes = types.reject(&.single?("Exception")) + shadowed.concat(nodes) unless nodes.empty? + catch_all = true + next else - exception_found ||= excs.any? &.== "Exception" - excs.each do |exc| - if exception_found && exc != "Exception" - shadowed << exc - else - shadowed << exc if previous_exceptions.any? &.== exc - end - previous_exceptions << exc - end + nodes = types.select { |tp| traverse(tp.to_s, traversed_types) } + shadowed.concat(nodes) unless nodes.empty? end - - shadowed end end + + private def filter_rescues(rescues) + rescues.compact_map(&.types.try &.select(Crystal::Path)) + end + + private def traverse(path, traversed_types) + dup = traversed_types.includes?(path) + dup || (traversed_types << path) + dup + end end end diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr index 4b15bee6..5e68efaf 100644 --- a/src/ameba/rule/style/redundant_begin.cr +++ b/src/ameba/rule/style/redundant_begin.cr @@ -90,7 +90,7 @@ module Ameba::Rule::Style end private def redundant_begin_in_expressions?(node) - !!(node.keyword.try &.begin?) + !!node.keyword.try(&.begin?) end private def inner_handler?(handler)