diff --git a/Gemfile b/Gemfile index 8545215b..6683f2fc 100644 --- a/Gemfile +++ b/Gemfile @@ -5,13 +5,15 @@ source 'https://rubygems.org' gem "activesupport", require: false gem "parser" gem "pry", require: false -gem "rubocop", "1.62.1", require: false +gem "rubocop", "1.64.1", require: false gem "rubocop-capybara", require: false +gem "rubocop-factory_bot", require: false gem "rubocop-graphql", require: false gem "rubocop-i18n", require: false gem "rubocop-minitest", require: false gem "rubocop-performance", require: false gem "rubocop-rails", require: false +gem "rubocop-rails-omakase", require: false gem "rubocop-rake", require: false gem "rubocop-rspec", require: false gem "rubocop-rspec_rails", require: false @@ -19,7 +21,6 @@ gem "rubocop-sequel", require: false gem "rubocop-shopify", require: false gem "rubocop-sorbet", require: false gem "rubocop-thread_safety", require: false -gem "rubocop-factory_bot", require: false gem "test-prof", require: false group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 26f3578e..6ce40714 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ GEM remote: https://rubygems.org/ specs: - activesupport (7.1.3.2) + activesupport (7.1.3.4) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) @@ -13,46 +13,47 @@ GEM tzinfo (~> 2.0) ast (2.4.2) base64 (0.2.0) - bigdecimal (3.1.7) + bigdecimal (3.1.8) coderay (1.1.3) - concurrent-ruby (1.2.3) + concurrent-ruby (1.3.3) connection_pool (2.4.1) diff-lcs (1.5.1) drb (2.2.1) - i18n (1.14.4) + i18n (1.14.5) concurrent-ruby (~> 1.0) - json (2.7.1) + json (2.7.2) language_server-protocol (3.17.0.3) - method_source (1.0.0) - minitest (5.22.3) + method_source (1.1.0) + minitest (5.24.1) mutex_m (0.2.0) - parallel (1.24.0) - parser (3.3.0.5) + parallel (1.25.1) + parser (3.3.3.0) ast (~> 2.4.1) racc pry (0.14.2) coderay (~> 1.1) method_source (~> 1.0) - racc (1.7.3) - rack (3.0.9.1) + racc (1.8.0) + rack (3.1.4) rainbow (3.1.1) - rake (13.1.0) - regexp_parser (2.9.0) - rexml (3.2.6) + rake (13.2.1) + regexp_parser (2.9.2) + rexml (3.3.1) + strscan rspec (3.13.0) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) rspec-core (3.13.0) rspec-support (~> 3.13.0) - rspec-expectations (3.13.0) + rspec-expectations (3.13.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) - rspec-mocks (3.13.0) + rspec-mocks (3.13.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-support (3.13.1) - rubocop (1.62.1) + rubocop (1.64.1) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) @@ -63,45 +64,50 @@ GEM rubocop-ast (>= 1.31.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.31.2) - parser (>= 3.3.0.4) - rubocop-capybara (2.20.0) + rubocop-ast (1.31.3) + parser (>= 3.3.1.0) + rubocop-capybara (2.21.0) rubocop (~> 1.41) - rubocop-factory_bot (2.25.1) - rubocop (~> 1.41) - rubocop-graphql (1.5.0) + rubocop-factory_bot (2.26.1) + rubocop (~> 1.61) + rubocop-graphql (1.5.2) rubocop (>= 0.90, < 2) rubocop-i18n (3.0.0) rubocop (~> 1.0) rubocop-minitest (0.35.0) rubocop (>= 1.61, < 2.0) rubocop-ast (>= 1.31.1, < 2.0) - rubocop-performance (1.20.2) + rubocop-performance (1.21.1) rubocop (>= 1.48.1, < 2.0) - rubocop-ast (>= 1.30.0, < 2.0) - rubocop-rails (2.24.0) + rubocop-ast (>= 1.31.1, < 2.0) + rubocop-rails (2.25.1) activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 1.33.0, < 2.0) rubocop-ast (>= 1.31.1, < 2.0) + rubocop-rails-omakase (1.0.0) + rubocop + rubocop-minitest + rubocop-performance + rubocop-rails rubocop-rake (0.6.0) rubocop (~> 1.0) - rubocop-rspec (2.27.1) - rubocop (~> 1.40) - rubocop-capybara (~> 2.17) - rubocop-factory_bot (~> 2.22) - rubocop-rspec_rails (2.28.2) - rubocop (~> 1.40) + rubocop-rspec (3.0.1) + rubocop (~> 1.61) + rubocop-rspec_rails (2.30.0) + rubocop (~> 1.61) + rubocop-rspec (~> 3, >= 3.0.1) rubocop-sequel (0.3.4) rubocop (~> 1.0) rubocop-shopify (2.15.1) rubocop (~> 1.51) - rubocop-sorbet (0.7.8) + rubocop-sorbet (0.8.3) rubocop (>= 0.90.0) rubocop-thread_safety (0.5.1) rubocop (>= 0.90.0) ruby-progressbar (1.13.0) - test-prof (1.3.2) + strscan (3.1.0) + test-prof (1.3.3.1) tzinfo (2.0.6) concurrent-ruby (~> 1.0) unicode-display_width (2.5.0) @@ -115,7 +121,7 @@ DEPENDENCIES pry rake rspec - rubocop (= 1.62.1) + rubocop (= 1.64.1) rubocop-capybara rubocop-factory_bot rubocop-graphql @@ -123,6 +129,7 @@ DEPENDENCIES rubocop-minitest rubocop-performance rubocop-rails + rubocop-rails-omakase rubocop-rake rubocop-rspec rubocop-rspec_rails diff --git a/config/contents/lint/debugger.md b/config/contents/lint/debugger.md index f94605a7..a36224e0 100644 --- a/config/contents/lint/debugger.md +++ b/config/contents/lint/debugger.md @@ -22,6 +22,11 @@ Lint/Debugger: MyDebugger.debug_this ``` +Some gems also ship files that will start a debugging session when required, +for example `require 'debug/start'` from `ruby/debug`. These requires can +be configured through `DebuggerRequires`. It has the same structure as +`DebuggerMethods`, which you can read about above. + ### Example: # bad (ok during development) @@ -56,4 +61,10 @@ Lint/Debugger: def some_method my_debugger - end \ No newline at end of file + end + +### Example: DebuggerRequires: [my_debugger/start] + + # bad (ok during development) + + require 'my_debugger/start' \ No newline at end of file diff --git a/config/contents/style/access_modifier_declarations.md b/config/contents/style/access_modifier_declarations.md index 27751c18..0811a77b 100644 --- a/config/contents/style/access_modifier_declarations.md +++ b/config/contents/style/access_modifier_declarations.md @@ -3,6 +3,17 @@ or inline before each method, depending on configuration. EnforcedStyle config covers only method definitions. Applications of visibility methods to symbols can be controlled using AllowModifiersOnSymbols config. +Also, the visibility of `attr*` methods can be controlled using +AllowModifiersOnAttrs config. + +In Ruby 3.0, `attr*` methods now return an array of defined method names +as symbols. So we can write the modifier and `attr*` in inline style. +AllowModifiersOnAttrs config allows `attr*` methods to be written in +inline style without modifying applications that have been maintained +for a long time in group style. Furthermore, developers who are not very +familiar with Ruby may know that the modifier applies to `def`, but they +may not know that it also applies to `attr*` methods. It would be easier +to understand if we could write `attr*` methods in inline style. ### Safety: @@ -62,4 +73,32 @@ the group access modifier. private :bar, :baz + end + +### Example: AllowModifiersOnAttrs: true (default) + # good + class Foo + + public attr_reader :bar + protected attr_writer :baz + private attr_accessor :qux + private attr :quux + + def public_method; end + + private + + def private_method; end + + end + +### Example: AllowModifiersOnAttrs: false + # bad + class Foo + + public attr_reader :bar + protected attr_writer :baz + private attr_accessor :qux + private attr :quux + end \ No newline at end of file diff --git a/config/contents/style/arguments_forwarding.md b/config/contents/style/arguments_forwarding.md index 2628da67..337c91cc 100644 --- a/config/contents/style/arguments_forwarding.md +++ b/config/contents/style/arguments_forwarding.md @@ -24,6 +24,8 @@ e.g., `*args`, `**options`, `&block`, and so on. Names not on this list are likely to be meaningful and are allowed by default. +This cop handles not only method forwarding but also forwarding to `super`. + ### Example: # bad def foo(*args, &block) diff --git a/config/contents/style/collection_compact.md b/config/contents/style/collection_compact.md index f6486e7d..666584ad 100644 --- a/config/contents/style/collection_compact.md +++ b/config/contents/style/collection_compact.md @@ -15,9 +15,7 @@ when the receiver is a hash object. ### Example: # bad array.reject(&:nil?) - array.delete_if(&:nil?) array.reject { |e| e.nil? } - array.delete_if { |e| e.nil? } array.select { |e| !e.nil? } array.grep_v(nil) array.grep_v(NilClass) @@ -27,7 +25,9 @@ when the receiver is a hash object. # bad hash.reject!(&:nil?) + array.delete_if(&:nil?) hash.reject! { |k, v| v.nil? } + array.delete_if { |e| e.nil? } hash.select! { |k, v| !v.nil? } # good diff --git a/config/contents/style/documentation.md b/config/contents/style/documentation.md index 068eff10..ea7e18ba 100644 --- a/config/contents/style/documentation.md +++ b/config/contents/style/documentation.md @@ -24,36 +24,36 @@ same for all its children. end # allowed - # Class without body + # Class without body + class Person + end + + # Namespace - A namespace can be a class or a module + # Containing a class + module Namespace + # Description/Explanation of Person class class Person + # ... end + end - # Namespace - A namespace can be a class or a module - # Containing a class - module Namespace - # Description/Explanation of Person class - class Person - # ... - end + # Containing constant visibility declaration + module Namespace + class Private end - # Containing constant visibility declaration - module Namespace - class Private - end - - private_constant :Private - end + private_constant :Private + end - # Containing constant definition - module Namespace - Public = Class.new - end + # Containing constant definition + module Namespace + Public = Class.new + end - # Macro calls - module Namespace - extend Foo - end + # Macro calls + module Namespace + extend Foo + end ### Example: AllowedConstants: ['ClassMethods'] @@ -62,4 +62,4 @@ same for all its children. module ClassMethods # ... end - end + end diff --git a/config/contents/style/documentation_method.md b/config/contents/style/documentation_method.md index 7654bef3..093d4e22 100644 --- a/config/contents/style/documentation_method.md +++ b/config/contents/style/documentation_method.md @@ -89,3 +89,14 @@ they are called constructor to distinguish it from method. def do_something end end + +### Example: AllowedMethods: ['method_missing', 'respond_to_missing?'] + + # good + class Foo + def method_missing(name, *args) + end + + def respond_to_missing?(symbol, include_private) + end + end diff --git a/config/contents/style/format_string.md b/config/contents/style/format_string.md index 9b9b8dab..b1958986 100644 --- a/config/contents/style/format_string.md +++ b/config/contents/style/format_string.md @@ -19,24 +19,24 @@ format('%s', [1, 2, 3]) #=> '[1, 2, 3]' ### Example: EnforcedStyle: format (default) # bad - puts sprintf('%10s', 'hoge') - puts '%10s' % 'hoge' + puts sprintf('%10s', 'foo') + puts '%10s' % 'foo' # good - puts format('%10s', 'hoge') + puts format('%10s', 'foo') ### Example: EnforcedStyle: sprintf # bad - puts format('%10s', 'hoge') - puts '%10s' % 'hoge' + puts format('%10s', 'foo') + puts '%10s' % 'foo' # good - puts sprintf('%10s', 'hoge') + puts sprintf('%10s', 'foo') ### Example: EnforcedStyle: percent # bad - puts format('%10s', 'hoge') - puts sprintf('%10s', 'hoge') + puts format('%10s', 'foo') + puts sprintf('%10s', 'foo') # good - puts '%10s' % 'hoge' + puts '%10s' % 'foo' diff --git a/config/contents/style/hash_syntax.md b/config/contents/style/hash_syntax.md index 19cc363b..778d5b74 100644 --- a/config/contents/style/hash_syntax.md +++ b/config/contents/style/hash_syntax.md @@ -24,6 +24,8 @@ The supported styles are: * never - forces use of explicit hash literal value * either - accepts both shorthand and explicit use of hash literal value * consistent - forces use of the 3.1 syntax only if all values can be omitted in the hash +* either_consistent - accepts both shorthand and explicit use of hash literal value, + but they must be consistent ### Example: EnforcedStyle: ruby19 (default) # bad @@ -104,3 +106,20 @@ The supported styles are: # good - can't omit `baz` {foo: foo, bar: baz} + +### Example: EnforcedShorthandSyntax: either_consistent + + # good - `foo` and `bar` values can be omitted, but they are consistent, so it's accepted + {foo: foo, bar: bar} + + # bad - `bar` value can be omitted + {foo:, bar: bar} + + # bad - mixed syntaxes + {foo:, bar: baz} + + # good + {foo:, bar:} + + # good - can't omit `baz` + {foo: foo, bar: baz} \ No newline at end of file diff --git a/config/contents/style/map_into_array.md b/config/contents/style/map_into_array.md new file mode 100644 index 00000000..d52f2ae9 --- /dev/null +++ b/config/contents/style/map_into_array.md @@ -0,0 +1,43 @@ +Checks for usages of `each` with `<<`, `push`, or `append` which +can be replaced by `map`. + +If `PreferredMethods` is configured for `map` in `Style/CollectionMethods`, +this cop uses the specified method for replacement. + +NOTE: The return value of `Enumerable#each` is `self`, whereas the +return value of `Enumerable#map` is an `Array`. They are not autocorrected +when a return value could be used because these types differ. + +NOTE: It only detects when the mapping destination is a local variable +initialized as an empty array and referred to only by the pushing operation. +This is because, if not, it's challenging to statically guarantee that the +mapping destination variable remains an empty array: + +```ruby +ret = [] +src.each { |e| ret << e * 2 } # `<<` method may mutate `ret` + +dest = [] +src.each { |e| dest << transform(e, dest) } # `transform` method may mutate `dest` +``` + +### Safety: + +This cop is unsafe because not all objects that have an `each` +method also have a `map` method (e.g. `ENV`). Additionally, for calls +with a block, not all objects that have a `map` method return an array +(e.g. `Enumerator::Lazy`). + +### Example: + # bad + dest = [] + src.each { |e| dest << e * 2 } + dest + + # good + dest = src.map { |e| e * 2 } + + # good - contains another operation + dest = [] + src.each { |e| dest << e * 2; puts e } + dest diff --git a/config/contents/style/send.md b/config/contents/style/send.md index b96d80da..bb07148b 100644 --- a/config/contents/style/send.md +++ b/config/contents/style/send.md @@ -2,9 +2,9 @@ Checks for the use of the send method. ### Example: # bad - Foo.send(:bar) - quuz.send(:fred) + Foo.send(bar) + quuz.send(fred) # good - Foo.__send__(:bar) - quuz.public_send(:fred) \ No newline at end of file + Foo.__send__(bar) + quuz.public_send(fred) \ No newline at end of file diff --git a/config/contents/style/send_with_literal_method_name.md b/config/contents/style/send_with_literal_method_name.md new file mode 100644 index 00000000..613420a0 --- /dev/null +++ b/config/contents/style/send_with_literal_method_name.md @@ -0,0 +1,34 @@ +Detects the use of the `public_send` method with a literal method name argument. +Since the `send` method can be used to call private methods, by default, +only the `public_send` method is detected. + +### Safety: + +This cop is not safe because it can incorrectly detect based on the receiver. +Additionally, when `AllowSend` is set to `true`, it cannot determine whether +the `send` method being detected is calling a private method. + +### Example: + # bad + obj.public_send(:method_name) + obj.public_send('method_name') + + # good + obj.method_name + +### Example: AllowSend: true (default) + # good + obj.send(:method_name) + obj.send('method_name') + obj.__send__(:method_name) + obj.__send__('method_name') + +### Example: AllowSend: false + # bad + obj.send(:method_name) + obj.send('method_name') + obj.__send__(:method_name) + obj.__send__('method_name') + + # good + obj.method_name diff --git a/config/contents/style/special_global_vars.md b/config/contents/style/special_global_vars.md index 782c88b4..988d4a95 100644 --- a/config/contents/style/special_global_vars.md +++ b/config/contents/style/special_global_vars.md @@ -54,9 +54,8 @@ true, replacing perl-style variables with english variables will break. ### Example: EnforcedStyle: use_builtin_english_names -Like `use_perl_names` but allows builtin global vars. - # good + # Like `use_perl_names` but allows builtin global vars. puts $LOAD_PATH puts $LOADED_FEATURES puts $PROGRAM_NAME diff --git a/config/contents/style/super_arguments.md b/config/contents/style/super_arguments.md new file mode 100644 index 00000000..e5ff00f2 --- /dev/null +++ b/config/contents/style/super_arguments.md @@ -0,0 +1,31 @@ +Checks for redundant argument forwarding when calling super +with arguments identical to the method definition. + +### Example: + # bad + def method(*args, **kwargs) + super(*args, **kwargs) + end + + # good - implicitly passing all arguments + def method(*args, **kwargs) + super + end + + # good - forwarding a subset of the arguments + def method(*args, **kwargs) + super(*args) + end + + # good - forwarding no arguments + def method(*args, **kwargs) + super() + end + + # good - assigning to the block variable before calling super + def method(&block) + # Assigning to the block variable would pass the old value to super, + # under this circumstance the block must be referenced explicitly. + block ||= proc { 'fallback behavior' } + super(&block) + end \ No newline at end of file diff --git a/config/contents/style/symbol_proc.md b/config/contents/style/symbol_proc.md index 6cecee42..e6f95d03 100644 --- a/config/contents/style/symbol_proc.md +++ b/config/contents/style/symbol_proc.md @@ -113,3 +113,20 @@ boxes.first.symbol_to_proc_matches?(*boxes) ### Example: AllowedPatterns: ['map'] (default) # good something.map { |s| s.upcase } + +### Example: AllCops:ActiveSupportExtensionsEnabled: false (default) + # bad + ->(x) { x.foo } + proc { |x| x.foo } + Proc.new { |x| x.foo } + + # good + lambda(&:foo) + proc(&:foo) + Proc.new(&:foo) + +### Example: AllCops:ActiveSupportExtensionsEnabled: true + # good + ->(x) { x.foo } + proc { |x| x.foo } + Proc.new { |x| x.foo } diff --git a/spec/cc/engine/issue_spec.rb b/spec/cc/engine/issue_spec.rb index 335a1573..70ceec94 100644 --- a/spec/cc/engine/issue_spec.rb +++ b/spec/cc/engine/issue_spec.rb @@ -1,5 +1,6 @@ require "spec_helper" require "cc/engine/issue" +require "ostruct" module CC::Engine describe Issue do diff --git a/spec/cc/engine/rubocop_spec.rb b/spec/cc/engine/rubocop_spec.rb index 022464e3..5a7a0352 100644 --- a/spec/cc/engine/rubocop_spec.rb +++ b/spec/cc/engine/rubocop_spec.rb @@ -1,5 +1,6 @@ require "spec_helper" require "cc/engine/rubocop" +require "ostruct" require "tmpdir" module CC::Engine