Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Style/FileDirnameLevel cop #10419

Merged

Conversation

koic
Copy link
Member

@koic koic commented Feb 15, 2022

Summary

This cop checks for nested File.dirname. It replaces repeats with the level argument introduced in Ruby 3.1.

# 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


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

module RuboCop
module Cop
module Style
# This cop checks for repeated `File.dirname`.
Copy link
Collaborator

@bbatsov bbatsov Mar 6, 2022

Choose a reason for hiding this comment

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

repeated -> nested

# # good
# File.dirname(path, 2)
#
class FileDirnameLevel < Base
Copy link
Collaborator

@bbatsov bbatsov Mar 6, 2022

Choose a reason for hiding this comment

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

I wonder if NestedFileDirname is not a better name, but I'm fine with the proposed name.

Copy link
Member Author

@koic koic Mar 9, 2022

Choose a reason for hiding this comment

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

Yeah, I was wondering whether to use that name. So, I've updated to NestedFileDirname to be consistent with the doc.


RSpec.describe RuboCop::Cop::Style::FileDirnameLevel, :config do
context 'Ruby >= 3.1', :ruby31 do
it 'registers and corrects an offense when using `File.dirname(path)` repeated two times' do
Copy link
Collaborator

@bbatsov bbatsov Mar 6, 2022

Choose a reason for hiding this comment

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

nested two times

And so on for the other example descriptions.

## 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
@koic koic force-pushed the new_add_new_style_file_dirname_level_cop branch from 193ce3d to a975c1b Compare Mar 9, 2022
@bbatsov bbatsov merged commit c95a26d into rubocop:master Mar 9, 2022
34 checks passed
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 9, 2022

Thanks!

@koic koic deleted the new_add_new_style_file_dirname_level_cop branch Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants