From a975c1b029c430e117fde2d997a7202adfcff626 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 15 Feb 2022 02:03:31 +0900 Subject: [PATCH] Add new `Style/NestedFileDirname` cop ## Summary This cop checks for nested `File.dirname`. It replaces nested `File.dirname` with the level argument introduced in Ruby 3.1. ```ruby # bad File.dirname(File.dirname(path)) # good File.dirname(path, 2) ``` See: https://bugs.ruby-lang.org/issues/12194 ## Other Information For `File.expand_path('../..', path)`, there is a partial conflict with `Style/ExpandPathArguments` cop. So this PR does not support it. https://docs.rubocop.org/rubocop/1.25/cops_style.html#styleexpandpatharguments --- ...w_add_new_style_nested_file_dirname_cop.md | 1 + config/default.yml | 5 ++ lib/rubocop.rb | 1 + lib/rubocop/cop/style/nested_file_dirname.rb | 66 +++++++++++++++++++ .../cop/style/nested_file_dirname_spec.rb | 47 +++++++++++++ 5 files changed, 120 insertions(+) create mode 100644 changelog/new_add_new_style_nested_file_dirname_cop.md create mode 100644 lib/rubocop/cop/style/nested_file_dirname.rb create mode 100644 spec/rubocop/cop/style/nested_file_dirname_spec.rb diff --git a/changelog/new_add_new_style_nested_file_dirname_cop.md b/changelog/new_add_new_style_nested_file_dirname_cop.md new file mode 100644 index 00000000000..299cfce2daa --- /dev/null +++ b/changelog/new_add_new_style_nested_file_dirname_cop.md @@ -0,0 +1 @@ +* [#10419](https://github.com/rubocop/rubocop/pull/10419): Add new `Style/NestedFileDirname` cop. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index 7dd3d931e19..b7cc534c100 100644 --- a/config/default.yml +++ b/config/default.yml @@ -4124,6 +4124,11 @@ Style/NegatedWhile: Enabled: true VersionAdded: '0.20' +Style/NestedFileDirname: + Description: 'Checks for nested `File.dirname`.' + Enabled: pending + VersionAdded: '<>' + Style/NestedModifier: Description: 'Avoid using nested modifiers.' StyleGuide: '#no-nested-modifiers' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 1e12bb939d1..5fdab922c26 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -552,6 +552,7 @@ require_relative 'rubocop/cop/style/negated_if_else_condition' require_relative 'rubocop/cop/style/negated_unless' require_relative 'rubocop/cop/style/negated_while' +require_relative 'rubocop/cop/style/nested_file_dirname' require_relative 'rubocop/cop/style/nested_modifier' require_relative 'rubocop/cop/style/nested_parenthesized_calls' require_relative 'rubocop/cop/style/nested_ternary_operator' diff --git a/lib/rubocop/cop/style/nested_file_dirname.rb b/lib/rubocop/cop/style/nested_file_dirname.rb new file mode 100644 index 00000000000..aceb5de5031 --- /dev/null +++ b/lib/rubocop/cop/style/nested_file_dirname.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop checks for nested `File.dirname`. + # It replaces nested `File.dirname` with the level argument introduced in Ruby 3.1. + # + # @example + # + # # bad + # File.dirname(File.dirname(path)) + # + # # good + # File.dirname(path, 2) + # + class NestedFileDirname < Base + include RangeHelp + extend AutoCorrector + extend TargetRubyVersion + + MSG = 'Use `dirname(%s, %s)` instead.' + RESTRICT_ON_SEND = %i[dirname].freeze + + minimum_target_ruby_version 3.1 + + # @!method file_dirname?(node) + def_node_matcher :file_dirname?, <<~PATTERN + (send + (const {cbase nil?} :File) :dirname ...) + PATTERN + + def on_send(node) + return if file_dirname?(node.parent) || !file_dirname?(node.first_argument) + + path, level = path_with_dir_level(node, 1) + return if level < 2 + + message = format(MSG, path: path, level: level) + range = offense_range(node) + + add_offense(range, message: message) do |corrector| + corrector.replace(range, "dirname(#{path}, #{level})") + end + end + + private + + def path_with_dir_level(node, level) + first_argument = node.first_argument + + if file_dirname?(first_argument) + level += 1 + path_with_dir_level(first_argument, level) + else + [first_argument.source, level] + end + end + + def offense_range(node) + range_between(node.loc.selector.begin_pos, node.source_range.end_pos) + end + end + end + end +end diff --git a/spec/rubocop/cop/style/nested_file_dirname_spec.rb b/spec/rubocop/cop/style/nested_file_dirname_spec.rb new file mode 100644 index 00000000000..490378b37e3 --- /dev/null +++ b/spec/rubocop/cop/style/nested_file_dirname_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::NestedFileDirname, :config do + context 'Ruby >= 3.1', :ruby31 do + it 'registers and corrects an offense when using `File.dirname(path)` nested two times' do + expect_offense(<<~RUBY) + File.dirname(File.dirname(path)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `dirname(path, 2)` instead. + RUBY + + expect_correction(<<~RUBY) + File.dirname(path, 2) + RUBY + end + + it 'registers and corrects an offense when using `File.dirname(path)` nested three times' do + expect_offense(<<~RUBY) + File.dirname(File.dirname(File.dirname(path))) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `dirname(path, 3)` instead. + RUBY + + expect_correction(<<~RUBY) + File.dirname(path, 3) + RUBY + end + + it 'does not register an offense when using non nested `File.dirname(path)`' do + expect_no_offenses(<<~RUBY) + File.dirname(path) + RUBY + end + + it 'does not register an offense when using `File.dirname(path, 2)`' do + expect_no_offenses(<<~RUBY) + File.dirname(path, 2) + RUBY + end + end + + context 'Ruby <= 3.0', :ruby30 do + it 'does not register an offense when using `File.dirname(path)` nested two times' do + expect_no_offenses(<<~RUBY) + File.dirname(File.dirname(path)) + RUBY + end + end +end