diff --git a/config/ameba.yml b/config/ameba.yml index 3450af70..d98cbab8 100644 --- a/config/ameba.yml +++ b/config/ameba.yml @@ -10,6 +10,10 @@ DebuggerStatement: # Disallows calls to debugger. Enabled: true +EmptyEnsure: + # Disallows empty ensure statement. + Enabled: true + EmptyExpression: # Disallows empty expressions. Enabled: true diff --git a/spec/ameba/rule/empty_ensure_spec.cr b/spec/ameba/rule/empty_ensure_spec.cr new file mode 100644 index 00000000..0d4c4e4b --- /dev/null +++ b/spec/ameba/rule/empty_ensure_spec.cr @@ -0,0 +1,68 @@ +require "../../spec_helper" + +module Ameba::Rule + describe EmptyEnsure do + subject = EmptyEnsure.new + + it "passes if there is no empty ensure blocks" do + s = Source.new %( + def some_method + do_some_stuff + ensure + do_something_else + end + + begin + do_some_stuff + ensure + do_something_else + end + + def method_with_rescue + rescue + ensure + nil + end + ) + subject.catch(s).should be_valid + end + + it "fails if there is an empty ensure in method" do + s = Source.new %( + def method + do_some_stuff + ensure + end + ) + subject.catch(s).should_not be_valid + 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 %( + begin + do_some_stuff + rescue + do_some_other_stuff + ensure + end + ), "source.cr" + subject.catch(s).should_not be_valid + error = s.errors.first + + error.rule.should_not be_nil + error.location.to_s.should eq "source.cr:3:11" + error.message.should eq "Empty `ensure` block detected" + end + end +end diff --git a/src/ameba/ast/traverse.cr b/src/ameba/ast/traverse.cr index ed1a3e8a..ffd4f6c5 100644 --- a/src/ameba/ast/traverse.cr +++ b/src/ameba/ast/traverse.cr @@ -12,6 +12,7 @@ module Ameba::AST ClassVar, Def, EnumDef, + ExceptionHandler, If, InstanceVar, LibDef, diff --git a/src/ameba/rule/empty_ensure.cr b/src/ameba/rule/empty_ensure.cr new file mode 100644 index 00000000..1d2f6822 --- /dev/null +++ b/src/ameba/rule/empty_ensure.cr @@ -0,0 +1,47 @@ +module Ameba::Rule + # A rule that disallows empty ensure statement. + # + # For example, this is considered invalid: + # + # ``` + # def some_method + # do_some_stuff + # ensure + # end + # + # begin + # do_some_stuff + # ensure + # end + # ``` + # + # And it should be written as this: + # + # + # ``` + # def some_method + # do_some_stuff + # ensure + # do_something_else + # end + # + # begin + # do_some_stuff + # ensure + # do_something_else + # end + # ``` + # + struct EmptyEnsure < Base + def test(source) + AST::Visitor.new self, source + end + + def test(source, node : Crystal::ExceptionHandler) + node_ensure = node.ensure + return if node_ensure.nil? || !node_ensure.nop? + + source.error self, node.location, "Empty `ensure` block detected" + end + end +end