Skip to content

Commit

Permalink
Add new RSpec/VariableDefinition cop
Browse files Browse the repository at this point in the history
Closes #908

Also refactor common matcher into a module & change
`RSpec/VariableName` cop to consider interpolated
strings & symbols (they are parsed as `dsym` & `dstr` nodes).
  • Loading branch information
tejasbubane committed May 10, 2020
1 parent c361d5d commit 4b1f284
Show file tree
Hide file tree
Showing 11 changed files with 209 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Master (Unreleased)

* Add new `RSpec/VariableName` cop. ([@tejasbubane][])
* Add new `RSpec/VariableDefinition` cop. ([@tejasbubane][])

## 1.39.0 (2020-05-01)

Expand Down
12 changes: 11 additions & 1 deletion config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -454,8 +454,18 @@ RSpec/UnspecifiedException:
Enabled: true
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/UnspecifiedException

RSpec/VariableDefinition:
Description: Checks that memoized helpers names are symbols or strings.
Enabled: true
EnforcedStyle: symbols
SupportedStyles:
- symbols
- strings
VersionAdded: '1.40'
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/VariableDefinition

RSpec/VariableName:
Description: This cop makes sure that all variables use the configured style.
Description: Checks that memoized helper names use the configured style.
Enabled: true
EnforcedStyle: snake_case
SupportedStyles:
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop-rspec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
require_relative 'rubocop/rspec/example_group'
require_relative 'rubocop/rspec/example'
require_relative 'rubocop/rspec/hook'
require_relative 'rubocop/rspec/variable'
require_relative 'rubocop/cop/rspec/cop'
require_relative 'rubocop/rspec/align_let_brace'
require_relative 'rubocop/rspec/factory_bot'
Expand Down
56 changes: 56 additions & 0 deletions lib/rubocop/cop/rspec/variable_definition.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# frozen_string_literal: true

module RuboCop
module Cop
module RSpec
# Checks that memoized helpers names are symbols or strings.
#
# @example EnforcedStyle: symbols (default)
# # bad
# let('user_name') { 'Adam' }
# subject('user') { create_user }
#
# # good
# let(:user_name) { 'Adam' }
# subject(:user) { create_user }
#
# @example EnforcedStyle: strings
# # bad
# let(:user_name) { 'Adam' }
# subject(:user) { create_user }
#
# # good
# let('user_name') { 'Adam' }
# subject('user') { create_user }
class VariableDefinition < Cop
include ConfigurableEnforcedStyle
include RuboCop::RSpec::Variable

MSG = 'Use %<style>s for variable names.'

def on_send(node)
variable_definition?(node) do |variable|
if style_violation?(variable)
add_offense(variable, message: format(MSG, style: style))
end
end
end

private

def style_violation?(variable)
style == :symbols && string?(variable) ||
style == :strings && symbol?(variable)
end

def string?(node)
node.str_type? || node.dstr_type?
end

def symbol?(node)
node.sym_type? || node.dsym_type?
end
end
end
end
end
10 changes: 4 additions & 6 deletions lib/rubocop/cop/rspec/variable_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module RuboCop
module Cop
module RSpec
# This cop makes sure that all variables use the configured style.
# Checks that memoized helper names use the configured style.
#
# @example EnforcedStyle: snake_case (default)
# # bad
Expand All @@ -24,16 +24,14 @@ module RSpec
# subject(:userName) { 'Adam' }
class VariableName < Cop
include ConfigurableNaming
include RuboCop::RSpec::Variable

MSG = 'Use %<style>s for variable names.'

def_node_matcher :variable_definition?, <<~PATTERN
(send #{RSPEC} #{(Helpers::ALL + Subject::ALL).node_pattern_union}
$({sym str} _) ...)
PATTERN

def on_send(node)
variable_definition?(node) do |variable|
return if variable.dstr_type? || variable.dsym_type?

check_name(node, variable.value, variable.loc.expression)
end
end
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rspec_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
require_relative 'rspec/single_argument_message_chain'
require_relative 'rspec/subject_stub'
require_relative 'rspec/unspecified_exception'
require_relative 'rspec/variable_definition'
require_relative 'rspec/variable_name'
require_relative 'rspec/verified_doubles'
require_relative 'rspec/void_expect'
Expand Down
16 changes: 16 additions & 0 deletions lib/rubocop/rspec/variable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

module RuboCop
module RSpec
# Helps check offenses with variable definitions
module Variable
include Language
extend RuboCop::NodePattern::Macros

