Unescaped model attribute in content_tag Open
s = content_tag(:label, Preference.human_attribute_name(pref_name), :for => model + "_" + pref_name)
- Read upRead up
- Exclude checks
Cross-site scripting (or XSS) is #2 on the 2010 OWASP Top Ten web security risks and it pops up nearly everywhere. XSS occurs when a user-manipulatable value is displayed on a web page without escaping it, allowing someone to inject Javascript or HTML into the page.
content_tag is a view helper which generates an HTML tag with some content:
>> content_tag :p, "Hi!"
=> "Hi!
"
In Rails 2, this content is unescaped (although attribute values are escaped):
>> content_tag :p, "<script>alert(1)</script>"
=> "<script>alert(1)</script>
"
In Rails 3, the content is escaped. However, only the content and the tag attribute values are escaped. The tag and attribute names are never escaped in Rails 2 or 3.
This is more dangerous than a typical method call because content_tag
marks its output as "HTML safe", meaning the rails_xss
plugin and Rails 3 auto-escaping will not escape its output. Due to this, content_tag
should be used carefully if user input is provided as an argument.
Note that while content_tag
does have an escape
parameter, this only applies to tag attribute values and is true by default.
PreferencesHelper has no descriptive comment Open
module PreferencesHelper
- Read upRead up
- Exclude checks
Classes and modules are the units of reuse and release. It is therefore considered good practice to annotate every class and module with a brief comment outlining its responsibilities.
Example
Given
class Dummy
# Do things...
end
Reek would emit the following warning:
test.rb -- 1 warning:
[1]:Dummy has no descriptive comment (IrresponsibleModule)
Fixing this is simple - just an explaining comment:
# The Dummy class is responsible for ...
class Dummy
# Do things...
end
PreferencesHelper#profile_delete_user calls 'user.id' 2 times Open
url_for({ :controller => 'users', :action => 'destroy', :id => user.id }),
{ :id => "delete_user_#{user.id}",
- Read upRead up
- Exclude checks
Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.
Reek implements a check for Duplicate Method Call.
Example
Here's a very much simplified and contrived example. The following method will report a warning:
def double_thing()
@other.thing + @other.thing
end
One quick approach to silence Reek would be to refactor the code thus:
def double_thing()
thing = @other.thing
thing + thing
end
A slightly different approach would be to replace all calls of double_thing
by calls to @other.double_thing
:
class Other
def double_thing()
thing + thing
end
end
The approach you take will depend on balancing other factors in your code.
PreferencesHelper#profile_delete_user calls 't('users.destroy_user')' 2 times Open
t('users.destroy_user'),
url_for({ :controller => 'users', :action => 'destroy', :id => user.id }),
{ :id => "delete_user_#{user.id}",
:class => "delete_user_button btn btn-danger",
:title => t('users.destroy_user'),
- Read upRead up
- Exclude checks
Duplication occurs when two fragments of code look nearly identical, or when two fragments of code have nearly identical effects at some conceptual level.
Reek implements a check for Duplicate Method Call.
Example
Here's a very much simplified and contrived example. The following method will report a warning:
def double_thing()
@other.thing + @other.thing
end
One quick approach to silence Reek would be to refactor the code thus:
def double_thing()
thing = @other.thing
thing + thing
end
A slightly different approach would be to replace all calls of double_thing
by calls to @other.double_thing
:
class Other
def double_thing()
thing + thing
end
end
The approach you take will depend on balancing other factors in your code.
PreferencesHelper takes parameters ['model', 'pref_name'] to 4 methods Open
def pref(model, pref_name, &block)
s = content_tag(:label, Preference.human_attribute_name(pref_name), :for => model + "_" + pref_name)
s << yield
s
end
- Read upRead up
- Exclude checks
In general, a Data Clump
occurs when the same two or three items frequently appear together in classes and parameter lists, or when a group of instance variable names start or end with similar substrings.
The recurrence of the items often means there is duplicate code spread around to handle them. There may be an abstraction missing from the code, making the system harder to understand.
Example
Given
class Dummy
def x(y1,y2); end
def y(y1,y2); end
def z(y1,y2); end
end
Reek would emit the following warning:
test.rb -- 1 warning:
[2, 3, 4]:Dummy takes parameters [y1, y2] to 3 methods (DataClump)
A possible way to fix this problem (quoting from Martin Fowler):
The first step is to replace data clumps with objects and use the objects whenever you see them. An immediate benefit is that you'll shrink some parameter lists. The interesting stuff happens as you begin to look for behavior to move into the new objects.
PreferencesHelper#pref has the variable name 's' Open
s = content_tag(:label, Preference.human_attribute_name(pref_name), :for => model + "_" + pref_name)
- Read upRead up
- Exclude checks
An Uncommunicative Variable Name
is a variable name that doesn't communicate its intent well enough.
Poor names make it hard for the reader to build a mental picture of what's going on in the code. They can also be mis-interpreted; and they hurt the flow of reading, because the reader must slow down to interpret the names.
Missing magic comment # frozen_string_literal: true
. Open
module PreferencesHelper
- Read upRead up
- Exclude checks
This cop is designed to help upgrade to Ruby 3.0. It will add the
comment # frozen_string_literal: true
to the top of files to
enable frozen string literals. Frozen string literals may be default
in Ruby 3.0. The comment will be added below a shebang and encoding
comment. The frozen string literal comment is only valid in Ruby 2.3+.
Example: EnforcedStyle: when_needed (default)
# The `when_needed` style will add the frozen string literal comment
# to files only when the `TargetRubyVersion` is set to 2.3+.
# bad
module Foo
# ...
end
# good
# frozen_string_literal: true
module Foo
# ...
end
Example: EnforcedStyle: always
# The `always` style will always add the frozen string literal comment
# to a file, regardless of the Ruby version or if `freeze` or `<<` are
# called on a string literal.
# bad
module Bar
# ...
end
# good
# frozen_string_literal: true
module Bar
# ...
end
Example: EnforcedStyle: never
# The `never` will enforce that the frozen string literal comment does
# not exist in a file.
# bad
# frozen_string_literal: true
module Baz
# ...
end
# good
module Baz
# ...
end
Line is too long. [131/120] Open
def pref_with_select_field(model, pref_name, collection = [[t('preferences.is_true'), true], [t('preferences.is_false'), false]])
- Exclude checks
Redundant curly braces around a hash parameter. Open
{ :id => "delete_user_#{user.id}",
:class => "delete_user_button btn btn-danger",
:title => t('users.destroy_user'),
:x_confirm_message => t('users.destroy_confirmation', :login => user.login)
})
- Read upRead up
- Exclude checks
This cop checks for braces around the last parameter in a method call
if the last parameter is a hash.
It supports braces
, no_braces
and context_dependent
styles.
Example: EnforcedStyle: braces
# The `braces` style enforces braces around all method
# parameters that are hashes.
# bad
some_method(x, y, a: 1, b: 2)
# good
some_method(x, y, {a: 1, b: 2})
Example: EnforcedStyle: no_braces (default)
# The `no_braces` style checks that the last parameter doesn't
# have braces around it.
# bad
some_method(x, y, {a: 1, b: 2})
# good
some_method(x, y, a: 1, b: 2)
Example: EnforcedStyle: context_dependent
# The `context_dependent` style checks that the last parameter
# doesn't have braces around it, but requires braces if the
# second to last parameter is also a hash literal.
# bad
some_method(x, y, {a: 1, b: 2})
some_method(x, y, {a: 1, b: 2}, a: 1, b: 2)
# good
some_method(x, y, a: 1, b: 2)
some_method(x, y, {a: 1, b: 2}, {a: 1, b: 2})
Redundant return
detected. Open
return link_to(
- Read upRead up
- Exclude checks
This cop checks for redundant return
expressions.
Example:
def test
return something
end
def test
one
two
three
return something
end
It should be extended to handle methods whose body is if/else or a case expression with a default branch.
Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument. Open
})
- Read upRead up
- Exclude checks
This cop checks that the closing brace in a method call is either on the same line as the last method argument, or a new line.
When using the symmetrical
(default) style:
If a method call's opening brace is on the same line as the first argument of the call, then the closing brace should be on the same line as the last argument of the call.
If an method call's opening brace is on the line above the first argument of the call, then the closing brace should be on the line below the last argument of the call.
When using the new_line
style:
The closing brace of a multi-line method call must be on the line after the last argument of the call.
When using the same_line
style:
The closing brace of a multi-line method call must be on the same line as the last argument of the call.
Example:
# symmetrical: bad
# new_line: good
# same_line: bad
foo(a,
b
)
# symmetrical: bad
# new_line: bad
# same_line: good
foo(
a,
b)
# symmetrical: good
# new_line: bad
# same_line: good
foo(a,
b)
# symmetrical: good
# new_line: good
# same_line: bad
foo(
a,
b
)
Closing hash brace must be on the same line as the last hash element when opening brace is on the same line as the first hash element. Open
})
- Read upRead up
- Exclude checks
This cop checks that the closing brace in a hash literal is either on the same line as the last hash element, or a new line.
When using the symmetrical
(default) style:
If a hash's opening brace is on the same line as the first element of the hash, then the closing brace should be on the same line as the last element of the hash.
If a hash's opening brace is on the line above the first element of the hash, then the closing brace should be on the line below the last element of the hash.
When using the new_line
style:
The closing brace of a multi-line hash literal must be on the line after the last element of the hash.
When using the same_line
style:
The closing brace of a multi-line hash literal must be on the same line as the last element of the hash.
Example: EnforcedStyle: symmetrical (default)
# bad
{ a: 1,
b: 2
}
# bad
{
a: 1,
b: 2 }
# good
{ a: 1,
b: 2 }
# good
{
a: 1,
b: 2
}
Example: EnforcedStyle: new_line
# bad
{
a: 1,
b: 2 }
# bad
{ a: 1,
b: 2 }
# good
{ a: 1,
b: 2
}
# good
{
a: 1,
b: 2
}
Example: EnforcedStyle: same_line
# bad
{ a: 1,
b: 2
}
# bad
{
a: 1,
b: 2
}
# good
{
a: 1,
b: 2 }
# good
{ a: 1,
b: 2 }
Redundant curly braces around a hash parameter. Open
url_for({ :controller => 'users', :action => 'destroy', :id => user.id }),
- Read upRead up
- Exclude checks
This cop checks for braces around the last parameter in a method call
if the last parameter is a hash.
It supports braces
, no_braces
and context_dependent
styles.
Example: EnforcedStyle: braces
# The `braces` style enforces braces around all method
# parameters that are hashes.
# bad
some_method(x, y, a: 1, b: 2)
# good
some_method(x, y, {a: 1, b: 2})
Example: EnforcedStyle: no_braces (default)
# The `no_braces` style checks that the last parameter doesn't
# have braces around it.
# bad
some_method(x, y, {a: 1, b: 2})
# good
some_method(x, y, a: 1, b: 2)
Example: EnforcedStyle: context_dependent
# The `context_dependent` style checks that the last parameter
# doesn't have braces around it, but requires braces if the
# second to last parameter is also a hash literal.
# bad
some_method(x, y, {a: 1, b: 2})
some_method(x, y, {a: 1, b: 2}, a: 1, b: 2)
# good
some_method(x, y, a: 1, b: 2)
some_method(x, y, {a: 1, b: 2}, {a: 1, b: 2})
Unused method argument - block
. If it's necessary, use _
or _block
as an argument name to indicate that it won't be used. Open
def pref(model, pref_name, &block)
- Read upRead up
- Exclude checks
This cop checks for unused method arguments.
Example:
# bad
def some_method(used, unused, _unused_but_allowed)
puts used
end
Example:
# good
def some_method(used, _unused, _unused_but_allowed)
puts used
end