From 3a814321491e1193b481dbf19742d7d9757e1b5b Mon Sep 17 00:00:00 2001 From: Kelly Norton Date: Tue, 24 Sep 2024 06:48:13 -0400 Subject: [PATCH 1/5] Add optional filename and line number information to patterns --- CHANGELOG.md | 4 + src/Parser.php | 17 ++-- src/Pattern.php | 10 ++- src/SourceInfo.php | 28 +++++++ tests/ParserTest.php | 185 ++++++++++++++++++++++++++++++++++++++----- 5 files changed, 214 insertions(+), 30 deletions(-) create mode 100644 src/SourceInfo.php diff --git a/CHANGELOG.md b/CHANGELOG.md index b6bb1c1..238d526 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.2.3] - 2024-09-24 +### Changed +- Add optional source and line number information to patterns - TBD + ## [2.2.2] - 2024-07-10 ### Changed - Update deprecated string interpolation usage - [#33](https://github.com/timoschinkel/codeowners/pull/33) by kchung diff --git a/src/Parser.php b/src/Parser.php index c57fb36..adc0079 100644 --- a/src/Parser.php +++ b/src/Parser.php @@ -15,7 +15,7 @@ final class Parser implements ParserInterface */ public function parseFile(string $file): array { - return $this->parseIterable($this->getFileIterable($file)); + return $this->parseIterable($this->getFileIterable($file), $file); } /** @@ -23,22 +23,23 @@ public function parseFile(string $file): array * @return Pattern[] * @throws UnableToParseException */ - public function parseString(string $lines): array + public function parseString(string $lines, ?string $filename = ''): array { - return $this->parseIterable(explode(PHP_EOL, $lines)); + return $this->parseIterable(explode(PHP_EOL, $lines), $filename); } /** * @param iterable $lines * @return Pattern[] */ - private function parseIterable(iterable $lines): array + private function parseIterable(iterable $lines, ?string $filename = null): array { $patterns = []; - foreach ($lines as $line) { + foreach ($lines as $index => $line) { $line = trim($line); - $pattern = $this->parseLine($line); + $sourcInfo = $filename !== null ? new SourceInfo($filename, $index + 1) : null; + $pattern = $this->parseLine($line, $sourcInfo); if ($pattern instanceof Pattern) { $patterns[] = $pattern; @@ -48,7 +49,7 @@ private function parseIterable(iterable $lines): array return $patterns; } - private function parseLine(string $line): ?Pattern + private function parseLine(string $line, ?SourceInfo $sourceInfo): ?Pattern { $line = trim($line); @@ -64,7 +65,7 @@ private function parseLine(string $line): ?Pattern if (preg_match('/^(?P[^\s]+)\s+(?P[^#]+)/si', $line, $matches) !== 0) { $owners = preg_split('/\s+/', trim($matches['owners'])); - return new Pattern($matches['file_pattern'], $owners); + return new Pattern($matches['file_pattern'], $owners, $sourceInfo); } return null; diff --git a/src/Pattern.php b/src/Pattern.php index e5a292d..8ed6d30 100644 --- a/src/Pattern.php +++ b/src/Pattern.php @@ -12,10 +12,13 @@ final class Pattern /** @var string[] */ private $owners; - public function __construct(string $pattern, array $owners) + private ?SourceInfo $sourceInfo; + + public function __construct(string $pattern, array $owners, ?SourceInfo $sourceInfo = null) { $this->pattern = $pattern; $this->owners = $owners; + $this->sourceInfo = $sourceInfo; } public function getPattern(): string @@ -27,4 +30,9 @@ public function getOwners(): array { return $this->owners; } + + public function getSourceInfo(): ?SourceInfo + { + return $this->sourceInfo; + } } diff --git a/src/SourceInfo.php b/src/SourceInfo.php new file mode 100644 index 0000000..aec6b77 --- /dev/null +++ b/src/SourceInfo.php @@ -0,0 +1,28 @@ +filename = $filename; + $this->line = $line; + } + + public function getFilename(): string + { + return $this->filename; + } + + public function getLine(): int + { + return $this->line; + } +} \ No newline at end of file diff --git a/tests/ParserTest.php b/tests/ParserTest.php index 9aeca64..86193c5 100644 --- a/tests/ParserTest.php +++ b/tests/ParserTest.php @@ -7,6 +7,7 @@ use CodeOwners\Exception\UnableToParseException; use CodeOwners\Parser; use CodeOwners\Pattern; +use CodeOwners\SourceInfo; use CodeOwners\Tests\Fixtures\FileOperations; use PHPUnit\Framework\TestCase; @@ -41,19 +42,60 @@ public function testParsingNonOpenableFileThrowsException() public function testParsingResultsInPatterns() { - $patterns = (new Parser())->parseFile(__DIR__ . '/Fixtures/CODEOWNERS.example'); + $filename = __DIR__ . '/Fixtures/CODEOWNERS.example'; + $patterns = (new Parser())->parseFile($filename); $this->assertEquals([ - new Pattern('*', ['@global-owner1', '@global-owner2']), - new Pattern('*.js', ['@js-owner']), - new Pattern('*.go', ['docs@example.com']), - new Pattern('/build/logs/', ['@doctocat']), - new Pattern('docs/*', ['docs@example.com']), - new Pattern('apps/', ['@octocat']), - new Pattern('/docs/', ['@doctocat']), - new Pattern('**/foo', ['@doctocat']), - new Pattern('abc/**', ['@doctocat']), - new Pattern('a/**/b', ['@doctocat']), + new Pattern( + '*', + ['@global-owner1', '@global-owner2'], + new SourceInfo($filename, 10) + ), + new Pattern( + '*.js', + ['@js-owner'], + new SourceInfo($filename, 16) + ), + new Pattern( + '*.go', + ['docs@example.com'], + new SourceInfo($filename, 21) + ), + new Pattern( + '/build/logs/', + ['@doctocat'], + new SourceInfo($filename, 26) + ), + new Pattern( + 'docs/*', + ['docs@example.com'], + new SourceInfo($filename, 31) + ), + new Pattern( + 'apps/', + ['@octocat'], + new SourceInfo($filename, 35) + ), + new Pattern( + '/docs/', + ['@doctocat'], + new SourceInfo($filename, 39) + ), + new Pattern( + '**/foo', + ['@doctocat'], + new SourceInfo($filename, 45) + ), + new Pattern( + 'abc/**', + ['@doctocat'], + new SourceInfo($filename, 50) + ), + new Pattern( + 'a/**/b', + ['@doctocat'], + new SourceInfo($filename, 55) + ), ], $patterns); } @@ -62,16 +104,117 @@ public function testParsingStringResultsInPatterns() $patterns = (new Parser())->parseString(file_get_contents(__DIR__ . '/Fixtures/CODEOWNERS.example')); $this->assertEquals([ - new Pattern('*', ['@global-owner1', '@global-owner2']), - new Pattern('*.js', ['@js-owner']), - new Pattern('*.go', ['docs@example.com']), - new Pattern('/build/logs/', ['@doctocat']), - new Pattern('docs/*', ['docs@example.com']), - new Pattern('apps/', ['@octocat']), - new Pattern('/docs/', ['@doctocat']), - new Pattern('**/foo', ['@doctocat']), - new Pattern('abc/**', ['@doctocat']), - new Pattern('a/**/b', ['@doctocat']), + new Pattern( + '*', + ['@global-owner1', '@global-owner2'], + new SourceInfo('', 10) + ), + new Pattern( + '*.js', + ['@js-owner'], + new SourceInfo('', 16) + ), + new Pattern( + '*.go', + ['docs@example.com'], + new SourceInfo('', 21) + ), + new Pattern( + '/build/logs/', + ['@doctocat'], + new SourceInfo('', 26) + ), + new Pattern( + 'docs/*', + ['docs@example.com'], + new SourceInfo('', 31) + ), + new Pattern( + 'apps/', + ['@octocat'], + new SourceInfo('', 35) + ), + new Pattern( + '/docs/', + ['@doctocat'], + new SourceInfo('', 39) + ), + new Pattern( + '**/foo', + ['@doctocat'], + new SourceInfo('', 45) + ), + new Pattern( + 'abc/**', + ['@doctocat'], + new SourceInfo('', 50) + ), + new Pattern( + 'a/**/b', + ['@doctocat'], + new SourceInfo('', 55) + ), + ], $patterns); + } + + public function testParsingStringWithOptionalFilename() + { + $patterns = (new Parser())->parseString( + file_get_contents(__DIR__ . '/Fixtures/CODEOWNERS.example'), + 'anonymous' + ); + + $this->assertEquals([ + new Pattern( + '*', + ['@global-owner1', '@global-owner2'], + new SourceInfo('anonymous', 10) + ), + new Pattern( + '*.js', + ['@js-owner'], + new SourceInfo('anonymous', 16) + ), + new Pattern( + '*.go', + ['docs@example.com'], + new SourceInfo('anonymous', 21) + ), + new Pattern( + '/build/logs/', + ['@doctocat'], + new SourceInfo('anonymous', 26) + ), + new Pattern( + 'docs/*', + ['docs@example.com'], + new SourceInfo('anonymous', 31) + ), + new Pattern( + 'apps/', + ['@octocat'], + new SourceInfo('anonymous', 35) + ), + new Pattern( + '/docs/', + ['@doctocat'], + new SourceInfo('anonymous', 39) + ), + new Pattern( + '**/foo', + ['@doctocat'], + new SourceInfo('anonymous', 45) + ), + new Pattern( + 'abc/**', + ['@doctocat'], + new SourceInfo('anonymous', 50) + ), + new Pattern( + 'a/**/b', + ['@doctocat'], + new SourceInfo('anonymous', 55) + ), ], $patterns); } } From 62885c0f44998626b50bf8facc86144ee4589b39 Mon Sep 17 00:00:00 2001 From: Kelly Norton Date: Tue, 24 Sep 2024 06:59:21 -0400 Subject: [PATCH 2/5] update CHANGELOG --- CHANGELOG.md | 2 +- src/SourceInfo.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 238d526..cadf26f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [2.2.3] - 2024-09-24 ### Changed -- Add optional source and line number information to patterns - TBD +- Add optional source and line number information to patterns - [#34](https://github.com/timoschinkel/codeowners/pull/34) ## [2.2.2] - 2024-07-10 ### Changed diff --git a/src/SourceInfo.php b/src/SourceInfo.php index aec6b77..5246bd4 100644 --- a/src/SourceInfo.php +++ b/src/SourceInfo.php @@ -25,4 +25,4 @@ public function getLine(): int { return $this->line; } -} \ No newline at end of file +} From 55379a5ebea10649c6ae27e5b78c8638570a4363 Mon Sep 17 00:00:00 2001 From: Kelly Norton Date: Tue, 24 Sep 2024 10:01:36 -0400 Subject: [PATCH 3/5] Fix a typo in a variable name --- src/Parser.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Parser.php b/src/Parser.php index adc0079..191626c 100644 --- a/src/Parser.php +++ b/src/Parser.php @@ -38,8 +38,8 @@ private function parseIterable(iterable $lines, ?string $filename = null): array foreach ($lines as $index => $line) { $line = trim($line); - $sourcInfo = $filename !== null ? new SourceInfo($filename, $index + 1) : null; - $pattern = $this->parseLine($line, $sourcInfo); + $sourceInfo = $filename !== null ? new SourceInfo($filename, $index + 1) : null; + $pattern = $this->parseLine($line, $sourceInfo); if ($pattern instanceof Pattern) { $patterns[] = $pattern; From 8988ae7bf339e35f4eb3b05cd3eb9a1e5e0563ef Mon Sep 17 00:00:00 2001 From: Kelly Norton Date: Fri, 27 Sep 2024 22:17:00 -0400 Subject: [PATCH 4/5] Review feedback 1. Increment minor semvar version. 2. Make filename property nullable for SourceInfo. 3. Parser::parseString($filename) includes SourceInfo on patterns but w/ null filename. --- CHANGELOG.md | 2 +- src/Parser.php | 7 ++++--- src/SourceInfo.php | 6 +++--- tests/ParserTest.php | 12 ++++++------ 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cadf26f..ad6f863 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [2.2.3] - 2024-09-24 +## [2.3.0] - 2024-09-24 ### Changed - Add optional source and line number information to patterns - [#34](https://github.com/timoschinkel/codeowners/pull/34) diff --git a/src/Parser.php b/src/Parser.php index 191626c..2415198 100644 --- a/src/Parser.php +++ b/src/Parser.php @@ -20,16 +20,18 @@ public function parseFile(string $file): array /** * @param string $lines + * @param ?string $filename * @return Pattern[] * @throws UnableToParseException */ - public function parseString(string $lines, ?string $filename = ''): array + public function parseString(string $lines, ?string $filename = null): array { return $this->parseIterable(explode(PHP_EOL, $lines), $filename); } /** * @param iterable $lines + * @param ?string $filename * @return Pattern[] */ private function parseIterable(iterable $lines, ?string $filename = null): array @@ -38,8 +40,7 @@ private function parseIterable(iterable $lines, ?string $filename = null): array foreach ($lines as $index => $line) { $line = trim($line); - $sourceInfo = $filename !== null ? new SourceInfo($filename, $index + 1) : null; - $pattern = $this->parseLine($line, $sourceInfo); + $pattern = $this->parseLine($line, new SourceInfo($filename, $index + 1)); if ($pattern instanceof Pattern) { $patterns[] = $pattern; diff --git a/src/SourceInfo.php b/src/SourceInfo.php index 5246bd4..6cc264b 100644 --- a/src/SourceInfo.php +++ b/src/SourceInfo.php @@ -4,19 +4,19 @@ final class SourceInfo { - private string $filename; + private ?string $filename; private int $line; public function __construct( - string $filename, + ?string $filename, int $line ) { $this->filename = $filename; $this->line = $line; } - public function getFilename(): string + public function getFilename(): ?string { return $this->filename; } diff --git a/tests/ParserTest.php b/tests/ParserTest.php index 86193c5..a0bcd9f 100644 --- a/tests/ParserTest.php +++ b/tests/ParserTest.php @@ -127,32 +127,32 @@ public function testParsingStringResultsInPatterns() new Pattern( 'docs/*', ['docs@example.com'], - new SourceInfo('', 31) + new SourceInfo(null, 31) ), new Pattern( 'apps/', ['@octocat'], - new SourceInfo('', 35) + new SourceInfo(null, 35) ), new Pattern( '/docs/', ['@doctocat'], - new SourceInfo('', 39) + new SourceInfo(null, 39) ), new Pattern( '**/foo', ['@doctocat'], - new SourceInfo('', 45) + new SourceInfo(null, 45) ), new Pattern( 'abc/**', ['@doctocat'], - new SourceInfo('', 50) + new SourceInfo(null, 50) ), new Pattern( 'a/**/b', ['@doctocat'], - new SourceInfo('', 55) + new SourceInfo(null, 55) ), ], $patterns); } From 16b5b09f6a02f288d07a09ec4d86fdc5035432b7 Mon Sep 17 00:00:00 2001 From: Kelly Norton Date: Wed, 2 Oct 2024 07:29:20 -0400 Subject: [PATCH 5/5] make SourceInfo nullable in Parser::parseLine --- src/Parser.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Parser.php b/src/Parser.php index 2415198..95a603a 100644 --- a/src/Parser.php +++ b/src/Parser.php @@ -50,7 +50,7 @@ private function parseIterable(iterable $lines, ?string $filename = null): array return $patterns; } - private function parseLine(string $line, ?SourceInfo $sourceInfo): ?Pattern + private function parseLine(string $line, SourceInfo $sourceInfo): ?Pattern { $line = trim($line);