From 8d08f59e8e0b74473b09a7a606bf0c597b9c2dfe Mon Sep 17 00:00:00 2001 From: Robert Fletcher Date: Sun, 19 Jan 2020 16:41:38 -0800 Subject: [PATCH] Add NoLet cop Fixes #862 --- CHANGELOG.md | 2 + config/default.yml | 6 ++ lib/rubocop/cop/rspec/no_let.rb | 85 +++++++++++++++++++++++++++ lib/rubocop/cop/rspec_cops.rb | 1 + manual/cops.md | 1 + manual/cops_rspec.md | 71 ++++++++++++++++++++++ spec/rubocop/cop/rspec/no_let_spec.rb | 64 ++++++++++++++++++++ 7 files changed, 230 insertions(+) create mode 100644 lib/rubocop/cop/rspec/no_let.rb create mode 100644 spec/rubocop/cop/rspec/no_let_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index aa827e6c5..557865da1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Fix `RSpec/InstanceVariable` detection inside custom matchers. ([@pirj][]) * Fix `RSpec/ScatteredSetup` to distinguish hooks with different metadata. ([@pirj][]) * Add autocorrect support for `RSpec/ExpectActual` cop. ([@dduugg][], [@pirj][]) +* Add `RSpec/NoLet` cop. ([@mockdeep][]) ## 1.37.1 (2019-12-16) @@ -477,3 +478,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@jfragoulis]: https://github.com/jfragoulis [@ybiquitous]: https://github.com/ybiquitous [@dduugg]: https://github.com/dduugg +[@mockdeep]: https://github.com/mockdeep diff --git a/config/default.yml b/config/default.yml index 25f7cef4f..c4a5447e0 100644 --- a/config/default.yml +++ b/config/default.yml @@ -344,6 +344,12 @@ RSpec/NestedGroups: Max: 3 StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NestedGroups +RSpec/NoLet: + Description: Checks for usage of `let` blocks in specs. + Enabled: false + AllowSubject: false + StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NoLet + RSpec/NotToNot: Description: Checks for consistent method usage for negating expectations. EnforcedStyle: not_to diff --git a/lib/rubocop/cop/rspec/no_let.rb b/lib/rubocop/cop/rspec/no_let.rb new file mode 100644 index 000000000..51677aa28 --- /dev/null +++ b/lib/rubocop/cop/rspec/no_let.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Checks for usage of `let` blocks in specs. + # + # This cop can be configured with the option `AllowSubject` which + # will configure the cop to only register offenses on explicit calls to + # `let` and not calls to `subject` + # + # @example + # # bad + # describe MyClass do + # let(:foo) { [] } + # it { expect(foo).to be_empty } + # end + # + # describe MyClass do + # subject(:foo) { [] } + # it { expect(foo).to be_empty } + # end + # + # # good + # describe MyClass do + # it do + # foo = [] + # expect(foo).to be_empty + # end + # end + # + # @example with AllowSubject configuration + # + # # rubocop.yml + # # RSpec/NoLet: + # # AllowSubject: true + # + # # bad + # describe MyClass do + # let(:foo) { [] } + # it { expect(foo).to be_empty } + # end + # + # # good + # describe MyClass do + # subject(:foo) { [] } + # it { expect(foo).to be_empty } + # end + # + # describe MyClass do + # it do + # foo = [] + # expect(foo).to be_empty + # end + # end + # + class NoLet < Cop + MSG = 'Avoid using `%s` ' \ + '– use a method call or local variable instead.' + + def_node_matcher :let?, <<~PATTERN + (send nil? :let ...) + PATTERN + + def_node_matcher :subject?, <<~PATTERN + (send nil? :subject ...) + PATTERN + + def on_send(node) + if subject?(node) && !allow_subject? + add_offense(node, message: format(MSG, method: 'subject')) + elsif let?(node) + add_offense(node, message: format(MSG, method: 'let')) + end + end + + private + + def allow_subject? + cop_config['AllowSubject'] + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index 309dfc6d8..c77c31d3b 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -66,6 +66,7 @@ require_relative 'rspec/multiple_subjects' require_relative 'rspec/named_subject' require_relative 'rspec/nested_groups' +require_relative 'rspec/no_let' require_relative 'rspec/not_to_not' require_relative 'rspec/overwriting_setup' require_relative 'rspec/pending' diff --git a/manual/cops.md b/manual/cops.md index 67c45ce2e..7633ad877 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -65,6 +65,7 @@ * [RSpec/MultipleSubjects](cops_rspec.md#rspecmultiplesubjects) * [RSpec/NamedSubject](cops_rspec.md#rspecnamedsubject) * [RSpec/NestedGroups](cops_rspec.md#rspecnestedgroups) +* [RSpec/NoLet](cops_rspec.md#rspecnolet) * [RSpec/NotToNot](cops_rspec.md#rspecnottonot) * [RSpec/OverwritingSetup](cops_rspec.md#rspecoverwritingsetup) * [RSpec/Pending](cops_rspec.md#rspecpending) diff --git a/manual/cops_rspec.md b/manual/cops_rspec.md index 1aec29405..3a9267815 100644 --- a/manual/cops_rspec.md +++ b/manual/cops_rspec.md @@ -2263,6 +2263,77 @@ Max | `3` | Integer * [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NestedGroups](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NestedGroups) +## RSpec/NoLet + +Enabled by default | Supports autocorrection +--- | --- +Disabled | No + +Checks for usage of `let` blocks in specs. + +This cop can be configured with the option `AllowSubject` which +will configure the cop to only register offenses on explicit calls to +`let` and not calls to `subject` + +### Examples + +```ruby +# bad +describe MyClass do + let(:foo) { [] } + it { expect(foo).to be_empty } +end + +describe MyClass do + subject(:foo) { [] } + it { expect(foo).to be_empty } +end + +# good +describe MyClass do + it do + foo = [] + expect(foo).to be_empty + end +end +``` +#### with AllowSubject configuration + +```ruby +# rubocop.yml +# RSpec/NoLet: +# AllowSubject: true + +# bad +describe MyClass do + let(:foo) { [] } + it { expect(foo).to be_empty } +end + +# good +describe MyClass do + subject(:foo) { [] } + it { expect(foo).to be_empty } +end + +describe MyClass do + it do + foo = [] + expect(foo).to be_empty + end +end +``` + +### Configurable attributes + +Name | Default value | Configurable values +--- | --- | --- +AllowSubject | `false` | Boolean + +### References + +* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NoLet](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/NoLet) + ## RSpec/NotToNot Enabled by default | Supports autocorrection diff --git a/spec/rubocop/cop/rspec/no_let_spec.rb b/spec/rubocop/cop/rspec/no_let_spec.rb new file mode 100644 index 000000000..66cefeb48 --- /dev/null +++ b/spec/rubocop/cop/rspec/no_let_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::NoLet do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new } + + # TODO: Write test code + # + # For example + it 'flags an offense when using `#let`' do + expect_offense(<<~RUBY) + let(:foo) { Foo.new } + ^^^^^^^^^ Avoid using `let` – use a method call or local variable instead. + RUBY + end + + it 'flags an offense when using `#subject` without name' do + expect_offense(<<~RUBY) + subject { Foo.new } + ^^^^^^^ Avoid using `subject` – use a method call or local variable instead. + RUBY + end + + it 'flags an offense when using `#subject` with name' do + expect_offense(<<~RUBY) + subject(:foo) { Foo.new } + ^^^^^^^^^^^^^ Avoid using `subject` – use a method call or local variable instead. + RUBY + end + + it 'does not flag an offense when using `#before`' do + expect_no_offenses(<<~RUBY) + before { foo } + RUBY + end + + context 'when using AllowSubject configuration', :config do + subject(:cop) { described_class.new(config) } + + let(:cop_config) do + { 'AllowSubject' => true } + end + + it 'flags an offense when using `#let`' do + expect_offense(<<~RUBY) + let(:foo) { Foo.new } + ^^^^^^^^^ Avoid using `let` – use a method call or local variable instead. + RUBY + end + + it 'does not flag an offense when using `#subject` without a name' do + expect_no_offenses(<<~RUBY) + subject { Foo.new } + RUBY + end + + it 'does not flag an offense when using `#subject` with a name' do + expect_no_offenses(<<~RUBY) + subject(:foo) { Foo.new } + RUBY + end + end +end