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

Hidden directories should not be searched by default #1656

Closed
sometimesfood opened this issue Feb 15, 2015 · 8 comments
Closed

Hidden directories should not be searched by default #1656

sometimesfood opened this issue Feb 15, 2015 · 8 comments
Assignees

Comments

@sometimesfood
Copy link
Contributor

According to rubocop's README hidden directories should not be searched by default:

Hidden directories (i.e., directories whose names start with a dot) are not searched by default.

However, when I upgraded to 0.29.1, rubocop started to include my .bundle directory by default.

Steps to reproduce:

cd $(mktemp -dt rubocop-XXXX)
cat > Gemfile <<EOS
source 'https://rubygems.org'
gem 'rubocop'
EOS
bundle install --path .bundle
bundle exec rubocop

For me, this results in Inspecting 29 files; all of them in .bundle. Is this the intended behaviour or should the README be updated?

@rrosenblum
Copy link
Contributor

I have found what the issue is here. Not all of the hidden files are being linted, only the special files that have been included by the .rubocop.yml or the default.yml file

# Common configuration.
AllCops:
  # Include gemspec and Rakefile
  Include:
    - '**/*.gemspec'
    - '**/*.podspec'
    - '**/*.jbuilder'
    - '**/*.rake'
    - '**/*.opal'
    - '**/Gemfile'
    - '**/Rakefile'
    - '**/Capfile'
    - '**/Guardfile'
    - '**/Podfile'
    - '**/Thorfile'
    - '**/Vagrantfile'
    - '**/Berksfile'
    - '**/Cheffile'
    - '**/Vagabondfile'
  Exclude:
    - 'vendor/**/*'

I have a few ideas on how to fix this, but this issue raises an interesting question on what configuration should be preferred, the configuration to skip hidden files, or the configuration of files to include. I would like the input of @bbatsov and @jonas054 as to what configuration should be preferred.

@rrosenblum
Copy link
Contributor

After thinking about this for a little bit, I am hesitant to call this a bug. RuboCop is doing what it is designed to do by including those files for linting. The typical place, that I know of, to install gems to is vendor/bundle. If the gems were installed there, this would not be a problem. Alternatively, @sometimesfood, you can update your .rubocop.yml file have:

Exclude:
  - '.bundle/**/*'

Again though, I would like to hear back from someone else before making any real calls on this issue.

@sometimesfood
Copy link
Contributor Author

Thanks for the update, Ryan. I hadn't noticed that only “included” files are checked.

After thinking about this for a little bit, I am hesitant to call this a bug. […] Alternatively, @sometimesfood, you can update your .rubocop.yml file

Thanks, I already did that yesterday. However, even if rubocop's behaviour remains as it currently is, this should be noted in the documentation somehow.

@sometimesfood
Copy link
Contributor Author

Btw, rubocop's behaviour in such cases seems to have changed between 0.28.0 and 0.29.1. I first encountered this issue after upgrading to 0.29.1. With 0.28.0 hidden directories were ignored completely.

@bquorning
Copy link
Contributor

It looks like this behavior was changed by #1618. Maybe a bug @jonas054?

@rrosenblum
Copy link
Contributor

From what I saw, there are no test in target_finder_spec.rb that handle hidden directories or files.

@jonas054
Copy link
Collaborator

Yes it does look like a bug. I'd be grateful for some help, since I am on vacation until March 11 without the means to provide a fix myself.

@bquorning
Copy link
Contributor

There is a test in cli_spec.rb that handles hidden directories, and can be changed to test this scenario:

diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb
index 38df046..7df42af 100644
--- a/spec/rubocop/cli_spec.rb
+++ b/spec/rubocop/cli_spec.rb
@@ -2212,9 +2212,11 @@ describe RuboCop::CLI, :isolated_environment do
       create_file('regexp', 'x=0')
       create_file('.dot1/file.rb', 'x=0') # Hidden but explicitly included
       create_file('.dot2/file.rb', 'x=0') # Hidden, excluded by default
+      create_file('.dot3/file.rake', 'x=0') # Hidden, not included by wildcard
       create_file('.rubocop.yml', ['AllCops:',
                                    '  Include:',
                                    '    - example',
+                                   '    - "**/*.rake"',
                                    '    - !ruby/regexp /regexp$/',
                                    '    - .dot1/**/*'
                                   ])

I am pretty sure that the bug is found in either Config#file_to_include? or PathUtil#match_path?, but I cannot figure it out at the moment.

bbatsov added a commit that referenced this issue Mar 22, 2015
[Fix #1656] Avoid unwanted matching of hidden directories
koic added a commit to koic/rubocop that referenced this issue Aug 16, 2018
Fixes rubocop#5181.
Fixes rubocop#4832.

This PR changes the path pattern (`*`) to match the hidden file.

This PR resolves the following problem.

```console
AllCops:
  Exclude:
    - 'vendor/bundle/**/*'
```

The above .rubocop.yml setting is expected to cover
hidden files as well. However, there is a problem that
offenses occur with hidden files.

```console
vendor/bundle/ruby/2.4.0/gems/backports-3.6.8/.irbrc:1:1: C:
Style/SpecialGlobalVars: Prefer $LOAD_PATH over $:.
$:.unshift "./lib"
^^

(snip)
```

This PR solves this problem by including hidden files in
the path match target.

And This PR only changes behavior to hidden files.

As resolved in rubocop#1401, rubocop#1656, hidden directories keep
their previous behavior.
So hidden files in the hidden directory don't match.

Probably it will be an intuitive path match to users.
bbatsov pushed a commit that referenced this issue Aug 16, 2018
Fixes #5181.
Fixes #4832.

This PR changes the path pattern (`*`) to match the hidden file.

This PR resolves the following problem.

```console
AllCops:
  Exclude:
    - 'vendor/bundle/**/*'
```

The above .rubocop.yml setting is expected to cover
hidden files as well. However, there is a problem that
offenses occur with hidden files.

```console
vendor/bundle/ruby/2.4.0/gems/backports-3.6.8/.irbrc:1:1: C:
Style/SpecialGlobalVars: Prefer $LOAD_PATH over $:.
$:.unshift "./lib"
^^

(snip)
```

This PR solves this problem by including hidden files in
the path match target.

And This PR only changes behavior to hidden files.

As resolved in #1401, #1656, hidden directories keep
their previous behavior.
So hidden files in the hidden directory don't match.

Probably it will be an intuitive path match to users.
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

No branches or pull requests

4 participants