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

Fixin the bugs like that #3182

Merged
merged 11 commits into from
Jun 6, 2016
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

### New features

* [#3095](https://github.com/bbatsov/rubocop/issues/3095): Add `IndentationWidth` configuration parameter for `Style/AlignParameters` cop. ([@alexdowad][])
* [#3066](https://github.com/bbatsov/rubocop/issues/3066): Add new `Style/ImplicitRuntimeError` cop which advises the use of an explicit exception class when raising an error. ([@alexdowad][])
* [#3018](https://github.com/bbatsov/rubocop/issues/3018): Add new `Style/EachForSimpleLoop` cop which advises the use of `Integer#times` for simple loops which iterate a fixed number of times. ([@alexdowad][])
* [#2595](https://github.com/bbatsov/rubocop/issues/2595): New `compact` style for `Style/SpaceInsideLiteralHashBraces`. ([@alexdowad][])
* [#2927](https://github.com/bbatsov/rubocop/issues/2927): Add autocorrect for `Rails/Validation` cop. ([@neodelf][])
* [#3135](https://github.com/bbatsov/rubocop/pull/3135): Add new `Rails/OutputSafety` cop. ([@josh][])
* [#3164](https://github.com/bbatsov/rubocop/pull/3164): Add [Fastlane](https://fastlane.tools/)'s Fastfile to the default Includes. ([@jules2689][])
Expand All @@ -12,6 +16,10 @@

### Bug fixes

* [#3037](https://github.com/bbatsov/rubocop/issues/3037): `Style/StringLiterals` understands that a bare '#', not '#@variable' or '#{interpolation}', does not require double quotes. ([@alexdowad][])
* [#2722](https://github.com/bbatsov/rubocop/issues/2722): `Style/ExtraSpacing` does not attempt to align an equals sign in an argument list with one in an assignment statement. ([@alexdowad][])
* [#3133](https://github.com/bbatsov/rubocop/issues/3133): `Style/MultilineMethodCallBraceLayout` does not register offenses for single-line calls. ([@alexdowad][])
* [#3170](https://github.com/bbatsov/rubocop/issues/3170): `Style/MutableConstant` does not infinite-loop when correcting an array with no brackets. ([@alexdowad][])
* [#3150](https://github.com/bbatsov/rubocop/issues/3150): Fix auto-correct for Style/MultilineArrayBraceLayout. ([@jspanjers][])
* [#3114](https://github.com/bbatsov/rubocop/issues/3114): Fix alignment `end` when auto-correcting `Style/EmptyElse`. ([@rrosenblum][])
* [#3120](https://github.com/bbatsov/rubocop/issues/3120): Fix `Lint/UselessAccessModifier` reporting useless access modifiers inside {Class,Module,Struct}.new blocks. ([@owst][])
Expand Down
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ Style/AlignParameters:
SupportedStyles:
- with_first_parameter
- with_fixed_indentation
# By default, the indentation width from Style/IndentationWidth is used
# But it can be overridden by setting this parameter
IndentationWidth: ~

Style/AndOr:
# Whether `and` and `or` are banned only in conditionals (conditionals)
Expand Down Expand Up @@ -908,6 +911,9 @@ Style/SpaceInsideHashLiteralBraces:
SupportedStyles:
- space
- no_space
# 'compact' normally requires a space inside hash braces, with the exception
# that successive left braces or right braces are collapsed together
- compact

Style/SpaceInsideStringInterpolation:
EnforcedStyle: no_space
Expand Down
6 changes: 6 additions & 0 deletions config/disabled.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ Style/FirstMethodParameterLineBreak:
multi-line method parameter definition.
Enabled: false

Style/ImplicitRuntimeError:
Description: >-
Use `raise` with an explicit exception class and message,
rather than just a message.
Enabled: false

Style/InlineComment:
Description: 'Avoid inline comments.'
Enabled: false
Expand Down
6 changes: 6 additions & 0 deletions config/enabled.yml
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ Style/DoubleNegation:
StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-bang-bang'
Enabled: true

Style/EachForSimpleLoop:
Description: >-
Use `Integer#times` for a simple loop which iterates a fixed
number of times.
Enabled: true

Style/EachWithObject:
Description: 'Prefer `each_with_object` over `inject` or `reduce`.'
Enabled: true
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@
require 'rubocop/cop/style/documentation'
require 'rubocop/cop/style/dot_position'
require 'rubocop/cop/style/double_negation'
require 'rubocop/cop/style/each_for_simple_loop'
require 'rubocop/cop/style/each_with_object'
require 'rubocop/cop/style/else_alignment'
require 'rubocop/cop/style/empty_case_condition'
Expand Down Expand Up @@ -242,6 +243,7 @@
require 'rubocop/cop/style/if_unless_modifier'
require 'rubocop/cop/style/if_unless_modifier_of_if_unless'
require 'rubocop/cop/style/if_with_semicolon'
require 'rubocop/cop/style/implicit_runtime_error'
require 'rubocop/cop/style/indent_array'
require 'rubocop/cop/style/indent_assignment'
require 'rubocop/cop/style/indent_hash'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/mixin/multiline_literal_brace_layout.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module MultilineLiteralBraceLayout
def check_brace_layout(node)
return unless node.loc.begin # Ignore implicit literals.
return if children(node).empty? # Ignore empty literals.
return if node.single_line? # Ignore single-line literals.

# If the last node is or contains a conflicting HEREDOC, we don't want
# to adjust the brace layout because this will result in invalid code.
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/string_literals_help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def wrong_quotes?(node)
if style == :single_quotes
src !~ /'/ && !double_quotes_acceptable?(node.str_content)
else
src !~ /" | \\ | \#/x
src !~ /" | \\ | \#(@|\{)/x
end
end

Expand Down
35 changes: 35 additions & 0 deletions lib/rubocop/cop/style/each_for_simple_loop.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# encoding: utf-8
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks for loops which iterate a constant number of times,
# using a Range literal and `#each`. This can be done more readably using
# `Integer#times`.
#
# This check only applies if the block takes no parameters.
#
# @example
# @bad
# (0..10).each { }
#
# @good
# 10.times { }
class EachForSimpleLoop < Cop
def_node_matcher :bad_each_loop, <<-PATTERN
(block (send (begin ({irange erange} int int)) :each) (args) ...)
PATTERN

def on_block(node)
if bad_each_loop(node)
send_node, = *node
range = send_node.receiver.source_range.join(send_node.loc.selector)
add_offense(node, range, 'Use `Integer#times` for a simple loop ' \
'which iterates a fixed number of times.')
end
end
end
end
end
end
13 changes: 13 additions & 0 deletions lib/rubocop/cop/style/extra_spacing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,15 @@ class ExtraSpacing < Cop
MSG_UNALIGNED_ASGN = '`=` is not aligned with the %s assignment.'.freeze

def investigate(processed_source)
return if processed_source.ast.nil?

if force_equal_sign_alignment?
@asgn_tokens = processed_source.tokens.select { |t| equal_sign?(t) }
# we don't want to operate on equals signs which are part of an
# optarg in a method definition
# e.g.: def method(optarg = default_val); end
@asgn_tokens = remove_optarg_equals(@asgn_tokens, processed_source)

# Only attempt to align the first = on each line
@asgn_tokens = Set.new(@asgn_tokens.uniq { |t| t.pos.line })
@asgn_lines = @asgn_tokens.map { |t| t.pos.line }
Expand Down Expand Up @@ -205,6 +212,12 @@ def align_column(asgn_token)
spaces = leading.size - (leading =~ / *\Z/)
asgn_token.pos.last_column - spaces + 1
end

def remove_optarg_equals(asgn_tokens, processed_source)
optargs = processed_source.ast.each_node(:optarg)
optarg_eql = optargs.map { |o| o.loc.operator.begin_pos }.to_set
asgn_tokens.reject { |t| optarg_eql.include?(t.pos.begin_pos) }
end
end
end
end
Expand Down
31 changes: 31 additions & 0 deletions lib/rubocop/cop/style/implicit_runtime_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# encoding: utf-8
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks for `raise` statements which do not specify an explicit
# exception class. (This raises a `RuntimeError`. Some projects might
# prefer to use exception classes which more precisely identify the
# nature of the error.)
#
# @example
# @bad
# raise 'Error message here'
#
# @good
# raise ArgumentError, 'Error message here'
class ImplicitRuntimeError < Cop
def_node_matcher :implicit_runtime_error_raise, '(send nil :raise str)'

def on_send(node)
if implicit_runtime_error_raise(node)
add_offense(node, :expression, 'Use `raise` with an explicit ' \
'exception class and message, ' \
'rather than just a message.')
end
end
end
end
end
end
9 changes: 8 additions & 1 deletion lib/rubocop/cop/style/mutable_constant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,14 @@ def on_or_asgn(node)

def autocorrect(node)
expr = node.source_range
->(corrector) { corrector.replace(expr, "#{expr.source}.freeze") }
lambda do |corrector|
if node.array_type? && node.loc.begin.nil? && node.loc.end.nil?
corrector.insert_before(expr, '[')
corrector.insert_after(expr, '].freeze')
else
corrector.insert_after(expr, '.freeze')
end
end
end

private
Expand Down
32 changes: 26 additions & 6 deletions lib/rubocop/cop/style/space_inside_hash_literal_braces.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,44 @@ def check(t1, t2)
return if t2.type == :tCOMMENT # Also indicates there's a line break.

is_empty_braces = t1.text == '{' && t2.text == '}'
expect_space = if is_empty_braces
cop_config['EnforcedStyleForEmptyBraces'] == 'space'
else
style == :space
end
expect_space = expect_space?(t1, t2)

if offense?(t1, t2, expect_space)
incorrect_style_detected(t1, t2, expect_space, is_empty_braces)
else
correct_style_detected
end
end

def expect_space?(t1, t2)
is_same_braces = t1.text == t2.text
is_empty_braces = t1.text == '{' && t2.text == '}'

if is_same_braces && style == :compact
false
elsif is_empty_braces
cop_config['EnforcedStyleForEmptyBraces'] != 'no_space'
else
style != :no_space
end
end

def incorrect_style_detected(t1, t2, expect_space, is_empty_braces)
brace = (t1.text == '{' ? t1 : t2).pos
range = expect_space ? brace : space_range(brace)
add_offense(range, range,
message(brace, is_empty_braces, expect_space)) do
opposite_style_detected
if expect_space
if t1.text == t2.text
ambiguous_style_detected(:no_space, :compact)
else
unexpected_style_detected(:no_space)
end
elsif t1.text == t2.text
unexpected_style_detected(:space)
else
ambiguous_style_detected(:space, :compact)
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion rubocop.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Gem::Specification.new do |s|
s.summary = 'Automatic Ruby code style checking tool.'

s.add_runtime_dependency('rainbow', '>= 1.99.1', '< 3.0')
s.add_runtime_dependency('parser', '>= 2.3.1.0', '< 3.0')
s.add_runtime_dependency('parser', '>= 2.3.1.1', '< 3.0')
s.add_runtime_dependency('powerpack', '~> 0.1')
s.add_runtime_dependency('ruby-progressbar', '~> 1.7')
s.add_runtime_dependency('unicode-display_width', '~> 1.0', '>= 1.0.1')
Expand Down
8 changes: 0 additions & 8 deletions spec/rubocop/cli/cli_autocorrect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -658,14 +658,6 @@ def abs(path)
'of an array.',
"#{e}:5:7: C: [Corrected] Use `%w` or `%W` " \
'for an array of words.',
"#{e}:5:8: C: [Corrected] Prefer single-quoted strings " \
"when you don't need string interpolation or special " \
'symbols.',
"#{e}:5:15: C: [Corrected] Prefer single-quoted strings " \
"when you don't need string interpolation or special " \
'symbols.',
"#{e}:5:21: C: [Corrected] Avoid comma after the last item " \
'of an array.',
''].join("\n"))
end

Expand Down
24 changes: 24 additions & 0 deletions spec/rubocop/cop/style/align_parameters_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,30 @@
.to eq(correct_source.join("\n"))
end
end

context 'with AlignParameters:IndentationWidth set to 4' do
let(:config) do
RuboCop::Config.new('Style/AlignParameters' =>
cop_config.merge('IndentationWidth' => 4))
end

it 'accepts the first parameter being on a new row' do
inspect_source(cop, [' assigned_value = match(',
' a,',
' b,',
' c',
' )'])
expect(cop.offenses).to be_empty
end

it 'accepts the first parameter being on method row' do
inspect_source(cop, [' assigned_value = match(a,',
' b,',
' c',
' )'])
expect(cop.offenses).to be_empty
end
end
end
end
end
39 changes: 39 additions & 0 deletions spec/rubocop/cop/style/each_for_simple_loop_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# encoding: utf-8
# frozen_string_literal: true

require 'spec_helper'

describe RuboCop::Cop::Style::EachForSimpleLoop do
subject(:cop) { described_class.new }

it 'registers an offense for (0..10).each {}' do
inspect_source(cop, '(0..10).each {}')
expect(cop.offenses.size).to eq 1
expect(cop.messages).to eq(['Use `Integer#times` for a simple loop which ' \
'iterates a fixed number of times.'])
expect(cop.highlights).to eq(['(0..10).each'])
end

it 'registers an offense for (0...10).each {}' do
inspect_source(cop, '(0...10).each {}')
expect(cop.offenses.size).to eq 1
expect(cop.messages).to eq(['Use `Integer#times` for a simple loop which ' \
'iterates a fixed number of times.'])
expect(cop.highlights).to eq(['(0...10).each'])
end

it "doesn't register an offense if range startpoint is not constant" do
inspect_source(cop, '(a..10).each {}')
expect(cop.offenses).to be_empty
end

it "doesn't register an offense if range endpoint is not constant" do
inspect_source(cop, '(0..b).each {}')
expect(cop.offenses).to be_empty
end

it "doesn't register an offense if block takes parameters" do
inspect_source(cop, '(0..10).each { |n| puts n }')
expect(cop.offenses).to be_empty
end
end
10 changes: 9 additions & 1 deletion spec/rubocop/cop/style/extra_spacing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@
expect(cop.offenses.size).to eq(0)
end

it 'aligns the first assignment with the following assingment' do
it 'aligns the first assignment with the following assignment' do
inspect_source(cop, ['# comment',
'a = 1',
'bb = 2'])
Expand Down Expand Up @@ -329,5 +329,13 @@
'a.attribute_name = 2',
'abc[1] = 3'].join("\n"))
end

it 'does not register an offense when optarg equals is not aligned with ' \
'assignment equals sign' do
inspect_source(cop, ['def method(arg = 1)',
' var = arg',
'end'])
expect(cop.offenses).to be_empty
end
end
end