From 56c4cd01432214bb9388de5e9e35ab81cc17dba7 Mon Sep 17 00:00:00 2001 From: Alexander Popov Date: Tue, 4 Dec 2018 14:18:28 +0300 Subject: [PATCH] [Fix #595] Exclude files ignored by `git` --- CHANGELOG.md | 4 ++ lib/rubocop/target_finder.rb | 38 +++++++++++++++---- manual/configuration.md | 1 + spec/rubocop/target_finder_spec.rb | 59 ++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1405c12cd15..1b19e73eaf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ * [#6570](https://github.com/rubocop-hq/rubocop/pull/6570): Fix a false positive for `Style/MethodCallWithArgsParentheses` `omit_parentheses` enforced style while splatting the result of a method invocation. ([@gsamokovarov][]) * [#6594](https://github.com/rubocop-hq/rubocop/pull/6594): Fix a false positive for `Rails/OutputSafety` when the receiver is a non-interpolated string literal. ([@amatsuda][]) +### Changes + +* [#595](https://github.com/rubocop-hq/rubocop/issues/595): Exclude files ignored by `git`. ([@AlexWayfer][]) + ## 0.61.1 (2018-12-06) ### Bug fixes diff --git a/lib/rubocop/target_finder.rb b/lib/rubocop/target_finder.rb index 2655ac71ce6..e9787260f86 100644 --- a/lib/rubocop/target_finder.rb +++ b/lib/rubocop/target_finder.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'set' +require 'open3' module RuboCop # This class finds target files to inspect by scanning the directory tree @@ -59,10 +60,13 @@ def target_files_in_dir(base_dir = Dir.pwd) end all_files = find_files(base_dir, File::FNM_DOTMATCH) hidden_files = Set.new(all_files - find_files(base_dir, 0)) + + git_files = ls_git_files(base_dir) + base_dir_config = @config_store.for(base_dir) target_files = all_files.select do |file| - to_inspect?(file, hidden_files, base_dir_config) + to_inspect?(file, git_files, hidden_files, base_dir_config) end # Most recently modified file first. @@ -71,13 +75,6 @@ def target_files_in_dir(base_dir = Dir.pwd) target_files end - def to_inspect?(file, hidden_files, base_dir_config) - return false if base_dir_config.file_to_exclude?(file) - return true if !hidden_files.include?(file) && ruby_file?(file) - - base_dir_config.file_to_include?(file) - end - # Search for files recursively starting at the given base directory using # the given flags that determine how the match is made. Excluded files will # be removed later by the caller, but as an optimization find_files removes @@ -100,6 +97,31 @@ def find_files(base_dir, flags) Dir.glob(pattern, flags).select { |path| FileTest.file?(path) } end + private + + def ls_git_files(base_dir) + return if `sh -c 'command -v git'`.empty? + + output, _error, status = Open3.capture3( + 'git', 'ls-files', '-z', base_dir, + '--exclude-standard', '--others', '--cached', '--modified' + ) + + return unless status.success? + + output.split("\0").map { |git_file| "#{base_dir}/#{git_file}" } + end + + def to_inspect?(file, git_files, hidden_files, base_dir_config) + return false if base_dir_config.file_to_exclude?(file) + + if !hidden_files.include?(file) && ruby_file?(file) + return git_files.nil? || git_files.include?(file) + end + + base_dir_config.file_to_include?(file) + end + def toplevel_dirs(base_dir, flags) Dir.glob(File.join(base_dir, '*'), flags).select do |dir| File.directory?(dir) && !dir.end_with?('/.', '/..') diff --git a/manual/configuration.md b/manual/configuration.md index aeb2ba9e877..8b079ef6d9d 100644 --- a/manual/configuration.md +++ b/manual/configuration.md @@ -229,6 +229,7 @@ ruby interpreters listed under `AllCops`/`RubyInterpreters` are inspected, unless the file also matches a pattern in `AllCops`/`Exclude`. Hidden directories (i.e., directories whose names start with a dot) are not searched by default. +Files ignored by Git are ignored by RuboCop by default. Here is an example that might be used for a Rails project: diff --git a/spec/rubocop/target_finder_spec.rb b/spec/rubocop/target_finder_spec.rb index 920c8db7c4d..1b4d5a79a15 100644 --- a/spec/rubocop/target_finder_spec.rb +++ b/spec/rubocop/target_finder_spec.rb @@ -398,6 +398,65 @@ expect(found_basenames).not_to include('ruby2.rb') end + context 'in git repo directory' do + before do + system 'git init' + + create_empty_file('git_regular.rb') + create_empty_file('git_group.rb') + create_empty_file('git_excluded.rb') + create_empty_file('git_glob.rb') + create_empty_file('git with spaces.rb') + create_empty_file('git_dir/thing.rb') + create_file('.gitignore', <<-'CONTENT'.strip_indent) + git_regular.rb + git_gr[ao]up.rb + !git_excluded.rb + git_glob* + git_dir/ + CONTENT + end + + it 'does not pick files ignored by git' do + expect(found_basenames).not_to include( + 'git_regular.rb', + 'git_group.rb', + 'git_glob.rb', + 'git_dir/thing.rb' + ) + expect(found_basenames).to include( + 'git_excluded.rb', + 'git with spaces.rb' + ) + end + + context 'without `git` executable' do + before do + allow_any_instance_of(Kernel).to receive(:`) + .with(/command -v git/) + .and_return('') + + allow_any_instance_of(Kernel).to receive(:`) + .with(start_with('git ls-files')) + .and_raise(Errno::ENOENT, 'git') + end + + it 'does not fail' do + expect { found_basenames }.not_to raise_error + end + end + + context 'with `*` in argument' do + let(:base_dir) { './git*' } + + it 'does not fail' do + expect(found_basenames).not_to include( + 'thing.rb' + ) + end + end + end + context 'when an exception is raised while reading file' do around do |example| original_stderr = $stderr