Rework breaking specs, break backward compatibility

shard.yml

@@ -15,6 +15,6 @@ scripts:
 executables:
   - ameba
 
-crystal: ">= 0.36.0"
+crystal: ">= 1.4.0"
 
 license: MIT

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
-      )
+      CRYSTAL
-      subject.catch(s).should be_valid
     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
-      )
+      CRYSTAL
-      subject.catch(s).should_not be_valid
     end
 
     it "fails if there is an empty ensure in a block" do
-      s = Source.new %(
+      expect_issue subject, <<-CRYSTAL
-        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 %(
         begin
           do_some_stuff
         rescue
           do_some_other_stuff
         ensure
+      # ^^^^^^ error: Empty `ensure` block detected
+          # nothing here
         end
-      ), "source.cr"
+      CRYSTAL
-      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"
     end
   end
 end

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
-      )
+      CRYSTAL
-      subject.catch(s).should be_valid
     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
+    it "fails if there are multiple shadowed exceptions in a single rescue" do
-      s = Source.new %q(
+      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"
+        CRYSTAL
-      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"
-      )
     end
   end
 end

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
+    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

src/ameba/ext/override.cr

@@ -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 %}

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

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)
+      rescues = node.rescues
-      return if (excs = shadowed excs).empty?
 
-      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)
+    private def shadowed(rescues, catch_all = false)
-      previous_exceptions = [] of String
+      traversed_types = Set(String).new
 
-      exceptions.reduce([] of String) do |shadowed, excs|
+      filter_rescues(rescues).each_with_object([] of Crystal::Path) do |types, shadowed|
-        excs = excs.try(&.map(&.to_s)) || %w[Exception]
+        case
-
+        when catch_all
-        if exception_found
+          shadowed.concat(types)
-          shadowed.concat excs
+          next
-          previous_exceptions.concat excs
+        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"
+          nodes = types.select { |tp| traverse(tp.to_s, traversed_types) }
-          excs.each do |exc|
+          shadowed.concat(nodes) unless nodes.empty?
-            if exception_found && exc != "Exception"
-              shadowed << exc
-            else
-              shadowed << exc if previous_exceptions.any? &.== exc
-            end
-            previous_exceptions << exc
-          end
         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

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)