Track issue.end_location properly

2024-08-15 00:53:29 +00:00 · 2018-11-24 19:38:13 +02:00 · 2018-11-24 19:38:13 +02:00 · 9885457227
commit 9885457227
parent ad2c6bad0e
45 changed files with 90 additions and 20 deletions
--- a/spec/ameba/ast/branch_spec.cr
+++ b/spec/ameba/ast/branch_spec.cr
@ -323,7 +323,7 @@ module Ameba::AST
        branch.to_s.should eq branch.node.to_s
      end

-      it "delegates location to node" do
+      it "delegates locations to node" do
        nodes = as_nodes %(
          if true
            a = 2
@ -332,6 +332,7 @@ module Ameba::AST
        branchable = Branchable.new nodes.if_nodes.first
        branch = Branch.new nodes.assign_nodes.first, branchable
        branch.location.should eq branch.node.location
+        branch.end_location.should eq branch.node.end_location
      end
    end

--- a/spec/ameba/ast/branchable_spec.cr
+++ b/spec/ameba/ast/branchable_spec.cr
@ -16,10 +16,11 @@ module Ameba::AST
        branchable.to_s.should eq node.to_s
      end

-      it "delegates location to @node" do
+      it "delegates locations to @node" do
        node = as_node %(a = 2 if true)
        branchable = Branchable.new node
        branchable.location.should eq node.location
+        branchable.end_location.should eq node.end_location
      end
    end

--- a/spec/ameba/ast/flow_expression_spec.cr
+++ b/spec/ameba/ast/flow_expression_spec.cr
@ -17,10 +17,11 @@ module Ameba::AST
          flow_expression.to_s.should eq node.to_s
        end

-        it "delegates location to @node" do
+        it "delegates locations to @node" do
          node = as_node %(break if true)
          flow_expression = FlowExpression.new node, false
          flow_expression.location.should eq node.location
+          flow_expression.end_location.should eq node.end_location
        end
      end

--- a/spec/ameba/ast/scope_spec.cr
+++ b/spec/ameba/ast/scope_spec.cr
@ -18,6 +18,21 @@ module Ameba::AST
    end
  end

+  describe "delegation" do
+    it "delegates to_s to node" do
+      node = as_node("def foo; end")
+      scope = Scope.new node
+      scope.to_s.should eq node.to_s
+    end
+
+    it "delegates locations to node" do
+      node = as_node("def foo; end")
+      scope = Scope.new node
+      scope.location.should eq node.location
+      scope.end_location.should eq node.end_location
+    end
+  end
+
  describe "#add_variable" do
    it "adds a new variable to the scope" do
      scope = Scope.new as_node("")
--- a/spec/ameba/ast/variabling/argument_spec.cr
+++ b/spec/ameba/ast/variabling/argument_spec.cr
@ -14,9 +14,10 @@ module Ameba::AST
    end

    describe "delegation" do
-      it "delegates location to node" do
+      it "delegates locations to node" do
        argument = Argument.new(arg, variable)
        argument.location.should eq arg.location
+        argument.end_location.should eq arg.end_location
      end

      it "delegates to_s to node" do
--- a/spec/ameba/ast/variabling/assignment_spec.cr
+++ b/spec/ameba/ast/variabling/assignment_spec.cr
@ -22,9 +22,10 @@ module Ameba::AST
    end

    describe "delegation" do
-      it "delegates location" do
+      it "delegates locations" do
        assignment = Assignment.new(node, variable)
        assignment.location.should eq node.location
+        assignment.end_location.should eq node.end_location
      end

      it "delegates to_s" do
--- a/spec/ameba/ast/variabling/variable_spec.cr
+++ b/spec/ameba/ast/variabling/variable_spec.cr
@ -13,9 +13,10 @@ module Ameba::AST
    end

    describe "delegation" do
-      it "delegates location" do
+      it "delegates locations" do
        variable = Variable.new(var_node, scope)
        variable.location.should eq var_node.location
+        variable.end_location.should eq var_node.end_location
      end

      it "delegates name" do
--- a/spec/ameba/rule/layout/line_length_spec.cr
+++ b/spec/ameba/rule/layout/line_length_spec.cr
@ -27,6 +27,7 @@ module Ameba::Rule::Layout
      issue = source.issues.first
      issue.rule.should eq subject
      issue.location.to_s.should eq "source.cr:1:#{subject.max_length + 1}"
+      issue.end_location.should be_nil
      issue.message.should eq "Line too long"
    end

--- a/spec/ameba/rule/layout/trailing_blank_lines_spec.cr
+++ b/spec/ameba/rule/layout/trailing_blank_lines_spec.cr
@ -31,6 +31,7 @@ module Ameba::Rule::Layout
      issue = source.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:3:1"
+      issue.end_location.should be_nil
      issue.message.should eq "Blank lines detected at the end of the file"
    end
  end
--- a/spec/ameba/rule/layout/trailing_whitespace_spec.cr
+++ b/spec/ameba/rule/layout/trailing_whitespace_spec.cr
@ -21,6 +21,7 @@ module Ameba::Rule::Layout
      issue = source.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:2:7"
+      issue.end_location.should be_nil
      issue.message.should eq "Trailing whitespace detected"
    end
  end
--- a/spec/ameba/rule/lint/comparison_to_boolean_spec.cr
+++ b/spec/ameba/rule/lint/comparison_to_boolean_spec.cr
@ -106,6 +106,7 @@ module Ameba::Rule::Lint
        issue = source.issues.first
        issue.rule.should_not be_nil
        issue.location.to_s.should eq "source.cr:1:1"
+        issue.end_location.to_s.should eq "source.cr:1:9"
        issue.message.should eq "Comparison to a boolean is pointless"
      end
    end
--- a/spec/ameba/rule/lint/debugger_statement_spec.cr
+++ b/spec/ameba/rule/lint/debugger_statement_spec.cr
@ -38,6 +38,7 @@ module Ameba::Rule::Lint
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:1:8"
      issue.message.should eq "Possible forgotten debugger statement detected"
    end
  end
--- a/spec/ameba/rule/lint/empty_ensure_spec.cr
+++ b/spec/ameba/rule/lint/empty_ensure_spec.cr
@ -62,6 +62,7 @@ module Ameba::Rule::Lint

      issue.rule.should_not be_nil
      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
  end
--- a/spec/ameba/rule/lint/empty_expression_spec.cr
+++ b/spec/ameba/rule/lint/empty_expression_spec.cr
@ -104,6 +104,7 @@ module Ameba
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:4"
+      issue.end_location.to_s.should eq "source.cr:1:5"
      issue.message.should eq "Avoid empty expressions"
    end
  end
--- a/spec/ameba/rule/lint/hash_duplicated_key_spec.cr
+++ b/spec/ameba/rule/lint/hash_duplicated_key_spec.cr
@ -35,6 +35,7 @@ module Ameba::Rule::Lint
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:5"
+      issue.end_location.to_s.should eq "source.cr:1:24"
      issue.message.should eq %(Duplicated keys in hash literal: "a")
    end

--- a/spec/ameba/rule/lint/literal_in_condition_spec.cr
+++ b/spec/ameba/rule/lint/literal_in_condition_spec.cr
@ -66,6 +66,7 @@ module Ameba::Rule::Lint
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:1:20"
      issue.message.should eq "Literal value found in conditional"
    end
  end
--- a/spec/ameba/rule/lint/literal_in_interpolation_spec.cr
+++ b/spec/ameba/rule/lint/literal_in_interpolation_spec.cr
@ -35,6 +35,7 @@ module Ameba::Rule::Lint
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:1:6"
      issue.message.should eq "Literal value found in interpolation"
    end
  end
--- a/spec/ameba/rule/lint/percent_arrays_spec.cr
+++ b/spec/ameba/rule/lint/percent_arrays_spec.cr
@ -61,6 +61,7 @@ module Ameba::Rule::Lint
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.should be_nil
      issue.message.should eq(
        "Symbols `,\"` may be unwanted in %w array literals"
      )
--- a/spec/ameba/rule/lint/rand_zero_spec.cr
+++ b/spec/ameba/rule/lint/rand_zero_spec.cr
@ -30,6 +30,7 @@ module Ameba::Rule::Lint

      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:1:7"
      issue.message.should eq "rand(1) always returns 0"
    end
  end
--- a/spec/ameba/rule/lint/shadowed_argument_spec.cr
+++ b/spec/ameba/rule/lint/shadowed_argument_spec.cr
@ -159,6 +159,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.end_location.to_s.should eq "source.cr:2:10"
      issue.message.should eq "Argument `bar` is assigned before it is used"
    end
  end
--- a/spec/ameba/rule/lint/shadowed_exception_spec.cr
+++ b/spec/ameba/rule/lint/shadowed_exception_spec.cr
@ -167,6 +167,7 @@ module Ameba::Rule::Lint

      issue.rule.should_not be_nil
      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"
      )
