diff --git a/spec/ameba/ast/util_spec.cr b/spec/ameba/ast/util_spec.cr index d2f63de8..17f5649b 100644 --- a/spec/ameba/ast/util_spec.cr +++ b/spec/ameba/ast/util_spec.cr @@ -44,7 +44,7 @@ module Ameba::AST ) node = Crystal::Parser.new(s).parse source = subject.node_source node, s.split("\n") - source.should eq ["a = 1"] + source.should eq "a = 1" end it "returns original source of multiline node" do @@ -55,11 +55,11 @@ module Ameba::AST ) node = Crystal::Parser.new(s).parse source = subject.node_source node, s.split("\n") - source.should eq([ - "if ()", - " :ok", - " end", - ]) + source.should eq <<-CRYSTAL + if () + :ok + end + CRYSTAL end it "does not report source of node which has incorrect location" do @@ -81,7 +81,7 @@ module Ameba::AST if SemanticVersion.parse(Crystal::VERSION) <= SemanticVersion.parse("0.35.1") source.should be_nil else - source.should eq %w(nil) + source.should eq "nil" end end end diff --git a/spec/ameba/rule/lint/ambiguous_assignment_spec.cr b/spec/ameba/rule/lint/ambiguous_assignment_spec.cr new file mode 100644 index 00000000..153f1796 --- /dev/null +++ b/spec/ameba/rule/lint/ambiguous_assignment_spec.cr @@ -0,0 +1,169 @@ +require "../../../spec_helper" + +module Ameba::Rule::Lint + describe AmbiguousAssignment do + subject = AmbiguousAssignment.new + + context "when using `-`" do + it "registers an offense with `x`" do + source = Source.new("x =- y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?" + issue.location.to_s.should eq "source.cr:1:3" + issue.end_location.to_s.should eq "source.cr:1:4" + end + + it "registers an offense with `@x`" do + source = Source.new("@x =- y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?" + issue.location.to_s.should eq "source.cr:1:4" + issue.end_location.to_s.should eq "source.cr:1:5" + end + + it "registers an offense with `@@x`" do + source = Source.new("@@x =- y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?" + issue.location.to_s.should eq "source.cr:1:5" + issue.end_location.to_s.should eq "source.cr:1:6" + end + + it "registers an offense with `X`" do + source = Source.new("X =- y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?" + issue.location.to_s.should eq "source.cr:1:3" + issue.end_location.to_s.should eq "source.cr:1:4" + end + + it "does not register an offense when no mistype assignments" do + subject.catch(Source.new(<<-CRYSTAL)).should be_valid + x = 1 + x -= y + x = -y + CRYSTAL + end + end + + context "when using `+`" do + it "registers an offense with `x`" do + source = Source.new("x =+ y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?" + issue.location.to_s.should eq "source.cr:1:3" + issue.end_location.to_s.should eq "source.cr:1:4" + end + + it "registers an offense with `@x`" do + source = Source.new("@x =+ y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?" + issue.location.to_s.should eq "source.cr:1:4" + issue.end_location.to_s.should eq "source.cr:1:5" + end + + it "registers an offense with `@@x`" do + source = Source.new("@@x =+ y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?" + issue.location.to_s.should eq "source.cr:1:5" + issue.end_location.to_s.should eq "source.cr:1:6" + end + + it "registers an offense with `X`" do + source = Source.new("X =+ y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?" + issue.location.to_s.should eq "source.cr:1:3" + issue.end_location.to_s.should eq "source.cr:1:4" + end + + it "does not register an offense when no mistype assignments" do + subject.catch(Source.new(<<-CRYSTAL)).should be_valid + x = 1 + x += y + x = +y + CRYSTAL + end + end + + context "when using `!`" do + it "registers an offense with `x`" do + source = Source.new("x =! y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?" + issue.location.to_s.should eq "source.cr:1:3" + issue.end_location.to_s.should eq "source.cr:1:4" + end + + it "registers an offense with `@x`" do + source = Source.new("@x =! y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?" + issue.location.to_s.should eq "source.cr:1:4" + issue.end_location.to_s.should eq "source.cr:1:5" + end + + it "registers an offense with `@@x`" do + source = Source.new("@@x =! y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?" + issue.location.to_s.should eq "source.cr:1:5" + issue.end_location.to_s.should eq "source.cr:1:6" + end + + it "registers an offense with `X`" do + source = Source.new("X =! y", "source.cr") + subject.catch(source).should_not be_valid + source.issues.size.should eq 1 + + issue = source.issues.first + issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?" + issue.location.to_s.should eq "source.cr:1:3" + issue.end_location.to_s.should eq "source.cr:1:4" + end + + it "does not register an offense when no mistype assignments" do + subject.catch(Source.new(<<-CRYSTAL)).should be_valid + x = false + x != y + x = !y + CRYSTAL + end + end + end +end diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr index 0690e31b..5b87608a 100644 --- a/src/ameba/ast/util.cr +++ b/src/ameba/ast/util.cr @@ -35,6 +35,11 @@ module Ameba::AST::Util return unless loc && end_loc + source_between(loc, end_loc, code_lines) + end + + # Returns the source code from *loc* to *end_loc* (inclusive). + def source_between(loc, end_loc, code_lines) : String? line, column = loc.line_number - 1, loc.column_number - 1 end_line, end_column = end_loc.line_number - 1, end_loc.column_number - 1 node_lines = code_lines[line..end_line] @@ -53,7 +58,7 @@ module Ameba::AST::Util return if last_line.size < end_column + 1 node_lines[-1] = last_line.sub(end_column + 1...last_line.size, "") - node_lines + node_lines.join('\n') end # Returns true if node is a flow command, false - otherwise. diff --git a/src/ameba/rule/lint/ambiguous_assignment.cr b/src/ameba/rule/lint/ambiguous_assignment.cr new file mode 100644 index 00000000..ad7cc9c5 --- /dev/null +++ b/src/ameba/rule/lint/ambiguous_assignment.cr @@ -0,0 +1,50 @@ +module Ameba::Rule::Lint + # This rule checks for mistyped shorthand assignments. + # + # # bad + # x =- y + # x =+ y + # x =! y + # + # # good + # x -= y # or x = -y + # x += y # or x = +y + # x != y # or x = !y + # + # YAML configuration example: + # + # ``` + # Lint/AmbiguousAssignment: + # Enabled: true + # ``` + class AmbiguousAssignment < Base + include AST::Util + + properties do + description "Disallows ambiguous `=-/=+/=!`" + end + + MSG = "Suspicious assignment detected. Did you mean `%s`?" + + MISTAKES = { + "=-" => "-=", + "=+" => "+=", + "=!" => "!=", + } + + def test(source, node : Crystal::Assign) + return unless op_end_location = node.value.location + + op_location = Crystal::Location.new( + op_end_location.filename, + op_end_location.line_number, + op_end_location.column_number - 1 + ) + op_text = source_between(op_location, op_end_location, source.lines) + return unless op_text + return unless MISTAKES.has_key?(op_text) + + issue_for op_location, op_end_location, MSG % MISTAKES[op_text] + end + end +end diff --git a/src/ameba/rule/lint/empty_expression.cr b/src/ameba/rule/lint/empty_expression.cr index 89ca5fdc..d237d7c0 100644 --- a/src/ameba/rule/lint/empty_expression.cr +++ b/src/ameba/rule/lint/empty_expression.cr @@ -39,7 +39,7 @@ module Ameba::Rule::Lint MSG_EXRS = "Avoid empty expressions" def test(source, node : Crystal::NilLiteral) - exp = node_source(node, source.lines).try &.join + exp = node_source(node, source.lines) return if exp.in?(nil, "nil") issue_for node, MSG % exp diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr index 3f3009f4..bfbf3813 100644 --- a/src/ameba/rule/style/redundant_begin.cr +++ b/src/ameba/rule/style/redundant_begin.cr @@ -84,7 +84,7 @@ module Ameba::Rule::Style private def redundant_begin_in_handler?(source, handler, node) return false if begin_exprs_in_handler?(handler) || inner_handler?(handler) - code = node_source(node, source.lines).try &.join('\n') + code = node_source(node, source.lines) def_redundant_begin? code if code rescue false