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)