--- a/spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr
+++ b/spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr
@ -161,6 +161,7 @@ module Ameba::Rule::Lint
      issue = source.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:2:12"
+      issue.end_location.should be_nil
      issue.message.should eq "Shadowing outer local variable `foo`"
    end
  end
--- a/spec/ameba/rule/lint/unreachable_code_spec.cr
+++ b/spec/ameba/rule/lint/unreachable_code_spec.cr
@ -682,5 +682,20 @@ module Ameba::Rule::Lint
        subject.catch(s).should be_valid
      end
    end
+
+    it "reports message, rule, location" do
+      s = Source.new %(
+        return
+        :unreachable
+      ), "source.cr"
+
+      subject.catch(s).should_not be_valid
+
+      issue = s.issues.first
+      issue.rule.should_not be_nil
+      issue.location.to_s.should eq "source.cr:2:1"
+      issue.end_location.to_s.should eq "source.cr:2:12"
+      issue.message.should eq "Unreachable code detected"
+    end
  end
 end
--- a/spec/ameba/rule/lint/useless_condition_in_when_spec.cr
+++ b/spec/ameba/rule/lint/useless_condition_in_when_spec.cr
@ -39,6 +39,7 @@ module Ameba::Rule::Lint
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:5:15"
+      issue.end_location.to_s.should eq "source.cr:5:27"
      issue.message.should eq "Useless condition in when detected"
    end
  end
