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

Add Capybara/VisibilityMatcher cop #886

Merged
merged 1 commit into from Mar 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -3,6 +3,7 @@
## Master (Unreleased)

* Fix `RSpec/FilePath` detection when absolute path includes test subject. ([@eitoball][])
* Add new `Capybara/VisibilityMatcher` cop. ([@aried3r][])

## 1.38.1 (2020-02-15)

Expand Down Expand Up @@ -492,3 +493,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
[@lazycoder9]: https://github.com/lazycoder9
[@elebow]: https://github.com/elebow
[@eitoball]: https://github.com/eitoball
[@aried3r]: https://github.com/aried3r
6 changes: 6 additions & 0 deletions config/default.yml
Expand Up @@ -475,6 +475,12 @@ Capybara/FeatureMethods:
EnabledMethods: []
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/FeatureMethods

Capybara/VisibilityMatcher:
Description: Checks for boolean visibility in capybara finders.
Enabled: true
VersionAdded: '1.39'
StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/VisibilityMatcher

FactoryBot/AttributeDefinedStatically:
Description: Always declare attribute values as blocks.
Enabled: true
Expand Down
54 changes: 54 additions & 0 deletions lib/rubocop/cop/rspec/capybara/visibility_matcher.rb
@@ -0,0 +1,54 @@
# frozen_string_literal: true

module RuboCop
module Cop
module RSpec
module Capybara
# Checks for boolean visibility in capybara finders.
#
# Capybara lets you find elements that match a certain visibility using
# the `:visible` option. `:visible` accepts both boolean and symbols as
# values, however using booleans can have unwanted effects. `visible:
# false` does not find just invisible elements, but both visible and
# invisible elements. For expressiveness and clarity, use one of the
# symbol values, `:all`, `:hidden` or `:visible`.
# (https://www.rubydoc.info/gems/capybara/Capybara%2FNode%2FFinders:all)
#
# @example
#
# # bad
# expect(page).to have_selector('.foo', visible: false)
#
# # bad
# expect(page).to have_selector('.foo', visible: true)
#
# # good
# expect(page).to have_selector('.foo', visible: :all)
#
# # good
# expect(page).to have_selector('.foo', visible: :hidden)
#
# # good
# expect(page).to have_selector('.foo', visible: :visible)
#
class VisibilityMatcher < Cop
MSG_FALSE = 'Use `:all` or `:hidden` instead of `false`.'
MSG_TRUE = 'Use `:visible` instead of `true`.'

def_node_matcher :visible_true?, <<~PATTERN
(send nil? :have_selector ... (hash <$(pair (sym :visible) true) ...>))
PATTERN

def_node_matcher :visible_false?, <<~PATTERN
(send nil? :have_selector ... (hash <$(pair (sym :visible) false) ...>))
PATTERN

def on_send(node)
visible_false?(node) { |arg| add_offense(arg, message: MSG_FALSE) }
visible_true?(node) { |arg| add_offense(arg, message: MSG_TRUE) }
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rspec_cops.rb
Expand Up @@ -2,6 +2,7 @@

require_relative 'rspec/capybara/current_path_expectation'
require_relative 'rspec/capybara/feature_methods'
require_relative 'rspec/capybara/visibility_matcher'

require_relative 'rspec/factory_bot/attribute_defined_statically'
require_relative 'rspec/factory_bot/create_list'
Expand Down
1 change: 1 addition & 0 deletions manual/cops.md
Expand Up @@ -3,6 +3,7 @@

* [Capybara/CurrentPathExpectation](cops_capybara.md#capybaracurrentpathexpectation)
* [Capybara/FeatureMethods](cops_capybara.md#capybarafeaturemethods)
* [Capybara/VisibilityMatcher](cops_capybara.md#capybaravisibilitymatcher)

#### Department [FactoryBot](cops_factorybot.md)

Expand Down
45 changes: 45 additions & 0 deletions manual/cops_capybara.md
Expand Up @@ -87,3 +87,48 @@ EnabledMethods | `[]` | Array
### References

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

## Capybara/VisibilityMatcher

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

Checks for boolean visibility in capybara finders.

Capybara lets you find elements that match a certain visibility using
the `:visible` option. `:visible` accepts both boolean and symbols as
values, however using booleans can have unwanted effects. `visible:
false` does not find just invisible elements, but both visible and
invisible elements. For expressiveness and clarity, use one of the
symbol values, `:all`, `:hidden` or `:visible`.
(https://www.rubydoc.info/gems/capybara/Capybara%2FNode%2FFinders:all)

### Examples

```ruby
# bad
expect(page).to have_selector('.foo', visible: false)

# bad
expect(page).to have_selector('.foo', visible: true)

# good
expect(page).to have_selector('.foo', visible: :all)

# good
expect(page).to have_selector('.foo', visible: :hidden)

# good
expect(page).to have_selector('.foo', visible: :visible)
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
VersionAdded | `1.39` | String

### References

* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/VisibilityMatcher](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/VisibilityMatcher)
69 changes: 69 additions & 0 deletions spec/rubocop/cop/rspec/capybara/visibility_matcher_spec.rb
@@ -0,0 +1,69 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::RSpec::Capybara::VisibilityMatcher do
subject(:cop) { described_class.new }

it 'registers an offense when using `visible: true`' do
expect_offense(<<-RUBY)
expect(page).to have_selector('.my_element', visible: true)
^^^^^^^^^^^^^ Use `:visible` instead of `true`.
RUBY
end

it 'registers an offense when using `visible: false`' do
expect_offense(<<-RUBY)
expect(page).to have_selector('.my_element', visible: false)
^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`.
RUBY
end

it 'registers an offense when using a selector`' do
expect_offense(<<-RUBY)
expect(page).to have_selector(:css, '.my_element', visible: false)
^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`.
RUBY
end

it 'registers an offense when using a using multiple options`' do
expect_offense(<<-RUBY)
expect(page).to have_selector('.my_element', count: 1, visible: false, normalize_ws: true)
^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`.
RUBY
end

it 'does not register an offense when no options are given`' do
expect_no_offenses(<<~RUBY)
expect(page).to have_selector('.my_element')
RUBY
end

it 'does not register an offense when using `visible: :all`' do
expect_no_offenses(<<~RUBY)
expect(page).to have_selector('.my_element', visible: :all)
RUBY
end

it 'does not register an offense when using `visible: :visible`' do
expect_no_offenses(<<~RUBY)
expect(page).to have_selector('.my_element', visible: :visible)
RUBY
end

it 'does not register an offense when using `visible: :hidden`' do
expect_no_offenses(<<~RUBY)
expect(page).to have_selector('.my_element', visible: :hidden)
RUBY
end

it 'does not register an offense when using other options' do
expect_no_offenses(<<~RUBY)
expect(page).to have_selector('.my_element', normalize_ws: true)
RUBY
end

it 'does not register an offense when using multiple options' do
expect_no_offenses(<<~RUBY)
expect(page).to have_selector('.my_element', count: 1, normalize_ws: true)
RUBY
end
end