AllCops > Include parameter isn't behaving as expected

[johnnyshields]

Given a very simple .rubocop.yml in my project root:

AllCops:
  Include:
    - lib/**/*

I expect that running $project-root/>rubocop on the command line should only scan files in the root lib dir, and exclude all else. Currently, this seems to be picking up files outside lib, even some files that don't have lib at all in their path. Please try it on a few projects you should get the same result.

[jonas054]

I have not been able to reproduce. Tried on your branch in bson-ruby (changing back to Include) and only got files under lib/, as expected.

add-static-code-analyzer ~/dev/mongodb/bson-ruby$ git status
On branch add-static-code-analyzer
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   .rubocop.yml

no changes added to commit (use "git add" and/or "git commit -a")
add-static-code-analyzer ~/dev/mongodb/bson-ruby$ git diff
diff --git a/.rubocop.yml b/.rubocop.yml
index 205ea15..80bea7f 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -1,12 +1,8 @@
 AllCops:
   TargetRubyVersion: 2.5
   NewCops: disable
-  Exclude:
-    # Workaround to exclude all files except lib dir
-    # Pending https://github.com/rubocop/rubocop/issues/10802
-    <% Dir.glob('**/*').grep_v(/^lib\//).each do |path| %>
-    - <%= path %>
-    <% end %>
+  Include:
+    - lib/**/*
 
 Bundler:
   Enabled: false
add-static-code-analyzer ~/dev/mongodb/bson-ruby$ rubocop --list-target-files
lib/bson.rb
lib/bson/active_support.rb
lib/bson/array.rb
lib/bson/big_decimal.rb
lib/bson/binary.rb
lib/bson/boolean.rb
lib/bson/code.rb
lib/bson/code_with_scope.rb
lib/bson/config.rb
lib/bson/date.rb
lib/bson/date_time.rb
lib/bson/db_pointer.rb
lib/bson/dbref.rb
lib/bson/decimal128.rb
lib/bson/decimal128/builder.rb
lib/bson/document.rb
lib/bson/environment.rb
lib/bson/error.rb
lib/bson/ext_json.rb
lib/bson/false_class.rb
lib/bson/float.rb
lib/bson/hash.rb
lib/bson/int32.rb
lib/bson/int64.rb
lib/bson/integer.rb
lib/bson/json.rb
lib/bson/max_key.rb
lib/bson/min_key.rb
lib/bson/nil_class.rb
lib/bson/object.rb
lib/bson/object_id.rb
lib/bson/open_struct.rb
lib/bson/regexp.rb
lib/bson/registry.rb
lib/bson/specialized.rb
lib/bson/string.rb
lib/bson/symbol.rb
lib/bson/time.rb
lib/bson/time_with_zone.rb
lib/bson/timestamp.rb
lib/bson/true_class.rb
lib/bson/undefined.rb
lib/bson/version.rb

[johnnyshields]

@jonas054 in bson-ruby, try running submodule init / submodule update and then run rubocop -L again. The issue may be related to submodules.

[jonas054]

Ah, OK. With the submodule checked out, I get inspection of 3 additional files:

• spec/shared/bin/get-mongodb-download-url
• spec/shared/bin/s3-copy
• spec/shared/bin/s3-upload

Do you get the same? Because those are executables and they get inspected even though they're not matched by the Include pattern. See https://docs.rubocop.org/rubocop/1.31/configuration.html#includingexcluding-files . I'm not sure why RuboCop does this, but it's been like that for a really long time. You have to use Exclude if you don't want to inspect the binaries.

[johnnyshields]

@jonas054 yes, I get the same.

I'm not sure why RuboCop does this, but it's been like that for a really long time

The point of this ticket is to fix this behavior, even though Rubocop has been this way for a long time. Include should be strict, otherwise it's just incorrect.

[jonas054]

I've realized that there is a good reason for this behavior. You can't specify extensionless executable ruby files with a pattern. That's why they have to be included by default.

[johnnyshields]

@jonas054 I don't see why that's a good reason. We should include extensionless Ruby files only if they satisfy the pattern.

For example, given Include: - lib/**/* :

• a file named spec/shared/bin/get-mongodb-download-url should not be included
• a file named lib/shared/bin/get-mongodb-download-url should be included

Would this be fine to change the behavior?

[jonas054]

The problem is that including fewer files by default would be a breaking change. We would either have to wait until RuboCop 2.0 to introduce it, or come up with some backwards compatible way to achieve the same thing.

I guess it would be possible to add a configuration parameter AllCops: IncludeExtensionlessExecutable or something. It would be true by default. You would then set it to false to only inspect the files matching the Include patterns.

The question is, would we have to support the new parameter for departments and cops too? Could get tricky.

[johnnyshields]

I think your proposal above is good. AllCops: IncludeExtensionlessExecutables should be a single global which affects the behavior of Include both AllCops and individual cops. Fine have it be true for now and change to false in Rubocop 2.0.

OK to raise a PR for this?

[jonas054]

I'm OK with that, if you want to do it. I'll review but leave it to someone else to merge, so we get more eyes on it.

[dvandersluis]

For context, the ability to scan extensionless executables was added in #58. I'm not exactly sure why these executables don't operate under the same rules of Include as everything else, but it probably shouldn't be the case. I agree that they should respect Include by default (or totally) in RuboCop 2.0.

I agree with @jonas054 that adding a configuration (defaulting to match existing behaviour) would be a good migration path. I've been trying to come up with a better name but this is a hard concept to name succinctly... perhaps IgnoreUnincludedExecutables? It doesn't help that we have both Include and Exclude 😖

In any case, the documentation should be updated to better reflect the current behaviour.

[p-mongo]

To clarify the requirements expressed in mongodb/bson-ruby#284: the request is for rubocop to be configured to check a specific subdirectory of the project only (in this case lib) even though there may be other lib subdirs elsewhere under the project root (e.g. foo/lib). It is not clear to me what the binary situation is being agreed on but extended to binaries, the request would be e.g. to check top-level bin and lib subdirs in a project only and ignore foo/bin and foo/lib.