def_node_matcher :variable_definition?, <<~PATTERN
(send #{RSPEC} #{(Helpers::ALL + Subject::ALL).node_pattern_union}
$({sym str dsym dstr} ...) ...)
PATTERN
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
* [RSpec/SingleArgumentMessageChain](cops_rspec.md#rspecsingleargumentmessagechain)
* [RSpec/SubjectStub](cops_rspec.md#rspecsubjectstub)
* [RSpec/UnspecifiedException](cops_rspec.md#rspecunspecifiedexception)
* [RSpec/VariableDefinition](cops_rspec.md#rspecvariabledefinition)
* [RSpec/VariableName](cops_rspec.md#rspecvariablename)
* [RSpec/VerifiedDoubles](cops_rspec.md#rspecverifieddoubles)
* [RSpec/VoidExpect](cops_rspec.md#rspecvoidexpect)
Expand Down
46 changes: 45 additions & 1 deletion manual/cops_rspec.md
Original file line number Diff line number Diff line change
Expand Up @@ -3013,13 +3013,57 @@ expect { do_something }.not_to raise_error

* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/UnspecifiedException](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/UnspecifiedException)

## RSpec/VariableDefinition

Enabled by default | Supports autocorrection
--- | ---
Enabled | No

Checks that memoized helpers names are symbols or strings.

### Examples

#### EnforcedStyle: symbols (default)

```ruby
# bad
let('user_name') { 'Adam' }
subject('user') { create_user }

# good
let(:user_name) { 'Adam' }
subject(:user) { create_user }
```
#### EnforcedStyle: strings

```ruby
# bad
let(:user_name) { 'Adam' }
subject(:user) { create_user }

# good
let('user_name') { 'Adam' }
subject('user') { create_user }
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
EnforcedStyle | `symbols` | `symbols`, `strings`
VersionAdded | `1.40` | String

### References

* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/VariableDefinition](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/VariableDefinition)

## RSpec/VariableName

Enabled by default | Supports autocorrection
--- | ---
Enabled | No

This cop makes sure that all variables use the configured style.
Checks that memoized helper names use the configured style.

### Examples

Expand Down
61 changes: 61 additions & 0 deletions spec/rubocop/cop/rspec/variable_definition_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::RSpec::VariableDefinition, :config do
subject(:cop) { described_class.new(config) }

context 'when EnforcedStyle is `symbols`' do
let(:cop_config) { { 'EnforcedStyle' => 'symbols' } }

it 'registers an offense for string name' do
expect_offense(<<~RUBY)
let("user_name") { 'Adam' }
^^^^^^^^^^^ Use symbols for variable names.
RUBY
end

it 'registers an offense for interpolated string' do
expect_offense(<<~'RUBY')
let("user-#{id}") { 'Adam' }
^^^^^^^^^^^^ Use symbols for variable names.
RUBY
end

it 'registers an offense for multiline string' do
expect_offense(<<~'RUBY')
let("user"\
^^^^^^^ Use symbols for variable names.
"-foo") { 'Adam' }
RUBY
end

it 'does not register offense for symbol names' do
expect_no_offenses(<<~RUBY)
let(:user_name) { 'Adam' }
RUBY
end
end

context 'when EnforcedStyle is `strings`' do
let(:cop_config) { { 'EnforcedStyle' => 'strings' } }

it 'registers an offense for symbol name' do
expect_offense(<<~RUBY)
let(:user_name) { 'Adam' }
^^^^^^^^^^ Use strings for variable names.
RUBY
end

it 'registers an offense for interpolated symbol' do
expect_offense(<<~'RUBY')
let(:"user-#{id}") { 'Adam' }
^^^^^^^^^^^^^ Use strings for variable names.
RUBY
end

it 'does not register offense for string names' do
expect_no_offenses(<<~RUBY)
let("user_name") { 'Adam' }
RUBY
end
end
end
12 changes: 12 additions & 0 deletions spec/rubocop/cop/rspec/variable_name_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
let(:user_name) { 'Adam' }
RUBY
end

it 'does not register offense for interpolated symbol' do
expect_no_offenses(<<~'RUBY')
let(:"user#{name}") { 'Adam' }
RUBY
end
end

context 'when `let` with string names' do
Expand All @@ -58,6 +64,12 @@
let('user_name') { 'Adam' }
RUBY
end

it 'does not register offense for interpolated string' do
expect_no_offenses(<<~'RUBY')
let("user#{name}") { 'Adam' }
RUBY
end
end

context 'when `let` with proc' do
Expand Down

0 comments on commit 4b1f284

Please sign in to comment.