From 0884750da0af06084cf9e8740b4e6519c7e05509 Mon Sep 17 00:00:00 2001 From: sunjinhui Date: Fri, 17 Dec 2021 11:54:06 +0800 Subject: [PATCH] Applies with ruby glob --- Gemfile | 3 -- lib/codeowner_parser/rule.rb | 35 ++++++++++------- lib/codeowner_parser/version.rb | 2 +- spec/lib/codeowner_parser/file_spec.rb | 8 ++-- spec/lib/codeowner_parser/rule_spec.rb | 53 +++++++++++++++++++++++--- 5 files changed, 73 insertions(+), 28 deletions(-) diff --git a/Gemfile b/Gemfile index a24b8aa..f0e699f 100644 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,3 @@ gem 'rspec', '~> 3.9' gem 'rubocop', '~> 0.81.0' gem 'rubocop-performance', '1.5', require: false gem 'rubocop-rspec', '~> 1.38', require: false - -gem 'byebug' -gem 'pry-byebug' diff --git a/lib/codeowner_parser/rule.rb b/lib/codeowner_parser/rule.rb index 721696d..089f314 100644 --- a/lib/codeowner_parser/rule.rb +++ b/lib/codeowner_parser/rule.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true +require 'pathspec' module CodeownerParser # Handles an individual rule and determines applicability. @@ -8,12 +9,10 @@ module CodeownerParser def initialize(rule_string, line_num = nil) @rule_path, *@owner = rule_string.split(/\s+/) @line_num = line_num - # Precompute this so that we aren't doing so repeatedly for additional checks. - @rule_regex = path_regex end def applies?(path) - @rule_regex =~ path + match?(@rule_path, path) end def to_hash @@ -26,17 +25,25 @@ module CodeownerParser private - def path_regex - literals = @rule_path.split('*', -1) - regex = '' - # If path started with a slash, this is rooted. - regex += '\A' if literals.first.start_with?('/') - # Join with a wildcard for anything other than a slash. - regex += literals.map { |literal| Regexp.escape(literal) }.join('[^\/]+') - # If path did not end with a slash, do not search into subdirectories. - regex += '\Z' unless literals.last.end_with?('/') - - Regexp.compile(regex) + def match?(pattern, file) + pattern = normalize_pattern(pattern) + flags = match_flags_for(pattern) + + ::File.fnmatch(pattern, file, flags) + end + + def normalize_pattern(pattern) + pattern += "**" if pattern.end_with?(::File::SEPARATOR) + pattern = '**/' + pattern unless pattern.start_with?(::File::SEPARATOR) + pattern end + + # '/*'结尾 Wildcards in File.fnmatch and Dir.glob patterns do not match directory separators + def match_flags_for(pattern) + return ::File::FNM_PATHNAME if pattern.end_with?(::File::SEPARATOR + "*") + + 0 + end + end end diff --git a/lib/codeowner_parser/version.rb b/lib/codeowner_parser/version.rb index 91cdc97..998ffb8 100644 --- a/lib/codeowner_parser/version.rb +++ b/lib/codeowner_parser/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module CodeownerParser - VERSION = '1.0.1' + VERSION = '1.1.0' end diff --git a/spec/lib/codeowner_parser/file_spec.rb b/spec/lib/codeowner_parser/file_spec.rb index 900cb98..9d43f62 100644 --- a/spec/lib/codeowner_parser/file_spec.rb +++ b/spec/lib/codeowner_parser/file_spec.rb @@ -74,7 +74,7 @@ RSpec.describe CodeownerParser::File do end context 'with js file' do - let(:path) { 'blah.js' } + let(:path) { '/blah.js' } it { is_expected.to eq(['@js-owner']) } end @@ -183,13 +183,13 @@ RSpec.describe CodeownerParser::File do end context 'with js file' do - let(:path) { 'blah.js' } + let(:path) { '/blah.js' } it { is_expected.to eq({ line_num: 2, owners: ['@js-owner'], rule_regex: '*.js' }) } end context 'with no match' do - let(:path) { 'blah.hello' } + let(:path) { '/blah.hello' } it { is_expected.to eq(nil) } end @@ -204,7 +204,7 @@ RSpec.describe CodeownerParser::File do end context 'with js file' do - let(:path) { 'blah.js' } + let(:path) { '/blah.js' } it { is_expected.to eq({ line_num: 2, owners: [], rule_regex: '*.js' }) } end diff --git a/spec/lib/codeowner_parser/rule_spec.rb b/spec/lib/codeowner_parser/rule_spec.rb index e65a7dc..08f7ca3 100644 --- a/spec/lib/codeowner_parser/rule_spec.rb +++ b/spec/lib/codeowner_parser/rule_spec.rb @@ -18,10 +18,22 @@ RSpec.describe CodeownerParser::Rule do subject(:rule) { described_class.new(rule_string) } # Initial examples come from https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners + context 'with * wildcard and group owner' do + let(:rule_string) { '* @js-owner' } + + it { is_expected.to apply_to('blah.js') } + it { is_expected.to apply_to('a/blah.js') } + it { is_expected.to apply_to('/docs/a/blah.js') } + it { is_expected.to have_owner(['@js-owner']) } + end + + context 'with js extension and group owner' do let(:rule_string) { '*.js @js-owner' } - it { is_expected.to apply_to('blah.js') } + it { is_expected.to apply_to('/blah.js') } + it { is_expected.to apply_to('/a/blah.js') } + it { is_expected.to apply_to('/a/blah.js') } it { is_expected.not_to apply_to('blah.rb') } it { is_expected.to have_owner(['@js-owner']) } end @@ -29,8 +41,8 @@ RSpec.describe CodeownerParser::Rule do context 'with go extension and email owner' do let(:rule_string) { '*.go docs@example.com' } - it { is_expected.to apply_to('blah.go') } - it { is_expected.not_to apply_to('blah.rb') } + it { is_expected.to apply_to('/blah.go') } + it { is_expected.not_to apply_to('/blah.rb') } it { is_expected.to have_owner(['docs@example.com']) } end @@ -38,6 +50,7 @@ RSpec.describe CodeownerParser::Rule do let(:rule_string) { '/build/logs/ @doctocat' } it { is_expected.to apply_to('/build/logs/blah.go') } + it { is_expected.not_to apply_to('build/logs/blah.go') } it { is_expected.not_to apply_to('/logs/blah.go') } it { is_expected.not_to apply_to('/tmp/build/logs/blah.go') } it { is_expected.not_to apply_to('blah.rb') } @@ -55,11 +68,14 @@ RSpec.describe CodeownerParser::Rule do it { is_expected.to have_owner(['@doctocat']) } end + # 匹配子目录？？ context 'with specific unrooted folder' do let(:rule_string) { 'docs/* docs@example.com' } - it { is_expected.to apply_to('/build/docs/blah.go') } it { is_expected.to apply_to('docs/blah.go') } + it { is_expected.to apply_to('/build/docs/blah.go') } + it { is_expected.not_to apply_to('docs/aaa/blah.go') } + it { is_expected.not_to apply_to('/docs/aaa/blah.go') } it { is_expected.not_to apply_to('docs/tmp/blah.go') } it { is_expected.not_to apply_to('/build/docs/other/blah.go') } it { is_expected.not_to apply_to('/logs/blah.go') } @@ -70,19 +86,32 @@ RSpec.describe CodeownerParser::Rule do context 'with entire unrooted folder' do let(:rule_string) { 'apps/ @octocat' } - it { is_expected.to apply_to('apps/blah.go') } - it { is_expected.to apply_to('apps/blah/blah.go') } + it { is_expected.to apply_to('/apps/blah.go') } + it { is_expected.to apply_to('/apps/blah/blah.go') } + it { is_expected.to apply_to('/apps/blah/blah.go') } + it { is_expected.not_to apply_to('/logs/blah.go') } + it { is_expected.not_to apply_to('/blah.rb') } + it { is_expected.to have_owner(['@octocat']) } + end + + context 'with entire unrooted folder' do + let(:rule_string) { 'apps/** @octocat' } + + it { is_expected.to apply_to('/apps/blah.go') } it { is_expected.to apply_to('/apps/blah/blah.go') } it { is_expected.not_to apply_to('/logs/blah.go') } it { is_expected.not_to apply_to('blah.rb') } it { is_expected.to have_owner(['@octocat']) } end + context 'with entire rooted folder' do let(:rule_string) { '/docs/ @doctocat' } it { is_expected.to apply_to('/docs/blah.go') } it { is_expected.to apply_to('/docs/blah/blah.go') } + it { is_expected.not_to apply_to('/helper/docs/blah/blah.go') } + it { is_expected.not_to apply_to('helper/docs/blah/blah.go') } it { is_expected.not_to apply_to('/help/docs/blah.go') } it { is_expected.not_to apply_to('/logs/blah.go') } it { is_expected.not_to apply_to('blah.rb') } @@ -105,4 +134,16 @@ RSpec.describe CodeownerParser::Rule do it { is_expected.to have_owner(['@doctocat', '@doctor']) } end + + + context 'with directory anywhere in the project' do + let(:rule_string) { '[a,b]*.rb @doctocat' } + + it { is_expected.to apply_to('/b.rb') } + it { is_expected.to apply_to('/a.rb') } + it { is_expected.to apply_to('/ab.rb') } + it { is_expected.not_to apply_to('/c.rb') } + it { is_expected.to have_owner(['@doctocat']) } + end + end -- Gitee