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.

Sign up for GitHub

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

Jump to bottom

Cop idea: Replace ENV['HOME'] with Dir.home #10568

Closed
ShockwaveNN opened this issue Apr 22, 2022 · 1 comment · Fixed by #10598
Closed

Cop idea: Replace ENV['HOME'] with Dir.home #10568

ShockwaveNN opened this issue Apr 22, 2022 · 1 comment · Fixed by #10598
Labels
feature request

Comments

@ShockwaveNN
Copy link
Contributor

Is your feature request related to a problem? Please describe.

While fixing issues from latest rubocop update with Style/FetchEnvVar cop I found a lot of placed in my code where construction like this used:

File.read("#{ENV['HOME']}/foo.file"

I got two problems:

  1. Autofix from Style/FetchEnvVar advice to use 
    File.read("#{ENV.fetch('HOME'), nil]}/foo.file"
    Which seem rather lengthy for me and do not solve any real problems
  2. Not sure that my original code ever worked on Windows system (but cannot check it)

Describe the solution you'd like

Cop that replace all ENV['HOME'] to Dir.home wich will be pure ruby solution to get home dir
But maybe there is some downside in this, but I'm not aware of such
@ShockwaveNN ShockwaveNN changed the title Cop idea: Replace ENV['HOME'] with Dir.home` Cop idea: Replace ENV['HOME'] with Dir.home Apr 22, 2022
@rubyFeedback
Copy link

I believe Dir.home may be the better alternative these days. I don't recall since when it was available, but I remember in the very old days we primarily used ENV['HOME']. Dir.home is probably a tiny bit easier to type too - not that it makes a huge difference. But I think if we have not forgotten about some use case then this proposal is probably useful. That is of course only my personal opinion.

@koic koic mentioned this issue May 2, 2022
Merged
8 tasks
koic added a commit to koic/rubocop that referenced this issue May 3, 2022
@koic
[Fix rubocop#10568] Add new Style/EnvHome cop
31332ba 
Fixes rubocop#10568.

This cop checks for consistent usage of `ENV['HOME']`. If `nil` is used as
the second argument of `ENV.fetch`, it is treated as a bad case like `ENV[]`.
This avoids conflicts with `Style/FetchEnvVar`'s autocorrection.

## Safety

The cop is unsafe because The result when `nil` is assigned to `ENV['HOME']` changes:

```ruby
ENV['HOME'] = nil
ENV['HOME'] # => nil
Dir.home    # => '/home/foo'
```

## Example

```ruby
# bad
ENV['HOME']
ENV.fetch('HOME', nil)

# good
Dir.home

# good
ENV.fetch('HOME', default)
```
bbatsov pushed a commit that referenced this issue May 6, 2022
@koic @bbatsov
[Fix #10568] Add new Style/EnvHome cop
914551a 
Fixes #10568.

This cop checks for consistent usage of `ENV['HOME']`. If `nil` is used as
the second argument of `ENV.fetch`, it is treated as a bad case like `ENV[]`.
This avoids conflicts with `Style/FetchEnvVar`'s autocorrection.

## Safety

The cop is unsafe because The result when `nil` is assigned to `ENV['HOME']` changes:

```ruby
ENV['HOME'] = nil
ENV['HOME'] # => nil
Dir.home    # => '/home/foo'
```

## Example

```ruby
# bad
ENV['HOME']
ENV.fetch('HOME', nil)

# good
Dir.home

# good
ENV.fetch('HOME', default)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fix #10568] Add new Style/EnvHome cop koic/rubocop
3 participants
@koic @ShockwaveNN @rubyFeedback