--- a/spec/ameba/rule/style/constant_names_spec.cr
+++ b/spec/ameba/rule/style/constant_names_spec.cr
@ -43,6 +43,7 @@ module Ameba
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:1:9"
      issue.message.should eq(
        "Constant name should be screaming-cased: CONST, not Const"
      )
--- a/spec/ameba/rule/style/large_numbers_spec.cr
+++ b/spec/ameba/rule/style/large_numbers_spec.cr
@ -118,6 +118,7 @@ module Ameba
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.should be_nil
      issue.message.should match /1_200_000/
    end

--- a/spec/ameba/rule/style/method_names_spec.cr
+++ b/spec/ameba/rule/style/method_names_spec.cr
@ -47,6 +47,7 @@ module Ameba
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:2:3"
      issue.message.should eq(
        "Method name should be underscore-cased: bad_name, not bad_Name"
      )
--- a/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr
+++ b/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr
@ -62,6 +62,7 @@ module Ameba::Rule::Style
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:1:21"
      issue.message.should eq "Avoid negated conditions in unless blocks"
    end
  end
--- a/spec/ameba/rule/style/predicate_name_spec.cr
+++ b/spec/ameba/rule/style/predicate_name_spec.cr
@ -41,6 +41,7 @@ module Ameba::Rule::Style
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:2:3"
+      issue.end_location.to_s.should eq "source.cr:4:5"
      issue.message.should eq(
        "Favour method name 'picture?' over 'has_picture?'")
    end
--- a/spec/ameba/rule/style/redundant_begin_spec.cr
+++ b/spec/ameba/rule/style/redundant_begin_spec.cr
@ -220,6 +220,7 @@ module Ameba::Rule::Style
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:7:3"
      issue.message.should eq "Redundant `begin` block detected"
    end
  end
--- a/spec/ameba/rule/style/type_names_spec.cr
+++ b/spec/ameba/rule/style/type_names_spec.cr
@ -52,6 +52,7 @@ module Ameba
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:2:3"
      issue.message.should eq(
        "Type name should be camelcased: MyClass, but it was My_class"
      )
--- a/spec/ameba/rule/style/unless_else_spec.cr
+++ b/spec/ameba/rule/style/unless_else_spec.cr
@ -38,6 +38,7 @@ module Ameba::Rule::Style
      issue.should_not be_nil
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:5:3"
      issue.message.should eq "Favour if over unless with else"
    end
  end
--- a/spec/ameba/rule/style/variable_names_spec.cr
+++ b/spec/ameba/rule/style/variable_names_spec.cr
@ -53,6 +53,7 @@ module Ameba
      issue = s.issues.first
      issue.rule.should_not be_nil
      issue.location.to_s.should eq "source.cr:1:1"
+      issue.end_location.to_s.should eq "source.cr:1:7"
      issue.message.should eq(
        "Var name should be underscore-cased: bad_name, not badName"
      )
--- a/spec/ameba/rule/style/while_true_spec.cr
+++ b/spec/ameba/rule/style/while_true_spec.cr
@ -36,6 +36,7 @@ module Ameba::Rule::Style

      issue = source.issues.first
      issue.location.to_s.should eq "source.cr:2:1"
+      issue.end_location.to_s.should eq "source.cr:5:3"
      issue.message.should eq "While statement using true literal as condition"
    end
  end
--- a/src/ameba/ast/branch.cr
+++ b/src/ameba/ast/branch.cr
@ -21,6 +21,7 @@ module Ameba::AST

    delegate to_s, to: @node
    delegate location, to: @node
+    delegate end_location, to: @node

    def_equals_and_hash node, location

--- a/src/ameba/ast/branchable.cr
+++ b/src/ameba/ast/branchable.cr
@ -25,6 +25,7 @@ module Ameba::AST

    delegate to_s, to: @node
    delegate location, to: @node
+    delegate end_location, to: @node

    # Creates a new branchable
    #
