Skip to content

Commit

Permalink
Add new Lint/RedundantDirGlobSort cop
Browse files Browse the repository at this point in the history
Sort globbed results by default in Ruby 3.0.
https://bugs.ruby-lang.org/issues/8709

This PR adds new `Lint/RedundantDirGlobSort` cop.
It checks for redundant `sort` method to `Dir.glob` and `Dir[]`.

```ruby
# bad
Dir.glob('./lib/**/*.rb').sort.each do |file|
end

Dir['./lib/**/*.rb'].sort.each do |file|
end

# good
Dir.glob('./lib/**/*.rb').each do |file|
end

Dir['./lib/**/*.rb'].each do |file|
end

# good - Respect intent if `sort` keyword option is specified.
Dir.glob('./lib/**/*.rb', sort: false).each do |file|
end
```

Related PR #9300.
  • Loading branch information
koic committed Dec 29, 2020
1 parent 5ec7718 commit adfea8c
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_new_lint_redundant_dir_glob_sort_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#9301](https://github.com/rubocop-hq/rubocop/pull/9301): Add new `Lint/RedundantDirGlobSort` cop. ([@koic][])
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1835,6 +1835,11 @@ Lint/RedundantCopEnableDirective:
Enabled: true
VersionAdded: '0.76'

Lint/RedundantDirGlobSort:
Description: 'Checks for redundant `sort` method to `Dir.glob` and `Dir[]`.'
Enabled: pending
VersionAdded: '<<next>>'

Lint/RedundantRequireStatement:
Description: 'Checks for unnecessary `require` statement.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@
require_relative 'rubocop/cop/lint/rand_one'
require_relative 'rubocop/cop/lint/redundant_cop_disable_directive'
require_relative 'rubocop/cop/lint/redundant_cop_enable_directive'
require_relative 'rubocop/cop/lint/redundant_dir_glob_sort'
require_relative 'rubocop/cop/lint/redundant_require_statement'
require_relative 'rubocop/cop/lint/redundant_safe_navigation'
require_relative 'rubocop/cop/lint/redundant_splat_expansion'
Expand Down
48 changes: 48 additions & 0 deletions lib/rubocop/cop/lint/redundant_dir_glob_sort.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# Sort globbed results by default in Ruby 3.0.
# This cop checks for redundant `sort` method to `Dir.glob` and `Dir[]`.
#
# @example
#
# # bad
# Dir.glob('./lib/**/*.rb').sort.each do |file|
# end
#
# Dir['./lib/**/*.rb'].sort.each do |file|
# end
#
# # good
# Dir.glob('./lib/**/*.rb').each do |file|
# end
#
# Dir['./lib/**/*.rb'].each do |file|
# end
#
class RedundantDirGlobSort < Base
extend AutoCorrector
extend TargetRubyVersion

minimum_target_ruby_version 3.0

MSG = 'Remove redundant `sort`.'
RESTRICT_ON_SEND = %i[sort].freeze
GLOB_METHODS = %i[glob []].freeze

def on_send(node)
return unless (receiver = node.receiver)
return unless receiver.receiver.const_type? && receiver.receiver.short_name == :Dir
return unless GLOB_METHODS.include?(receiver.method_name)

add_offense(node.loc.selector) do |corrector|
corrector.remove(node.loc.selector)
corrector.remove(node.loc.dot)
end
end
end
end
end
end
98 changes: 98 additions & 0 deletions spec/rubocop/cop/lint/redundant_dir_glob_sort_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::RedundantDirGlobSort, :config do
context 'when Ruby 3.0 or higher', :ruby30 do
it 'registers an offense and correction when using `Dir.glob.sort`' do
expect_offense(<<~RUBY)
Dir.glob(Rails.root.join('test', '*.rb')).sort.each(&method(:require))
^^^^ Remove redundant `sort`.
RUBY

expect_correction(<<~RUBY)
Dir.glob(Rails.root.join('test', '*.rb')).each(&method(:require))
RUBY
end

it 'registers an offense and correction when using `::Dir.glob.sort`' do
expect_offense(<<~RUBY)
::Dir.glob(Rails.root.join('test', '*.rb')).sort.each(&method(:require))
^^^^ Remove redundant `sort`.
RUBY

expect_correction(<<~RUBY)
::Dir.glob(Rails.root.join('test', '*.rb')).each(&method(:require))
RUBY
end

it 'registers an offense and correction when using `Dir[].sort.each do`' do
expect_offense(<<~RUBY)
Dir['./lib/**/*.rb'].sort.each do |file|
^^^^ Remove redundant `sort`.
end
RUBY

expect_correction(<<~RUBY)
Dir['./lib/**/*.rb'].each do |file|
end
RUBY
end

it 'registers an offense and correction when using `Dir[].sort.each(&do_something)`' do
expect_offense(<<~RUBY)
Dir['./lib/**/*.rb'].sort.each(&method(:require))
^^^^ Remove redundant `sort`.
RUBY

expect_correction(<<~RUBY)
Dir['./lib/**/*.rb'].each(&method(:require))
RUBY
end

it 'does not register an offense when not using `sort` with `sort: false` option for `Dir`' do
expect_no_offenses(<<~RUBY)
Dir.glob(Rails.root.join('test', '*.rb'), sort: false).each do
end
RUBY
end
end

context 'when Ruby 2.7 or lower', :ruby27 do
it 'does not register an offense and correction when using `Dir.glob.sort`' do
expect_no_offenses(<<~RUBY)
Dir.glob(Rails.root.join('test', '*.rb')).sort.each(&method(:require))
RUBY
end

it 'does not register an offense and correction when using `::Dir.glob.sort`' do
expect_no_offenses(<<~RUBY)
::Dir.glob(Rails.root.join('test', '*.rb')).sort.each(&method(:require))
RUBY
end

it 'does not register an offense and correction when using `Dir[].sort.each do`' do
expect_no_offenses(<<~RUBY)
Dir['./lib/**/*.rb'].sort.each do |file|
end
RUBY
end

it 'does not register an offense and correction when using `Dir[].sort.each(&do_something)`' do
expect_no_offenses(<<~RUBY)
Dir['./lib/**/*.rb'].sort.each(&method(:require))
RUBY
end
end

it 'does not register an offense when not using `sort` for `Dir`' do
expect_no_offenses(<<~RUBY)
Dir['./lib/**/*.rb'].each do |file|
end
RUBY
end

it 'does not register an offense when using `sort` without a receiver' do
expect_no_offenses(<<~RUBY)
sort.do_something
RUBY
end
end

0 comments on commit adfea8c

Please sign in to comment.