Skip to content

Commit

Permalink
Add new Performance/OpenStruct cop (#6486)
Browse files Browse the repository at this point in the history
Add new `Performance/OpenStruct` cop which checks for `OpenStruct.new` calls. The suggested way to fix this issue is to replace `OpenStruct`s with static `Struct`s. The cop is disabled by default.
  • Loading branch information
xlts authored and bbatsov committed Nov 20, 2018
1 parent 4b81d17 commit c74c199
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@

* [#6457](https://github.com/rubocop-hq/rubocop/pull/6457): Support inner slash correction on Style/RegexpLiteral. ([@r7kamura][])
* [#6475](https://github.com/rubocop-hq/rubocop/pull/6475): Support brace correction on Style/Lambda. ([@r7kamura][])
* New cop `Performance/OpenStruct` checks for `OpenStruct.new` calls. ([@xlts][])

### Bug fixes

Expand Down Expand Up @@ -3656,3 +3657,4 @@
[@andrew-aladev]: https://github.com/andrew-aladev
[@y-yagi]: https://github.com/y-yagi
[@DiscoStarslayer]: https://github.com/DiscoStarslayer
[@xlts]: https://github.com/xlts
6 changes: 6 additions & 0 deletions config/default.yml
Expand Up @@ -2063,6 +2063,12 @@ Performance/LstripRstrip:
Enabled: true
VersionAdded: '0.36'

Performance/OpenStruct:
Description: 'Use `Struct` instead of `OpenStruct`.'
Enabled: false
VersionAdded: '0.61'
Safe: false

Performance/RangeInclude:
Description: 'Use `Range#cover?` instead of `Range#include?`.'
Reference: 'https://github.com/JuanitoFatas/fast-ruby#cover-vs-include-code'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -368,6 +368,7 @@
require_relative 'rubocop/cop/performance/flat_map'
require_relative 'rubocop/cop/performance/inefficient_hash_search'
require_relative 'rubocop/cop/performance/lstrip_rstrip'
require_relative 'rubocop/cop/performance/open_struct'
require_relative 'rubocop/cop/performance/range_include'
require_relative 'rubocop/cop/performance/redundant_block_call'
require_relative 'rubocop/cop/performance/redundant_match'
Expand Down
46 changes: 46 additions & 0 deletions lib/rubocop/cop/performance/open_struct.rb
@@ -0,0 +1,46 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Performance
# This cop checks for `OpenStruct.new` calls.
# Instantiation of an `OpenStruct` invalidates
# Ruby global method cache as it causes dynamic method
# definition during program runtime.
# This could have an effect on performance,
# especially in case of single-threaded
# applications with multiple `OpenStruct` instantiations.

# @example
# # bad
# class MyClass
# def my_method
# OpenStruct.new(my_key1: 'my_value1', my_key2: 'my_value2')
# end
# end
#
# # good
# class MyClass
# MyStruct = Struct.new(:my_key1, :my_key2)
# def my_method
# MyStruct.new('my_value1', 'my_value2')
# end
# end
#
class OpenStruct < Cop
MSG = 'Consider using `Struct` over `OpenStruct` ' \
'to optimize the performance.'.freeze

def_node_matcher :open_struct, <<-PATTERN
(send (const {nil? cbase} :OpenStruct) :new ...)
PATTERN

def on_send(node)
open_struct(node) do |method|
add_offense(node, location: :selector, message: format(MSG, method))
end
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Expand Up @@ -311,6 +311,7 @@ In the following section you find all available cops:
* [Performance/FlatMap](cops_performance.md#performanceflatmap)
* [Performance/InefficientHashSearch](cops_performance.md#performanceinefficienthashsearch)
* [Performance/LstripRstrip](cops_performance.md#performancelstriprstrip)
* [Performance/OpenStruct](cops_performance.md#performanceopenstruct)
* [Performance/RangeInclude](cops_performance.md#performancerangeinclude)
* [Performance/RedundantBlockCall](cops_performance.md#performanceredundantblockcall)
* [Performance/RedundantMatch](cops_performance.md#performanceredundantmatch)
Expand Down
27 changes: 27 additions & 0 deletions manual/cops_performance.md
Expand Up @@ -478,6 +478,33 @@ This cop identifies places where `lstrip.rstrip` can be replaced by
'abc'.strip
```

## Performance/OpenStruct

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Disabled | No | No | 0.61 | -



### Examples

```ruby
# bad
class MyClass
def my_method
OpenStruct.new(my_key1: 'my_value1', my_key2: 'my_value2')
end
end

# good
class MyClass
MyStruct = Struct.new(:my_key1, :my_key2)
def my_method
MyStruct.new('my_value1', 'my_value2')
end
end
```

## Performance/RangeInclude

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

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

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

it 'registers an offense for OpenStruct.new' do
expect_offense(<<-RUBY.strip_indent)
OpenStruct.new(key: "value")
^^^ Consider using `Struct` over `OpenStruct` to optimize the performance.
RUBY
end

it 'registers an offense for a fully qualified ::OpenStruct.new' do
expect_offense(<<-RUBY.strip_indent)
::OpenStruct.new(key: "value")
^^^ Consider using `Struct` over `OpenStruct` to optimize the performance.
RUBY
end

it 'does not register offense for Struct' do
expect_no_offenses(<<-RUBY.strip_indent)
MyStruct = Struct.new(:key)
MyStruct.new('value')
RUBY
end
end

1 comment on commit c74c199

@matkoniecz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it is disabled by default? https://github.com/rubocop-hq/rubocop/blob/master/config/default.yml typically lists what is problematic with cops (even one enabled by default).

I considered enabling this one, but as I don't understand potential danger I kept it disabled.

Please sign in to comment.