Skip to content

Commit

Permalink
[Fix #7680] Add new Lint/StructNewOverride cop (#7699)
Browse files Browse the repository at this point in the history
  • Loading branch information
ybiquitous committed Mar 21, 2020
1 parent ea1d48f commit 4dacec0
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,7 @@
* [#7786](https://github.com/rubocop-hq/rubocop/pull/7786): Support Ruby 2.7's pattern match for `Layout/ElseAlignment` cop. ([@koic][])
* [#7784](https://github.com/rubocop-hq/rubocop/pull/7784): Support Ruby 2.7's numbered parameter for `Lint/SafeNavigationChain`. ([@koic][])
* [#7331](https://github.com/rubocop-hq/rubocop/issues/7331): Add `forbidden` option to `Style/ModuleFunction` cop. ([@weh][])
* [#7699](https://github.com/rubocop-hq/rubocop/pull/7699): Add new `Lint/StructNewOverride` cop. ([@ybiquitous][])

### Bug fixes

Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Expand Up @@ -1714,6 +1714,11 @@ Lint/ShadowingOuterLocalVariable:
Enabled: true
VersionAdded: '0.9'

Lint/StructNewOverride:
Description: 'Disallow overriding the `Struct` built-in methods via `Struct.new`.'
Enabled: pending
VersionAdded: '0.81'

Lint/SuppressedException:
Description: "Don't suppress exceptions."
StyleGuide: '#dont-hide-exceptions'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -348,6 +348,7 @@
require_relative 'rubocop/cop/lint/shadowed_argument'
require_relative 'rubocop/cop/lint/shadowed_exception'
require_relative 'rubocop/cop/lint/shadowing_outer_local_variable'
require_relative 'rubocop/cop/lint/struct_new_override'
require_relative 'rubocop/cop/lint/suppressed_exception'
require_relative 'rubocop/cop/lint/syntax'
require_relative 'rubocop/cop/lint/to_json'
Expand Down
58 changes: 58 additions & 0 deletions lib/rubocop/cop/lint/struct_new_override.rb
@@ -0,0 +1,58 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks unexpected overrides of the `Struct` built-in methods
# via `Struct.new`.
#
# @example
# # bad
# Bad = Struct.new(:members, :clone, :count)
# b = Bad.new([], true, 1)
# b.members #=> [] (overriding `Struct#members`)
# b.clone #=> true (overriding `Object#clone`)
# b.count #=> 1 (overriding `Enumerable#count`)
#
# # good
# Good = Struct.new(:id, :name)
# g = Good.new(1, "foo")
# g.members #=> [:id, :name]
# g.clone #=> #<struct Good id=1, name="foo">
# g.count #=> 2
#
class StructNewOverride < Cop
MSG = '`%<member_name>s` member overrides `Struct#%<method_name>s`' \
' and it may be unexpected.'

STRUCT_METHOD_NAMES = Struct.instance_methods
STRUCT_MEMBER_NAME_TYPES = %i[sym str].freeze

def_node_matcher :struct_new, <<~PATTERN
(send
(const ${nil? cbase} :Struct) :new ...)
PATTERN

def on_send(node)
return unless struct_new(node) do
node.arguments.each_with_index do |arg, index|
# Ignore if the first argument is a class name
next if index.zero? && arg.str_type?

# Ignore if the argument is not a member name
next unless STRUCT_MEMBER_NAME_TYPES.include?(arg.type)

member_name = arg.value

next unless STRUCT_METHOD_NAMES.include?(member_name.to_sym)

message = format(MSG, member_name: member_name.inspect,
method_name: member_name.to_s)
add_offense(arg, message: message)
end
end
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Expand Up @@ -243,6 +243,7 @@ In the following section you find all available cops:
* [Lint/ShadowedArgument](cops_lint.md#lintshadowedargument)
* [Lint/ShadowedException](cops_lint.md#lintshadowedexception)
* [Lint/ShadowingOuterLocalVariable](cops_lint.md#lintshadowingouterlocalvariable)
* [Lint/StructNewOverride](cops_lint.md#lintstructnewoverride)
* [Lint/SuppressedException](cops_lint.md#lintsuppressedexception)
* [Lint/Syntax](cops_lint.md#lintsyntax)
* [Lint/ToJSON](cops_lint.md#linttojson)
Expand Down
27 changes: 27 additions & 0 deletions manual/cops_lint.md
Expand Up @@ -2298,6 +2298,33 @@ def some_method
end
```

## Lint/StructNewOverride

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Pending | Yes | No | 0.81 | -

This cop checks unexpected overrides of the `Struct` built-in methods
via `Struct.new`.

### Examples

```ruby
# bad
Bad = Struct.new(:members, :clone, :count)
b = Bad.new([], true, 1)
b.members #=> [] (overriding `Struct#members`)
b.clone #=> true (overriding `Object#clone`)
b.count #=> 1 (overriding `Enumerable#count`)

# good
Good = Struct.new(:id, :name)
g = Good.new(1, "foo")
g.members #=> [:id, :name]
g.clone #=> #<struct Good id=1, name="foo">
g.count #=> 2
```

## Lint/SuppressedException

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
85 changes: 85 additions & 0 deletions spec/rubocop/cop/lint/struct_new_override_spec.rb
@@ -0,0 +1,85 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::StructNewOverride do
subject(:cop) { described_class.new(config) }

let(:config) { RuboCop::Config.new }

it 'registers an offense using `Struct.new(symbol)`' do
expect_offense(<<~RUBY)
Bad = Struct.new(:members)
^^^^^^^^ `:members` member overrides `Struct#members` and it may be unexpected.
RUBY
end

it 'registers an offense using `::Struct.new(symbol)`' do
expect_offense(<<~RUBY)
Bad = ::Struct.new(:members)
^^^^^^^^ `:members` member overrides `Struct#members` and it may be unexpected.
RUBY
end

it 'registers an offense using `Struct.new(string, ...symbols)`' do
expect_offense(<<~RUBY)
Struct.new('Bad', :members, :name)
^^^^^^^^ `:members` member overrides `Struct#members` and it may be unexpected.
RUBY
end

it 'registers an offense using `Struct.new(...symbols)`' do
expect_offense(<<~RUBY)
Bad = Struct.new(:name, :members, :address)
^^^^^^^^ `:members` member overrides `Struct#members` and it may be unexpected.
RUBY
end

it 'registers an offense using `Struct.new(symbol, string)`' do
expect_offense(<<~RUBY)
Bad = Struct.new(:name, "members")
^^^^^^^^^ `"members"` member overrides `Struct#members` and it may be unexpected.
RUBY
end

it 'registers an offense using `Struct.new(...)` with a block' do
expect_offense(<<~RUBY)
Struct.new(:members) do
^^^^^^^^ `:members` member overrides `Struct#members` and it may be unexpected.
def members?
!members.empty?
end
end
RUBY
end

it 'registers an offense using `Struct.new(...)` with multiple overrides' do
expect_offense(<<~RUBY)
Struct.new(:members, :clone, :zip)
^^^^ `:zip` member overrides `Struct#zip` and it may be unexpected.
^^^^^^ `:clone` member overrides `Struct#clone` and it may be unexpected.
^^^^^^^^ `:members` member overrides `Struct#members` and it may be unexpected.
RUBY
end

it 'registers an offense using `Struct.new(...)` with an option argument' do
expect_offense(<<~RUBY)
Struct.new(:members, keyword_init: true)
^^^^^^^^ `:members` member overrides `Struct#members` and it may be unexpected.
RUBY
end

it 'does not register an offense with no overrides' do
expect_no_offenses(<<~RUBY)
Good = Struct.new(:id, :name)
RUBY
end

it 'does not register an offense with an override within a given block' do
expect_no_offenses(<<~RUBY)
Good = Struct.new(:id, :name) do
def members
super.tap { |ret| pp "members: " + ret.to_s }
end
end
RUBY
end
end

0 comments on commit 4dacec0

Please sign in to comment.