Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cop for checking empty raise #10

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
source 'https://rubygems.org'

gem "rspec", "~> 3.8.0"
gem "rubocop"
gem "rubocop-rspec"
52 changes: 52 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
GEM
remote: https://rubygems.org/
specs:
ast (2.4.2)
diff-lcs (1.5.0)
json (2.6.2)
parallel (1.22.1)
parser (3.1.2.0)
ast (~> 2.4.1)
rainbow (3.1.1)
regexp_parser (2.5.0)
rexml (3.2.5)
rspec (3.8.0)
rspec-core (~> 3.8.0)
rspec-expectations (~> 3.8.0)
rspec-mocks (~> 3.8.0)
rspec-core (3.8.2)
rspec-support (~> 3.8.0)
rspec-expectations (3.8.6)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.8.0)
rspec-mocks (3.8.2)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.8.0)
rspec-support (3.8.3)
rubocop (1.32.0)
json (~> 2.3)
parallel (~> 1.10)
parser (>= 3.1.0.0)
rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 1.8, < 3.0)
rexml (>= 3.2.5, < 4.0)
rubocop-ast (>= 1.19.1, < 2.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 1.4.0, < 3.0)
rubocop-ast (1.19.1)
parser (>= 3.1.1.0)
rubocop-rspec (2.12.1)
rubocop (~> 1.31)
ruby-progressbar (1.11.0)
unicode-display_width (2.2.0)

PLATFORMS
x86_64-linux

DEPENDENCIES
rspec (~> 3.8.0)
rubocop
rubocop-rspec

BUNDLED WITH
2.3.7
22 changes: 22 additions & 0 deletions lib/cops/raise_message_cop.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true

# This cop checks if there is any "raise" code without a message
# Examples:
# Good -> raise "Message"
# Good ->
adip1818 marked this conversation as resolved.
Show resolved Hide resolved
module RuboCop
module Cop
module CustomCops
class RaiseMessageCop < Base
def on_send(node)
return unless node.command?(:raise)
# If the raise encountered does not have any arguments, add the offense
# It ensures that raise is always accompanied with an argument.
if node.arguments.size<1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess raise without argument is valid inside an exception handler block. In that case, it will raise the exception which it just caught, can you verify it?

Copy link
Contributor Author

@adip1818 adip1818 Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For exception handler cases, raise will return a "RunTime" error if given without argument by default, if we want to raise an Exception, we need to pass it as an argument.

With no arguments, raises the exception in "$!" or raises a RuntimeError if "$!" is nil.

https://stackoverflow.com/questions/4800698/what-is-the-difference-between-raise-foo-and-raise-exception-newfoo

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking about this case: https://stackoverflow.com/a/23771227

add_offense(node, message: "Raise should be accompanied by a message/argument.")
end
end
end
end
end
end
46 changes: 46 additions & 0 deletions lib/cops/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,23 @@ PutLoggerFormatCop ensures that the message format received on encountering **pu
~~~ruby
"<module/class>#method_name:<space><message>".
~~~

~~~ ruby
# not allowed.
puts("Message here")
~~~

~~~ruby
# allowed, if both class/module name and method name are available.
puts("moduleS#method2: Message here")
~~~

~~~ruby
# allowed, if only method_name is available.
puts("methodS: Message here")
~~~
Disabled by default

Enabling the cop inside .rubocop.yml :
~~~ruby
CustomCop/PutLoggerFormatCop:
Expand Down Expand Up @@ -57,3 +74,32 @@ to disable it add the following lines in .rubocop.yml of your repo:
CustomCop/DuplicateConstantCop:
Enabled: false
~~~

### RaiseMessageCop
RaiseMessageCop ensures that there is no empty occurence of raise, i.e it makes sure that "raise" is always accompanied by an argument/message.

### Reason to add this cop
if we have an occurence of raise without a message or an argument it breaks the flow of the code and the rest of the code becomes unreachable, hence raising "unreachable code" offense on rubocop.
adip1818 marked this conversation as resolved.
Show resolved Hide resolved

~~~ ruby
# not allowed, raise without a message or argument.
raise
~~~

~~~ ruby
# allowed
raise "Error message
~~~

~~~ ruby
# allowed
raise StandardError, 'message'
~~~

Enabled by default

to disable it add the following lines in .rubocop.yml of your repo:
~~~ruby
CustomCop/RaiseMessageCop:
Enabled: false
~~~
6 changes: 5 additions & 1 deletion rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ AllCops:
require:
- './lib/cops/logger_format_cop.rb'
- './lib/cops/duplicate_constant_cop.rb'
- './lib/cops/raise_message_cop.rb'
- 'rubocop-rails'


CustomCops/RaiseMessageCop:
Enabled: true

CustomCops/PutLoggerFormatCop:
Enabled: false

Expand Down
66 changes: 66 additions & 0 deletions spec/raise_message_cop_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
require 'spec_helper'
require_relative '../lib/cops/raise_message_cop.rb'
RSpec.describe RuboCop::Cop::CustomCops::RaiseMessageCop do
context 'corrector' do
adip1818 marked this conversation as resolved.
Show resolved Hide resolved
subject(:cop) { described_class.new }
it 'it should register an offense if there is an empty raise present' do
expect_offense(<<~RUBY)
module D
class A
def B
raise
^^^^^ Raise should be accompanied by a message/argument.
end
end
end
RUBY
end
it 'it should not register an offense if raise is accompanied by an argument/message' do
expect_no_offenses (<<~RUBY)
module D
class A
def B
raise "A message with context to the raised error."
end
end
end

RUBY

expect_no_offenses (<<~RUBY)
module D
class A
def B
raise StandardError, 'message'
adip1818 marked this conversation as resolved.
Show resolved Hide resolved
end
end
end

RUBY
end
it 'it should register an offense for an inline condtion' do
expect_offense(<<~RUBY)
module D
class A
def B
raise unless status
^^^^^ Raise should be accompanied by a message/argument.
end
end
end

RUBY

expect_no_offenses (<<~RUBY)
module D
class A
def B
raise StandardError, 'message' unless status
end
end
end

RUBY
end
end
end
7 changes: 7 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
require 'rubocop'
require 'rubocop/rspec/support'
require 'rspec'

RSpec.configure do |config|
config.include RuboCop::RSpec::ExpectOffense
adip1818 marked this conversation as resolved.
Show resolved Hide resolved
end