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

Avoid instance variables in helpers #29

Merged
merged 7 commits into from
Jan 7, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ Rails/HasManyOrHasOneDependent:
Include:
- app/models/**/*.rb

Rails/HelperInstanceVariable:
Include:
- app/helpers/**/*.rb

Rails/HttpStatus:
EnforcedStyle: symbolic
SupportedStyles:
Expand Down
4 changes: 4 additions & 0 deletions config/enabled.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ Rails/HasManyOrHasOneDependent:
StyleGuide: 'https://github.com/rubocop-hq/rails-style-guide#has_many-has_one-dependent-option'
Enabled: true

Rails/HelperInstanceVariable:
Description: 'Do not use instance variables in helpers'
Enabled: true

Rails/HttpPositionalArguments:
Description: 'Use keyword arguments instead of positional arguments in http method calls.'
Enabled: true
Expand Down
39 changes: 39 additions & 0 deletions lib/rubocop/cop/rails/helper_instance_variable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# This cop checks for use of the helper methods which reference
# instance variables.
#
# Relying on instance variables makes it difficult to re-use helper
# methods.
#
# If it seems awkward to explicitly pass in each dependent
# variable, consider moving the behaviour elsewhere, for
# example to a model, decorator or presenter.
#
# @example
# # bad
# def welcome_message
# "Hello #{@user.name}"
# end
#
# # good
# def welcome_message(user)
# "Hello #{user.name}"
# end
class HelperInstanceVariable < Cop
MSG = 'Do not use instance variables in helpers.'.freeze

def on_ivar(node)
add_offense(node)
end

def on_ivasgn(node)
add_offense(node, location: :name)
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ module Cop
require_relative 'rails/find_each'
require_relative 'rails/has_and_belongs_to_many'
require_relative 'rails/has_many_or_has_one_dependent'
require_relative 'rails/helper_instance_variable'
require_relative 'rails/http_positional_arguments'
require_relative 'rails/http_status'
require_relative 'rails/inverse_of'
Expand Down
36 changes: 36 additions & 0 deletions manual/cops_rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,42 @@ Include | `app/models/**/*.rb` | Array

* [https://github.com/rubocop-hq/rails-style-guide#has_many-has_one-dependent-option](https://github.com/rubocop-hq/rails-style-guide#has_many-has_one-dependent-option)

## Rails/HelperInstanceVariable

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

This cop checks for use of the helper methods which reference
instance variables.

Relying on instance variables makes it difficult to re-use helper
methods.

If it seems awkward to explicitly pass in each dependent
variable, consider moving the behaviour elsewhere, for
example to a model, decorator or presenter.

### Examples

```ruby
# bad
def welcome_message
"Hello #{@user.name}"
end

# good
def welcome_message(user)
"Hello #{user.name}"
end
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
Include | `app/helpers/**/*.rb` | Array

## Rails/HttpPositionalArguments

Enabled by default | Supports autocorrection
Expand Down
31 changes: 31 additions & 0 deletions spec/rubocop/cop/rails/helper_instance_variable_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::HelperInstanceVariable do
subject(:cop) { described_class.new }

it 'reports uses of instance variables' do
expect_offense(<<-'RUBY'.strip_indent)
def welcome_message
"Hello #{@user.name}"
^^^^^ Do not use instance variables in helpers.
end
RUBY
end

it 'reports instance variable assignments' do
expect_offense(<<-RUBY.strip_indent)
def welcome_message(user)
@user_name = user.name
^^^^^^^^^^ Do not use instance variables in helpers.
end
RUBY
end

specify do
expect_no_offenses(<<-'RUBY'.strip_indent)
def welcome_message(user)
"Hello #{user.name}"
end
RUBY
end
end