diff --git a/Gemfile b/Gemfile new file mode 100644 index 0000000..71f442c --- /dev/null +++ b/Gemfile @@ -0,0 +1,5 @@ +source 'https://rubygems.org' + +gem "rspec", "~> 3.8.0" +gem "rubocop" +gem "rubocop-rspec" \ No newline at end of file diff --git a/Gemfile.lock b/Gemfile.lock new file mode 100644 index 0000000..5d882f6 --- /dev/null +++ b/Gemfile.lock @@ -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 diff --git a/lib/cops/raise_message_cop.rb b/lib/cops/raise_message_cop.rb new file mode 100644 index 0000000..80fe5fa --- /dev/null +++ b/lib/cops/raise_message_cop.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +# This cop checks if there is any "raise" code without a message/argument. +# Examples: +# Good +# `raise "Message"` +# `raise StandardError, 'message'` +# `raise StandardError` +# +# Bad +# `raise` +# `raise unless status #inline conditions with raise` + +module RuboCop + module Cop + module CustomCops + class RaiseMessageCop < Base + # We use #on_new_investigation instead of #initialize because #initialize is called for each class/const node + # whereas #on_new_investigation is called at the beginning of each file and we want to maintain + # these variables/arrays until the entire file is checked. + # + # @candidates - Array to store all the possible empty nodes where an offense might occur. + # @rescnodes - Array to store all the rescue nodes having an empty raise as a child. + # @finalnode - Array to store the final nodes where offenses needed to be added. + def on_new_investigation() + super + @candidates=[] + @rescnodes=[] + @finalnodes=[] + end + + # This pattern matcher is used to get all the occurences of empty raises. + def_node_matcher :empty_raise?, <<~PATTERN + (send nil? :raise) + PATTERN + # This pattern matcher is used to get all the rescue nodes having an empty raise descendant. + def_node_matcher :rescue_raises?, <<~PATTERN + (resbody ... `(send nil? :raise)) + PATTERN + + # This is used to pickup rescue nodes wrapping empty raises. + # The #rescue_raises? matcher will pickup only rescue nodes having empty raises and add them to @rescnodes + def on_resbody(node) + if rescue_raises?(node) + puts(node) + @rescnodes << node + end + end + + # This is used to pickup all the instances of empty raises. + # The #empty_raise? matcher will pickup all the empty raises and add them to @candidates. + def on_send(node) + if empty_raise?(node) + @candidates << node + end + end + + # We use #on_investigation_end here to add offenses after parsing the whole file. + def on_investigation_end + @candidates.each do |node| + flag = true + @rescnodes.each do |rescnode| + if rescnode.source_range.contains?(node.source_range) + flag = false + break + end + end + @finalnodes << node if flag + end + @finalnodes.each do |f| + add_offense(f,message:"Raise should be accompanied by a message/argument.") + end + end + end + end + end +end diff --git a/lib/cops/readme.md b/lib/cops/readme.md index f902c45..5acb921 100644 --- a/lib/cops/readme.md +++ b/lib/cops/readme.md @@ -14,6 +14,23 @@ PutLoggerFormatCop ensures that the message format received on encountering **pu ~~~ruby "#method_name:". ~~~ + +~~~ 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: @@ -57,3 +74,42 @@ 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 reason for the exception will be unkown. + +~~~ ruby +# not allowed, raise without a message or argument. +raise +~~~ + +~~~ruby +# not allowed, raise with an inline condition. +raise unless status +~~~ + +~~~ruby +# allowed +raise StandardError,'message' unless status +~~~ + +~~~ 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 +~~~ \ No newline at end of file diff --git a/rubocop.yml b/rubocop.yml index d97489f..689135d 100644 --- a/rubocop.yml +++ b/rubocop.yml @@ -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 diff --git a/spec/raise_message_cop_spec.rb b/spec/raise_message_cop_spec.rb new file mode 100644 index 0000000..d5a215a --- /dev/null +++ b/spec/raise_message_cop_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../lib/cops/raise_message_cop' +RSpec.describe RuboCop::Cop::CustomCops::RaiseMessageCop do + context 'raise message cop' do + 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' + 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 + it 'it should not register an offense if raise is wrapped by a rescue block' do + expect_no_offenses(<<~RUBY) + module D + class A + def B + begin + this_will_fail! + rescue Failure => error + log.error error.message + raise + end + end + end + end + RUBY + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 0000000..026bf49 --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'rubocop' +require 'rubocop/rspec/support' +require 'rspec' + +RSpec.configure do |config| + config.include RuboCop::RSpec::ExpectOffense + config.before(:suite) do + RuboCop::Cop::Registry.global.freeze + # This ensures that there are no side effects from running a particular spec. + # Use `:restore_registry` / `RuboCop::Cop::Registry.with_temporary_global` if + # need to modify registry (e.g. with `stub_cop_class`). + end + config.after(:suite) { RuboCop::Cop::Registry.reset! } +end