From b023ae0baa838769b91d9f1a0651ecf9d3090fe6 Mon Sep 17 00:00:00 2001 From: Hugo Abonizio Date: Mon, 27 Nov 2017 11:35:15 -0200 Subject: [PATCH] Disallows `while true` (#22) * Disallows `while true` * Add syntax highlighting in documentation * Replace `while true` occurrencies * Add WhileTrue rule to config sample --- config/ameba.yml | 4 +++ spec/ameba/rule/while_true_spec.cr | 42 +++++++++++++++++++++++++++++ src/ameba/ast/traverse.cr | 1 + src/ameba/rule/redundant_begin.cr | 2 +- src/ameba/rule/while_true.cr | 43 ++++++++++++++++++++++++++++++ src/ameba/tokenizer.cr | 6 ++--- 6 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 spec/ameba/rule/while_true_spec.cr create mode 100644 src/ameba/rule/while_true.cr diff --git a/config/ameba.yml b/config/ameba.yml index 657eb62f..92133d51 100644 --- a/config/ameba.yml +++ b/config/ameba.yml @@ -88,3 +88,7 @@ UnlessElse: VariableNames: # Enforces variable names to be in the underscored case. Enabled: true + +WhileTrue: + # Disallows `while` statements with a `true` literal as condition + Enabled: true \ No newline at end of file diff --git a/spec/ameba/rule/while_true_spec.cr b/spec/ameba/rule/while_true_spec.cr new file mode 100644 index 00000000..b23b2503 --- /dev/null +++ b/spec/ameba/rule/while_true_spec.cr @@ -0,0 +1,42 @@ +require "../../spec_helper" + +valid_source = <<-EOF +a = 1 +loop do + a += 1 + break if a > 5 +end +EOF + +invalid_source = <<-EOF +a = 1 +while true + a += 1 + break if a > 5 +end +EOF + +module Ameba::Rule + subject = WhileTrue.new + + describe WhileTrue do + it "passes if there is no `while true`" do + source = Source.new valid_source + subject.catch(source).should be_valid + end + + it "fails if there is `while true`" do + source = Source.new invalid_source + subject.catch(source).should_not be_valid + end + + it "reports rule, pos and message" do + source = Source.new invalid_source, "source.cr" + subject.catch(source).should_not be_valid + + error = source.errors.first + error.location.to_s.should eq "source.cr:2:1" + error.message.should eq "While statement using true literal as condition" + end + end +end diff --git a/src/ameba/ast/traverse.cr b/src/ameba/ast/traverse.cr index 1332a1b1..2ee803da 100644 --- a/src/ameba/ast/traverse.cr +++ b/src/ameba/ast/traverse.cr @@ -23,6 +23,7 @@ module Ameba::AST StringInterpolation, Unless, Var, + While, ] # An AST Visitor used by rules. diff --git a/src/ameba/rule/redundant_begin.cr b/src/ameba/rule/redundant_begin.cr index 4833f9f0..f47c3bc5 100644 --- a/src/ameba/rule/redundant_begin.cr +++ b/src/ameba/rule/redundant_begin.cr @@ -102,7 +102,7 @@ module Ameba::Rule private def def_redundant_begin?(code) lexer = Crystal::Lexer.new code in_body? = in_argument_list? = false - while true + loop do token = lexer.next_token case token.type diff --git a/src/ameba/rule/while_true.cr b/src/ameba/rule/while_true.cr new file mode 100644 index 00000000..0ecc724e --- /dev/null +++ b/src/ameba/rule/while_true.cr @@ -0,0 +1,43 @@ +module Ameba::Rule + # A rule that disallows the use of `while true` instead of using the idiomatic `loop` + # + # For example, this is considered invalid: + # + # ``` + # while true + # do_something + # break if some_condition + # end + # ``` + # + # And should be replaced by the following: + # + # ``` + # loop do + # do_something + # break if some_condition + # end + # ``` + # + # YAML configuration example: + # + # ``` + # WhileTrue: + # Enabled: true + # ``` + # + struct WhileTrue < Base + properties do + description = "Disallows while statements with a true literal as condition" + end + + def test(source) + AST::Visitor.new self, source + end + + def test(source, node : Crystal::While) + return unless node.cond.true_literal? + source.error self, node.location, "While statement using true literal as condition" + end + end +end diff --git a/src/ameba/tokenizer.cr b/src/ameba/tokenizer.cr index deefb111..445c64ee 100644 --- a/src/ameba/tokenizer.cr +++ b/src/ameba/tokenizer.cr @@ -45,7 +45,7 @@ module Ameba private def run_normal_state(lexer, break_on_rcurly = false, &block : Crystal::Token -> _) - while true + loop do token = @lexer.next_token block.call token @@ -63,7 +63,7 @@ module Ameba end private def run_delimiter_state(lexer, token, &block : Crystal::Token -> _) - while true + loop do token = @lexer.next_string_token(token.delimiter_state) block.call token @@ -79,7 +79,7 @@ module Ameba end private def run_array_state(lexer, token, &block : Crystal::Token -> _) - while true + loop do lexer.next_string_array_token block.call token