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

[Fix #8102] Consider class-length instead of block-length for Struct.new #8122

Merged
merged 1 commit into from Sep 29, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@
### Changes

* [#8646](https://github.com/rubocop-hq/rubocop/issues/8646): Faster find of all files in `TargetFinder` class which improves rubocop initial startup speed. ([@tleish][])
* [#8102](https://github.com/rubocop-hq/rubocop/issues/8102): Consider class-length instead of block-length for `Struct.new`. ([@tejasbubane][])

## 0.92.0 (2020-09-25)

Expand Down
4 changes: 4 additions & 0 deletions docs/modules/ROOT/pages/cops_metrics.adoc
Expand Up @@ -57,6 +57,8 @@ You can set literals you want to fold with `CountAsOne`.
Available are: 'array', 'hash', and 'heredoc'. Each literal
will be counted as one line regardless of its actual size.

NOTE: This cop does not apply for `Struct` definitions.

=== Examples

==== CountAsOne: ['array', 'heredoc']
Expand Down Expand Up @@ -165,6 +167,8 @@ You can set literals you want to fold with `CountAsOne`.
Available are: 'array', 'hash', and 'heredoc'. Each literal
will be counted as one line regardless of its actual size.

NOTE: This cop also applies for `Struct` definitions.

=== Examples

==== CountAsOne: ['array', 'heredoc']
Expand Down
4 changes: 3 additions & 1 deletion lib/rubocop/cop/metrics/block_length.rb
Expand Up @@ -29,14 +29,16 @@ module Metrics
# content.
# HEREDOC
# end # 5 points
#
# NOTE: This cop does not apply for `Struct` definitions.
class BlockLength < Base
include CodeLength

LABEL = 'Block'

def on_block(node)
return if excluded_method?(node)
return if node.class_constructor?
return if node.class_constructor? || node.struct_constructor?

check_code_length(node)
end
Expand Down
11 changes: 4 additions & 7 deletions lib/rubocop/cop/metrics/class_length.rb
Expand Up @@ -29,6 +29,8 @@ module Metrics
# HEREDOC
# end # 5 points
#
#
# NOTE: This cop also applies for `Struct` definitions.
class ClassLength < Base
include CodeLength

Expand All @@ -37,17 +39,12 @@ def on_class(node)
end

def on_casgn(node)
class_definition?(node) do
check_code_length(node)
end
_scope, _name, block_node = *node
check_code_length(node) if block_node.class_definition?
end

private

def_node_matcher :class_definition?, <<~PATTERN
(casgn nil? _ (block (send (const {nil? cbase} :Class) :new) ...))
PATTERN

def message(length, max_length)
format('Class has too many lines. [%<length>d/%<max>d]',
length: length,
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/hash_transform_method.rb
Expand Up @@ -137,7 +137,7 @@ def transformation_uses_both_args?
end

# Internal helper class to hold autocorrect data
Autocorrection = Struct.new(:match, :block_node, :leading, :trailing) do # rubocop:disable Metrics/BlockLength
Autocorrection = Struct.new(:match, :block_node, :leading, :trailing) do
def self.from_each_with_object(node, match)
new(match, node, 0, 0)
end
Expand Down
4 changes: 0 additions & 4 deletions lib/rubocop/cop/variable_force/branch.rb
Expand Up @@ -20,8 +20,6 @@ def self.of(target_node, scope: nil)
nil
end

# rubocop:disable Metrics/BlockLength

# Abstract base class for branch classes.
# A branch represents a conditional branch in a scope.
#
Expand All @@ -42,8 +40,6 @@ def self.of(target_node, scope: nil)
# do_something # no branch
# end
Base = Struct.new(:child_node, :scope) do
# rubocop:enable Metrics/BlockLength

def self.classes
@classes ||= []
end
Expand Down
21 changes: 21 additions & 0 deletions spec/rubocop/cop/metrics/block_length_spec.rb
Expand Up @@ -159,6 +159,27 @@
end
end

context 'when defining a Struct' do
it 'does not register an offense' do
expect_no_offenses(<<~'RUBY')
Person = Struct.new(:first_name, :last_name) do
def full_name
"#{first_name} #{last_name}"
end

def foo
a = 1
a = 2
a = 3
a = 4
a = 5
a = 6
end
end
RUBY
end
end

context 'when CountComments is enabled' do
before { cop_config['CountComments'] = true }

Expand Down
16 changes: 16 additions & 0 deletions spec/rubocop/cop/metrics/class_length_spec.rb
Expand Up @@ -201,4 +201,20 @@ class Test
RUBY
end
end

context 'when inspecting a class defined with Struct.new' do
it 'registers an offense' do
expect_offense(<<~RUBY)
Foo = Struct.new(:foo, :bar) do
^^^ Class has too many lines. [6/5]
a = 1
a = 2
a = 3
a = 4
a = 5
a = 6
end
RUBY
end
end
end