--- a/src/ameba/ast/flow_expression.cr
+++ b/src/ameba/ast/flow_expression.cr
@ -26,6 +26,7 @@ module Ameba::AST

    delegate to_s, to: @node
    delegate location, to: @node
+    delegate end_location, to: @node

    # Creates a new flow expression.
    #
--- a/src/ameba/ast/scope.cr
+++ b/src/ameba/ast/scope.cr
@ -21,6 +21,7 @@ module Ameba::AST

    delegate to_s, to: node
    delegate location, to: node
+    delegate end_location, to: node

    def_equals_and_hash node, location

@ -44,7 +45,7 @@ module Ameba::AST
    end

    def add_argument(node)
-      add_variable Crystal::Var.new(node.name).at(node.location)
+      add_variable Crystal::Var.new(node.name).at(node)
      arguments << Argument.new(node, variables.last)
    end

--- a/src/ameba/ast/variabling/argument.cr
+++ b/src/ameba/ast/variabling/argument.cr
@ -20,6 +20,7 @@ module Ameba::AST
    getter variable : Variable

    delegate location, to: @node
+    delegate end_location, to: @node
    delegate to_s, to: @node

    # Creates a new argument.
--- a/src/ameba/ast/variabling/assignment.cr
+++ b/src/ameba/ast/variabling/assignment.cr
@ -17,6 +17,8 @@ module Ameba::AST
    getter branch : Branch?

    delegate to_s, to: @node
+    delegate location, to: @node
+    delegate end_location, to: @node
    delegate scope, to: @variable

    # Creates a new assignment.
@ -58,18 +60,18 @@ module Ameba::AST
      !branch.nil?
    end

-    # Returns the location of the current variable in the assignment.
-    def location
+    # Returns the target node of the variable in this assignment.
+    def target_node
      case assign = node
-      when Crystal::Assign           then assign.target.location
-      when Crystal::OpAssign         then assign.target.location
-      when Crystal::UninitializedVar then assign.var.location
+      when Crystal::Assign           then assign.target
+      when Crystal::OpAssign         then assign.target
+      when Crystal::UninitializedVar then assign.var
      when Crystal::MultiAssign
-        assign.targets.find do |target|
+        assign.targets.find(node) do |target|
          target.is_a?(Crystal::Var) && target.name == variable.name
-        end.try &.location
+        end
      else
-        node.location
+        node
      end
    end
  end
--- a/src/ameba/ast/variabling/variable.cr
+++ b/src/ameba/ast/variabling/variable.cr
@ -18,6 +18,7 @@ module Ameba::AST
    getter assign_before_reference : Crystal::ASTNode?

    delegate location, to: @node
+    delegate end_location, to: @node
    delegate name, to: @node
    delegate to_s, to: @node

--- a/src/ameba/reportable.cr
+++ b/src/ameba/reportable.cr
@ -11,12 +11,12 @@ module Ameba
    end

    # Adds a new issue for AST *node*.
-    def add_issue(rule, node, message, **args)
+    def add_issue(rule, node : Crystal::ASTNode, message, **args)
      add_issue rule, node.location, node.end_location, message, **args
    end

    # Adds a new issue for Crystal *token*.
-    def add_issue(rule, token, message, **args)
+    def add_issue(rule, token : Crystal::Token, message, **args)
      add_issue rule, token.location, nil, message, **args
    end

--- a/src/ameba/rule/lint/shadowing_local_outer_var.cr
+++ b/src/ameba/rule/lint/shadowing_local_outer_var.cr
@ -55,7 +55,7 @@ module Ameba::Rule::Lint
      scope.arguments.each do |arg|
        next if arg.ignored?
        if scope.outer_scope.try &.find_variable(arg.name)
-          issue_for arg, MSG % arg.name
+          issue_for arg.node, MSG % arg.name
        end
      end
    end
--- a/src/ameba/rule/lint/unused_argument.cr
+++ b/src/ameba/rule/lint/unused_argument.cr
@ -58,7 +58,7 @@ module Ameba::Rule::Lint
        next if argument.ignored? || scope.references?(argument.variable)

        name_suggestion = scope.node.is_a?(Crystal::Block) ? '_' : "_#{argument.name}"
-        issue_for argument, MSG % {argument.name, name_suggestion}
+        issue_for argument.node, MSG % {argument.name, name_suggestion}
      end
    end
  end
--- a/src/ameba/rule/lint/useless_assign.cr
+++ b/src/ameba/rule/lint/useless_assign.cr
@ -43,7 +43,7 @@ module Ameba::Rule::Lint

        var.assignments.each do |assign|
          next if assign.referenced?
-          issue_for assign, MSG % var.name
+          issue_for assign.target_node, MSG % var.name
        end
      end
    end