diff --git a/.changelog/git-rule-precedence.md b/.changelog/git-rule-precedence.md new file mode 100644 index 0000000000..a8dafc6860 --- /dev/null +++ b/.changelog/git-rule-precedence.md @@ -0,0 +1,6 @@ +--- +category: fixed +type: patch +--- + +Preserve consumer rule ordering and overrides when synchronizing `.gitignore` and `.gitattributes`, including repeated patterns, export exceptions, and significant ignore-pattern whitespace. Add generated defaults before existing rules without reordering consumer exceptions. diff --git a/docs/commands/gitattributes.rst b/docs/commands/gitattributes.rst index 09e0a108b6..ff1e6a82d7 100644 --- a/docs/commands/gitattributes.rst +++ b/docs/commands/gitattributes.rst @@ -68,7 +68,12 @@ Behavior paths in exported archives. - Respects ``extra.gitattributes.no-export-ignore`` as an alias. - Preserves existing custom ``.gitattributes`` rules. -- Deduplicates equivalent entries and sorts them (directories before files, then alphabetically). +- Missing generated exclusions are prepended as defaults. Existing rules retain + order and repetitions, so later consumer ``-export-ignore`` or + ``!export-ignore`` overrides remain effective. No-op and repeated merges retain + the same effective attributes. +- Explicit keep-in-export paths still remove matching simple positive exclusions; + they do not rewrite arbitrary compound attribute rules or glob exceptions. - Uses CandidateProvider, ExistenceChecker, ExportIgnoreFilter, Merger, Reader, and Writer components from the GitAttributes namespace. - ``--dry-run`` and ``--check`` render a diff against the normalized managed diff --git a/docs/commands/gitignore.rst b/docs/commands/gitignore.rst index 37de349cc9..b7aba85f39 100644 --- a/docs/commands/gitignore.rst +++ b/docs/commands/gitignore.rst @@ -77,7 +77,12 @@ Behavior - Reads the canonical .gitignore from dev-tools and merges with the project's. - By default, the source is the packaged .gitignore and the target is the project's root. -- Duplicates are removed and entries are sorted alphabetically. +- Canonical defaults precede consumer rules, so the last matching consumer rule + keeps Git precedence. Existing order, repetitions and pattern whitespace are + preserved; patterns are not sorted or globally deduplicated. +- An already present canonical prefix is not prepended again, making repeated + synchronization idempotent. Changed defaults are added before existing rules; + synchronization does not silently remove historical consumer rules. - Uses the Reader, Merger, and Writer components from the GitIgnore namespace. - ``--dry-run`` and ``--check`` render a diff against the normalized managed result before deciding whether to write. diff --git a/src/Console/Command/GitIgnoreCommand.php b/src/Console/Command/GitIgnoreCommand.php index 54d4199990..bf2c79dd46 100644 --- a/src/Console/Command/GitIgnoreCommand.php +++ b/src/Console/Command/GitIgnoreCommand.php @@ -40,7 +40,7 @@ * Provides functionality to merge and synchronize .gitignore files. * * This command merges the canonical .gitignore from dev-tools with the project's - * existing .gitignore, removing duplicates and sorting entries. + * existing .gitignore, preserving ordered consumer overrides. * * The command accepts two options: --source and --target to specify the paths * to the canonical and project .gitignore files respectively. diff --git a/src/GitAttributes/Merger.php b/src/GitAttributes/Merger.php index ce6bfcae8a..255d8c327f 100644 --- a/src/GitAttributes/Merger.php +++ b/src/GitAttributes/Merger.php @@ -27,9 +27,8 @@ * Merges .gitattributes content with generated export-ignore rules. * * This class preserves existing custom entries while adding missing - * export-ignore rules for known candidate paths, deduplicates semantically - * equivalent entries, and sorts export-ignore rules with directories before - * files. + * export-ignore defaults before consumer rules. Existing rules MUST retain their + * order and repetitions so later unset and unspecified attributes keep precedence. */ final class Merger implements MergerInterface { @@ -38,9 +37,9 @@ final class Merger implements MergerInterface * * This method: * 1. Preserves custom user-defined entries in their original order - * 2. Adds missing generated export-ignore entries for existing paths - * 3. Deduplicates entries using normalized path comparison - * 4. Sorts export-ignore entries with directories before files + * 2. Prepends missing generated defaults before consumer overrides + * 3. Omits generated duplicates without deduplicating existing rules + * 4. Removes simple positive rules for explicitly kept paths * * @param string $existingContent The raw .gitattributes content currently stored. * @param list $exportIgnoreEntries the export-ignore entries to manage @@ -50,90 +49,41 @@ final class Merger implements MergerInterface */ public function merge(string $existingContent, array $exportIgnoreEntries, array $keepInExportPaths = []): string { - $nonExportIgnoreLines = []; - $seenNonExportIgnoreLines = []; - $exportIgnoreLines = []; + $existingLines = []; + $existingRules = []; $keptExportLookup = $this->keepInExportLookup($keepInExportPaths); - $generatedDirectoryLookup = $this->generatedDirectoryLookup($exportIgnoreEntries); foreach ($this->parseExistingLines($existingContent) as $line) { - $normalizedLine = $this->normalizeLine($line); + $pathSpec = $this->extractExportIgnorePathSpec($line); - if ('' === $normalizedLine) { - continue; - } + if (null !== $pathSpec) { + $pathKey = $this->normalizePathKey($pathSpec); - $pathSpec = $this->extractExportIgnorePathSpec($normalizedLine); - - if (null === $pathSpec) { - if (isset($seenNonExportIgnoreLines[$normalizedLine])) { + if ($this->isKeptPath($pathKey, $keptExportLookup)) { continue; } - $nonExportIgnoreLines[] = $normalizedLine; - $seenNonExportIgnoreLines[$normalizedLine] = true; - - continue; - } - - $pathKey = $this->normalizePathKey($pathSpec); - if ($this->isKeptPath($pathKey, $keptExportLookup)) { - continue; - } - - if (isset($exportIgnoreLines[$pathKey])) { - continue; + $existingRules[$pathKey] = true; } - $exportIgnoreLines[$pathKey] = [ - 'line' => $normalizedLine, - 'sort_key' => $this->sortKey($pathSpec), - 'is_directory' => str_ends_with($pathSpec, '/') || isset($generatedDirectoryLookup[$pathKey]), - ]; + $existingLines[] = $line; } - foreach ($exportIgnoreEntries as $entry) { - $trimmedEntry = trim($entry); - $pathKey = $this->normalizePathKey($trimmedEntry); - - if ($this->isKeptPath($pathKey, $keptExportLookup)) { - continue; - } + $generatedLines = []; - if (! isset($exportIgnoreLines[$pathKey])) { - $exportIgnoreLines[$pathKey] = [ - 'line' => $trimmedEntry . ' export-ignore', - 'sort_key' => $this->sortKey($trimmedEntry), - 'is_directory' => str_ends_with($trimmedEntry, '/'), - ]; + foreach ($exportIgnoreEntries as $entry) { + $entry = trim($entry); + $pathKey = $this->normalizePathKey($entry); + if ('' === $entry || $this->isKeptPath($pathKey, $keptExportLookup) || isset($existingRules[$pathKey])) { continue; } - $exportIgnoreLines[$pathKey]['is_directory'] = $exportIgnoreLines[$pathKey]['is_directory'] - || str_ends_with($trimmedEntry, '/'); + $generatedLines[] = $entry . ' export-ignore'; + $existingRules[$pathKey] = true; } - $sortedExportIgnoreLines = array_values($exportIgnoreLines); - - usort( - $sortedExportIgnoreLines, - static function (array $left, array $right): int { - if ($left['is_directory'] !== $right['is_directory']) { - return $left['is_directory'] ? -1 : 1; - } - - $naturalOrder = strnatcasecmp($left['sort_key'], $right['sort_key']); - - if (0 !== $naturalOrder) { - return $naturalOrder; - } - - return strcmp($left['sort_key'], $right['sort_key']); - } - ); - - return implode("\n", [...$nonExportIgnoreLines, ...array_column($sortedExportIgnoreLines, 'line')]); + return implode("\n", [...$generatedLines, ...$existingLines]); } /** @@ -203,52 +153,6 @@ private function isKeptPath(string $pathKey, array $keptExportLookup): bool return false; } - /** - * Builds a lookup table of generated directory candidates. - * - * @param list $exportIgnoreEntries the generated export-ignore path list - * - * @return array the normalized directory lookup - */ - private function generatedDirectoryLookup(array $exportIgnoreEntries): array - { - $lookup = []; - - foreach ($exportIgnoreEntries as $entry) { - $trimmedEntry = trim($entry); - - if (! str_ends_with($trimmedEntry, '/')) { - continue; - } - - $lookup[$this->normalizePathKey($trimmedEntry)] = true; - } - - return $lookup; - } - - /** - * Normalizes a .gitattributes line for deterministic comparison and output. - * - * @param string $line the raw line to normalize - * - * @return string the normalized line - */ - private function normalizeLine(string $line): string - { - $trimmedLine = trim($line); - - if ('' === $trimmedLine) { - return ''; - } - - if (str_starts_with($trimmedLine, '#')) { - return $trimmedLine; - } - - return preg_replace('/(?normalizePathSpec($pathSpec), '/'); - } - /** * Normalizes a gitattributes path spec for sorting. * @@ -321,7 +213,7 @@ private function normalizePathKey(string $pathSpec): string return ltrim(rtrim($normalizedPathSpec, '/'), '/'); } - return $normalizedPathSpec; + return trim($pathSpec); } /** diff --git a/src/GitAttributes/MergerInterface.php b/src/GitAttributes/MergerInterface.php index 6d9b4f5970..a8f331677d 100644 --- a/src/GitAttributes/MergerInterface.php +++ b/src/GitAttributes/MergerInterface.php @@ -24,8 +24,8 @@ * * This interface defines the contract for managing .gitattributes files, * specifically handling the merging of canonical export-ignore rules with - * existing custom entries while removing obsolete generated markers and - * duplicate lines. + * existing custom entries. New defaults MUST precede consumer overrides; + * existing order and repeated rules MUST be retained. */ interface MergerInterface { diff --git a/src/GitIgnore/GitIgnore.php b/src/GitIgnore/GitIgnore.php index 0d4d295949..4257469afc 100644 --- a/src/GitIgnore/GitIgnore.php +++ b/src/GitIgnore/GitIgnore.php @@ -96,10 +96,10 @@ public static function fromFile(string $gitignorePath): self $entries = []; foreach ($file as $line) { - $trimmed = trim($line ?: ''); + $entry = false === $line ? '' : $line; - if ('' !== $trimmed) { - $entries[] = $trimmed; + if ('' !== trim($entry)) { + $entries[] = $entry; } } diff --git a/src/GitIgnore/Merger.php b/src/GitIgnore/Merger.php index 93ef4be1d4..62ebd7e4e4 100644 --- a/src/GitIgnore/Merger.php +++ b/src/GitIgnore/Merger.php @@ -20,82 +20,49 @@ namespace FastForward\DevTools\GitIgnore; /** - * Merges, deduplicates, and sorts .gitignore entries. + * Merges ordered .gitignore rules with canonical defaults before consumer overrides. * - * This service SHALL combine canonical and project-specific .gitignore - * definitions into a single normalized result. The resulting entry list MUST - * exclude blank lines and comment lines from the merged output, MUST remove - * duplicate entries, and MUST group directory entries before file entries. - * Directory and file groups SHALL be sorted independently in ascending string - * order to provide deterministic output. + * Existing patterns MUST retain their order, whitespace and repeated occurrences. */ final readonly class Merger implements MergerInterface { /** - * Initializes the merger with a classifier implementation. - * - * The classifier MUST be capable of determining whether a normalized - * .gitignore entry represents a directory or a file pattern. When no - * classifier is provided, a default Classifier instance SHALL be used. - * - * @param ClassifierInterface $classifier the classifier responsible for - * distinguishing directory entries - * from file entries during merging + * Retains the classifier constructor parameter for compatibility with consumers. */ public function __construct( private ClassifierInterface $classifier = new Classifier() ) {} /** - * Merges canonical and project .gitignore entries into a normalized result. - * - * The implementation MUST combine entries from both sources, MUST remove - * duplicates, and MUST ignore blank or commented entries after trimming. - * Entries identified as directories SHALL be collected separately from file - * entries. Each group MUST be sorted using string comparison, and directory - * entries MUST appear before file entries in the final result. - * - * The returned GitIgnore instance SHALL preserve the project path provided by - * the $project argument. + * Prepends canonical defaults once, allowing later consumer rules to override them. * - * @param GitIgnoreInterface $canonical The canonical .gitignore source whose - * entries provide the shared baseline. - * @param GitIgnoreInterface $project The project-specific .gitignore source - * whose path MUST be preserved in the - * merged result. - * - * @return GitIgnoreInterface A new merged .gitignore representation containing - * normalized, deduplicated, and ordered entries. + * @param GitIgnoreInterface $canonical the canonical baseline + * @param GitIgnoreInterface $project the consumer rules and destination path */ public function merge(GitIgnoreInterface $canonical, GitIgnoreInterface $project): GitIgnoreInterface { - $entries = array_unique(array_merge($canonical->entries(), $project->entries())); - - $directories = []; - $files = []; - - foreach ($entries as $entry) { - $trimmed = trim($entry); - if ('' === $trimmed) { - continue; - } - - if (str_starts_with($trimmed, '#')) { - continue; - } + $canonicalEntries = $this->patterns($canonical->entries()); + $projectEntries = $this->patterns($project->entries()); - if ($this->classifier->isDirectory($trimmed)) { - $directories[] = $trimmed; - } else { - $files[] = $trimmed; - } + if (array_slice($projectEntries, 0, count($canonicalEntries)) === $canonicalEntries) { + return new GitIgnore($project->path(), $projectEntries); } - sort($directories, \SORT_STRING); - sort($files, \SORT_STRING); - - $mergedEntries = array_merge($directories, $files); + return new GitIgnore($project->path(), [...$canonicalEntries, ...$projectEntries]); + } - return new GitIgnore($project->path(), array_values($mergedEntries)); + /** + * Removes only blank lines and comments, never normalizing pattern whitespace. + * + * @param list $entries the raw rules + * + * @return list + */ + private function patterns(array $entries): array + { + return array_values(array_filter( + $entries, + static fn(string $entry): bool => '' !== trim($entry) && ! str_starts_with($entry, '#'), + )); } } diff --git a/src/GitIgnore/MergerInterface.php b/src/GitIgnore/MergerInterface.php index e593d162ee..ca1aa72433 100644 --- a/src/GitIgnore/MergerInterface.php +++ b/src/GitIgnore/MergerInterface.php @@ -23,18 +23,16 @@ * Defines the contract for merging .gitignore entries. * * This service SHALL combine canonical and project-specific .gitignore - * definitions into a single normalized result. The resulting entry list MUST - * exclude blank lines and comment lines from the merged output, MUST remove - * duplicate entries, and MUST group directory entries before file entries. - * Directory and file groups SHALL be sorted independently in ascending string - * order to provide deterministic output. + * definitions into an ordered result. Canonical defaults MUST precede consumer + * rules. The merger MUST preserve pattern whitespace and repeated rules because + * Git resolves conflicts using the last matching pattern. */ interface MergerInterface { /** - * Merges two GitIgnore instances, removing duplicates and sorting entries. + * Merges two GitIgnore instances without changing consumer rule precedence. * - * Directories are placed before files in the resulting list. + * An already present canonical prefix MUST NOT be added again. * The path from $project is used in the returned instance. * * @param GitIgnoreInterface $canonical the canonical .gitignore from dev-tools diff --git a/tests/GitAttributes/MergerTest.php b/tests/GitAttributes/MergerTest.php index 8a6e108eb8..0a5ec2dd11 100644 --- a/tests/GitAttributes/MergerTest.php +++ b/tests/GitAttributes/MergerTest.php @@ -21,319 +21,120 @@ use FastForward\DevTools\GitAttributes\Merger; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -use ReflectionMethod; #[CoversClass(Merger::class)] final class MergerTest extends TestCase { - private Merger $merger; - - /** - * @return void - */ - protected function setUp(): void - { - $this->merger = new Merger(); - } - - /** - * @return void - */ - #[Test] - public function mergeWillCreateManagedBlockWhenFileIsEmpty(): void - { - $result = $this->merger->merge('', ['/docs/', '/README.md']); - - self::assertSame("/docs/ export-ignore\n" . '/README.md export-ignore', $result); - } - - /** - * @return void - */ - #[Test] - public function mergeWillPreserveCustomEntries(): void - { - $existingContent = implode("\n", ['*.zip -diff', '*.phar binary']); - - $result = $this->merger->merge($existingContent, ['/docs/']); - - self::assertSame("*.zip -diff\n*.phar binary\n" . '/docs/ export-ignore', $result); - } - /** * @return void */ #[Test] - public function mergeWillDeduplicateExistingAndGeneratedEntries(): void + public function mergeWillCreateMissingDefaultsInProvidedOrder(): void { - $existingContent = implode("\n", [ - '* text=auto', - '/.github/ export-ignore', - '/docs/ export-ignore', - '/tests/ export-ignore', - '/.gitattributes export-ignore', - '/.gitignore export-ignore', - '/.gitmodules export-ignore', - 'AGENTS.md export-ignore', - '/.github/ export-ignore', - '/.vscode/ export-ignore', - '/docs/ export-ignore', - '/tests/ export-ignore', - '/.editorconfig export-ignore', - '/.gitattributes export-ignore', - '/.gitignore export-ignore', - '/.gitmodules export-ignore', - '/README.md export-ignore', - ]); - - $result = $this->merger->merge($existingContent, [ - '/.github/', - '/.vscode/', - '/docs/', - '/tests/', - '/.editorconfig', - '/.gitattributes', - '/.gitignore', - '/.gitmodules', - '/README.md', - ]); - self::assertSame( - "* text=auto\n" - . "/.github/ export-ignore\n" - . "/.vscode/ export-ignore\n" - . "/docs/ export-ignore\n" - . "/tests/ export-ignore\n" - . "/.editorconfig export-ignore\n" - . "/.gitattributes export-ignore\n" - . "/.gitignore export-ignore\n" - . "/.gitmodules export-ignore\n" - . "AGENTS.md export-ignore\n" - . '/README.md export-ignore', - $result, + "/README.md export-ignore\n/docs/ export-ignore\n/tests/ export-ignore", + (new Merger())->merge('', ['/README.md', '/docs/', '/tests/', '/docs/']), ); } /** - * @return void - */ - #[Test] - public function mergeWillRemoveExistingExportIgnoreRulesForKeptPaths(): void - { - $existingContent = implode("\n", [ - '* text=auto', - '/.gitignore export-ignore', - 'AGENTS.md export-ignore', - '/README.md export-ignore', - ]); - - $result = $this->merger->merge($existingContent, ['/README.md'], ['/.gitignore', '/AGENTS.md']); - - self::assertSame("* text=auto\n" . '/README.md export-ignore', $result); - } - - /** - * @return void + * @param string $content */ #[Test] - public function mergeWillRemoveNestedExportIgnoreRulesWhenParentPathIsKept(): void + #[DataProvider('orderedRules')] + public function mergeWithoutGeneratedRulesPreservesConsumerPrecedence(string $content): void { - $existingContent = implode("\n", [ - '* text=auto', - '/.agents/agents/ export-ignore', - '/.agents/skills/ export-ignore', - '/tests/ export-ignore', - ]); + $merger = new Merger(); - $result = $this->merger->merge( - $existingContent, - ['/.agents/agents/', '/.agents/skills/', '/tests/'], - ['/.agents/'], - ); - - self::assertSame("* text=auto\n" . '/tests/ export-ignore', $result); + self::assertSame($content, $merger->merge($content, [])); + self::assertSame($content, $merger->merge($merger->merge($content, []), [])); } /** - * @return void + * @return iterable */ - #[Test] - public function mergeWillSortExportIgnoreEntriesWithDirectoriesBeforeFiles(): void + public static function orderedRules(): iterable { - $existingContent = implode("\n", [ - '* text=auto', - '/README.md export-ignore', - '/docs/ export-ignore', - '/AGENTS.md export-ignore', - ]); - - $result = $this->merger->merge($existingContent, ['/.vscode/', '/tests/', '/.gitattributes']); - - self::assertSame( - "* text=auto\n" - . "/.vscode/ export-ignore\n" - . "/docs/ export-ignore\n" - . "/tests/ export-ignore\n" - . "/.gitattributes export-ignore\n" - . "/AGENTS.md export-ignore\n" - . '/README.md export-ignore', - $result, - ); + yield 'explicit unset exception' => ["*.txt export-ignore\nkeep.txt -export-ignore"]; + yield 'explicit unspecified exception' => ["*.txt export-ignore\nkeep.txt !export-ignore"]; + yield 'repeated positive after exception' => ["*.txt export-ignore\nkeep.txt -export-ignore\n*.txt export-ignore"]; + yield 'repeated custom attributes' => ["* text=auto\n*.txt -text\n* text=auto"]; + yield 'overlapping patterns' => ["docs/** export-ignore\ndocs/keep.* -export-ignore\ndocs/keep.txt export-ignore"]; + yield 'comments and whitespace' => ["# comment\n/docs/ export-ignore\n/tests/ export-ignore"]; + yield 'quoted path' => ['"docs with spaces/file.txt" export-ignore']; } /** * @return void */ #[Test] - public function mergeWillPreserveComments(): void + public function mergePlacesGeneratedDefaultsBeforeConsumerOverrides(): void { - $existingContent = "# This is a comment\n*.zip -diff"; - - $result = $this->merger->merge($existingContent, ['/docs/']); + $content = "keep.txt -export-ignore\nother.txt !export-ignore\n*.zip -diff"; + $merger = new Merger(); + $result = $merger->merge($content, ['*.txt']); - self::assertStringStartsWith('#', $result); + self::assertSame("*.txt export-ignore\n" . $content, $result); + self::assertSame($result, $merger->merge($result, ['*.txt'])); } /** * @return void */ #[Test] - public function mergeWillNormalizeWhitespaceInExistingEntries(): void + public function mergePreservesExistingPositiveRulesInsteadOfAddingDuplicates(): void { - $existingContent = '/docs/ export-ignore'; + $content = "/docs/ export-ignore\n/docs/ -export-ignore\n/docs/ export-ignore"; - $result = $this->merger->merge($existingContent, ['/tests/']); - - self::assertStringContainsString('/docs/ export-ignore', $result); + self::assertSame($content, (new Merger())->merge($content, ['/docs/', '/docs/'])); } /** * @return void */ #[Test] - public function mergeWillDeduplicateNormalizedEntries(): void + public function mergeRemovesSimplePositiveRulesForExplicitlyKeptPaths(): void { - $existingContent = '/.github/ export-ignore'; - - $result = $this->merger->merge($existingContent, ['/.github/']); + $content = "* text=auto\n/.agents/agents/ export-ignore\n/.agents/skills/ export-ignore\nAGENTS.md export-ignore\n/tests/ export-ignore"; - self::assertCount(1, explode("\n", trim($result))); - } - - /** - * @return void - */ - #[Test] - public function mergeWithEmptyExistingContentWillCreateCleanOutput(): void - { - $result = $this->merger->merge('', ['/docs/', '/tests/']); - - self::assertSame("/docs/ export-ignore\n/tests/ export-ignore", $result); - } - - /** - * @return void - */ - #[Test] - public function mergeWillHandleGlobPatternsWithExportIgnore(): void - { - $existingContent = '*.pdf export-ignore'; - - $result = $this->merger->merge($existingContent, ['/docs/']); - - self::assertStringContainsString('*.pdf export-ignore', $result); - } - - /** - * @return void - */ - #[Test] - public function mergeWillSortDirectoriesBeforeFiles(): void - { - $result = $this->merger->merge('', ['/README.md', '/docs/', '/tests/', '/.editorconfig']); - - $lines = explode("\n", $result); - self::assertSame('/docs/ export-ignore', $lines[0]); - self::assertSame('/tests/ export-ignore', $lines[1]); - } - - /** - * @return void - */ - #[Test] - public function mergeWillHandleMultipleWhitespaceNormalize(): void - { - $existingContent = "/docs/ export-ignore\n/tests/ export-ignore"; - - $result = $this->merger->merge($existingContent, []); - - self::assertStringContainsString('/docs/ export-ignore', $result); + self::assertSame( + "* text=auto\n/tests/ export-ignore", + (new Merger())->merge($content, ['/.agents/agents/', '/AGENTS.md', '/tests/'], ['/.agents/', '/AGENTS.md']), + ); } /** * @return void */ #[Test] - public function mergeWillRemoveDuplicateNonExportIgnoreLines(): void + public function mergeKeepsAttributeOverridesForExplicitlyKeptPaths(): void { - $existingContent = "* text=auto\n* text=auto"; - - $result = $this->merger->merge($existingContent, []); + $content = "/docs/ -export-ignore\n/docs/ !export-ignore"; - self::assertSame(1, substr_count($result, '* text=auto')); + self::assertSame($content, (new Merger())->merge($content, ['/docs/'], ['/docs/'])); } /** * @return void */ #[Test] - public function mergeWillPreserveLeadingWhitespaceInCustomEntries(): void + public function mergeDoesNotConflateRootAnchoredAndUnanchoredGlobs(): void { - $existingContent = ' * text=auto'; - - $result = $this->merger->merge($existingContent, []); - - self::assertStringContainsString('* text=auto', $result); + self::assertSame( + "/*.txt export-ignore\n*.txt export-ignore", + (new Merger())->merge('*.txt export-ignore', ['/*.txt']), + ); } /** * @return void */ #[Test] - public function privateHelpersWillNormalizeAndFilterPathMetadataConsistently(): void + public function mergeIgnoresBlankGeneratedAndKeepEntries(): void { - $parseExistingLines = new ReflectionMethod($this->merger, 'parseExistingLines'); - $keepInExportLookup = new ReflectionMethod($this->merger, 'keepInExportLookup'); - $generatedDirectoryLookup = new ReflectionMethod($this->merger, 'generatedDirectoryLookup'); - $normalizeLine = new ReflectionMethod($this->merger, 'normalizeLine'); - $extractExportIgnorePathSpec = new ReflectionMethod($this->merger, 'extractExportIgnorePathSpec'); - $sortKey = new ReflectionMethod($this->merger, 'sortKey'); - $normalizePathKey = new ReflectionMethod($this->merger, 'normalizePathKey'); - $isLiteralPathSpec = new ReflectionMethod($this->merger, 'isLiteralPathSpec'); - - self::assertSame( - ['/docs/ export-ignore', '# comment'], - $parseExistingLines->invoke($this->merger, "\n/docs/ export-ignore\n\n# comment\n"), - ); - self::assertSame([ - 'docs' => true, - ], $keepInExportLookup->invoke($this->merger, [' ', '/docs/']),); - self::assertSame( - [ - 'docs' => true, - ], - $generatedDirectoryLookup->invoke($this->merger, [' /docs/ ', '/README.md']), - ); - self::assertSame('# comment', $normalizeLine->invoke($this->merger, ' # comment ')); - self::assertSame( - 'docs\\ path export-ignore', - $normalizeLine->invoke($this->merger, "docs\\ path\texport-ignore") - ); - self::assertNull($extractExportIgnorePathSpec->invoke($this->merger, '*.zip -diff')); - self::assertSame('docs/file.txt', $sortKey->invoke($this->merger, '///docs//file.txt')); - self::assertSame('/*.md', $normalizePathKey->invoke($this->merger, '/*.md')); - self::assertFalse($isLiteralPathSpec->invoke($this->merger, '/*.md')); + self::assertSame('/docs/ export-ignore', (new Merger())->merge('', ['', ' ', '/docs/'], ['', ' '])); } } diff --git a/tests/GitIgnore/GitIgnoreTest.php b/tests/GitIgnore/GitIgnoreTest.php index e2a5c44be4..6a695e9448 100644 --- a/tests/GitIgnore/GitIgnoreTest.php +++ b/tests/GitIgnore/GitIgnoreTest.php @@ -133,4 +133,21 @@ public function fromFileFiltersEmptyLines(): void unlink($tempFile); } } + /** + * @return void + */ + #[Test] + public function fromFilePreservesSignificantPatternWhitespaceAndZero(): void + { + $tempFile = sys_get_temp_dir() . '/test_gitignore_' . uniqid() . '.gitignore'; + $entries = [' leading', 'trailing\\ ', '\\#literal', '\\!literal', '0']; + file_put_contents($tempFile, implode("\n", $entries) . "\n"); + + try { + self::assertSame($entries, GitIgnore::fromFile($tempFile)->entries()); + } finally { + unlink($tempFile); + } + } + } diff --git a/tests/GitIgnore/MergerTest.php b/tests/GitIgnore/MergerTest.php index e56dd02c9e..8f179e69c6 100644 --- a/tests/GitIgnore/MergerTest.php +++ b/tests/GitIgnore/MergerTest.php @@ -19,236 +19,85 @@ namespace FastForward\DevTools\Tests\GitIgnore; -use FastForward\DevTools\GitIgnore\ClassifierInterface; use FastForward\DevTools\GitIgnore\GitIgnore; -use FastForward\DevTools\GitIgnore\GitIgnoreInterface; use FastForward\DevTools\GitIgnore\Merger; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; -use Prophecy\PhpUnit\ProphecyTrait; -use Prophecy\Prophecy\ObjectProphecy; #[CoversClass(Merger::class)] #[UsesClass(GitIgnore::class)] final class MergerTest extends TestCase { - use ProphecyTrait; - - /** - * @var ObjectProphecy - */ - private ObjectProphecy $classifier; - - /** - * @var ObjectProphecy - */ - private ObjectProphecy $gitIgnoreDevTools; - - /** - * @var ObjectProphecy - */ - private ObjectProphecy $gitIgnoreProject; - - private Merger $merger; - /** - * @return void - */ - protected function setUp(): void - { - $this->classifier = $this->prophesize(ClassifierInterface::class); - $this->gitIgnoreDevTools = $this->prophesize(GitIgnoreInterface::class); - $this->gitIgnoreProject = $this->prophesize(GitIgnoreInterface::class); - - $this->gitIgnoreDevTools->path() - ->willReturn('/canonical/.gitignore'); - $this->gitIgnoreProject->path() - ->willReturn('/project/.gitignore'); - - $this->merger = new Merger($this->classifier->reveal()); - } - - /** - * @param string $entries - */ - private function withDirectoryClassification(string ...$entries): self - { - foreach ($entries as $entry) { - $this->classifier->isDirectory($entry) - ->willReturn(true); - } - - return $this; - } - - /** - * @param string $entries - */ - private function withFileClassification(string ...$entries): self - { - foreach ($entries as $entry) { - $this->classifier->isDirectory($entry) - ->willReturn(false); - } - - return $this; - } - - /** - * @return void + * @param list $entries */ #[Test] - public function mergeWithEmptyEntries(): void + #[DataProvider('orderedRules')] + public function mergeWithEmptyCanonicalPreservesConsumerRules(array $entries): void { - $this->withFileClassification('*.log'); - - $this->gitIgnoreDevTools->entries() - ->willReturn([]); - $this->gitIgnoreProject->entries() - ->willReturn(['*.log']); + $result = (new Merger())->merge(new GitIgnore('/canonical', []), new GitIgnore('/project', $entries)); - $result = $this->merger->merge($this->gitIgnoreDevTools->reveal(), $this->gitIgnoreProject->reveal()); - - self::assertSame(['*.log'], $result->entries()); + self::assertSame($entries, $result->entries()); + self::assertSame('/project', $result->path()); } /** - * @return void + * @return iterable}> */ - #[Test] - public function mergeRemovesDuplicates(): void - { - $this->withDirectoryClassification('vendor/', 'node_modules/'); - $this->withFileClassification('*.log'); - - $this->gitIgnoreDevTools->entries() - ->willReturn(['vendor/', '*.log']); - $this->gitIgnoreProject->entries() - ->willReturn(['vendor/', 'node_modules/']); - - $result = $this->merger->merge($this->gitIgnoreDevTools->reveal(), $this->gitIgnoreProject->reveal()); - - self::assertCount(3, $result->entries()); - self::assertContains('vendor/', $result->entries()); - self::assertContains('*.log', $result->entries()); - self::assertContains('node_modules/', $result->entries()); - } - - /** - * @return void - */ - #[Test] - public function mergeSortsDirectoriesFirst(): void - { - $this->withDirectoryClassification('vendor/', 'node_modules/'); - $this->withFileClassification('*.log'); - - $this->gitIgnoreDevTools->entries() - ->willReturn(['*.log']); - $this->gitIgnoreProject->entries() - ->willReturn(['vendor/', 'node_modules/']); - - $result = $this->merger->merge($this->gitIgnoreDevTools->reveal(), $this->gitIgnoreProject->reveal()); - $entries = $result->entries(); - - self::assertCount(3, $entries); - self::assertContains('vendor/', $entries); - self::assertContains('node_modules/', $entries); - self::assertContains('*.log', $entries); - - self::assertTrue($this->isBeforeInArray('vendor/', $entries, '*.log')); - self::assertTrue($this->isBeforeInArray('node_modules/', $entries, '*.log')); - } - - /** - * @param string $needle - * @param array $haystack - * @param string $before - * - * @return bool - */ - private function isBeforeInArray(string $needle, array $haystack, string $before): bool + public static function orderedRules(): iterable { - return array_search($needle, $haystack, true) < array_search($before, $haystack, true); + yield 'negation' => [['*.log', '!important.log']]; + yield 'repeated rule after exception' => [['*.log', '!important.log', '*.log']]; + yield 'directory traversal exception' => [['build/*', '!build/keep/', 'build/keep/*', '!build/keep/file.txt']]; + yield 'significant whitespace and escaped markers' => [[' leading', 'trailing\\ ', '\\#literal', '\\!literal', '0']]; } /** * @return void */ #[Test] - public function mergeRemovesComments(): void + public function mergePlacesCanonicalDefaultsBeforeConsumerOverrides(): void { - $this->withDirectoryClassification('vendor/'); - $this->withFileClassification('*.log'); + $canonical = new GitIgnore('/canonical', ['*.log', 'build/*']); + $project = new GitIgnore('/project', ['!important.log', '!build/keep/']); + $merger = new Merger(); - $this->gitIgnoreDevTools->entries() - ->willReturn(['# comment', 'vendor/']); - $this->gitIgnoreProject->entries() - ->willReturn(['# another comment', '*.log']); + $result = $merger->merge($canonical, $project); - $result = $this->merger->merge($this->gitIgnoreDevTools->reveal(), $this->gitIgnoreProject->reveal()); - - self::assertNotContains('# comment', $result->entries()); - self::assertNotContains('# another comment', $result->entries()); + self::assertSame(['*.log', 'build/*', '!important.log', '!build/keep/'], $result->entries()); + self::assertSame($result->entries(), $merger->merge($canonical, $result)->entries()); } /** * @return void */ #[Test] - public function mergeRemovesEmptyEntries(): void + public function mergePreservesExistingOccurrencesAfterCanonicalPrefix(): void { - $this->withDirectoryClassification('vendor/'); - $this->withFileClassification('*.log'); - - $this->gitIgnoreDevTools->entries() - ->willReturn(['', ' ', 'vendor/']); - $this->gitIgnoreProject->entries() - ->willReturn(['*.log', '', ' ']); + $canonical = new GitIgnore('/canonical', ['*.log']); + $project = new GitIgnore('/project', ['!important.log', '*.log']); + $merger = new Merger(); - $result = $this->merger->merge($this->gitIgnoreDevTools->reveal(), $this->gitIgnoreProject->reveal()); + $result = $merger->merge($canonical, $project); - self::assertNotContains('', $result->entries()); - self::assertNotContains(' ', $result->entries()); + self::assertSame(['*.log', '!important.log', '*.log'], $result->entries()); + self::assertSame($result->entries(), $merger->merge($canonical, $result)->entries()); } /** * @return void */ #[Test] - public function mergeUsesProjectPath(): void + public function mergeRemovesOnlyCommentsAndBlankLines(): void { - $this->withDirectoryClassification('vendor/'); - $this->withFileClassification('*.log'); - - $this->gitIgnoreDevTools->entries() - ->willReturn(['vendor/']); - $this->gitIgnoreProject->entries() - ->willReturn(['*.log']); - - $result = $this->merger->merge($this->gitIgnoreDevTools->reveal(), $this->gitIgnoreProject->reveal()); - - self::assertSame('/project/.gitignore', $result->path()); - } - - /** - * @return void - */ - #[Test] - public function mergeReturnsGitIgnoreInstance(): void - { - $this->withDirectoryClassification('vendor/'); - $this->withFileClassification('*.log'); - - $this->gitIgnoreDevTools->entries() - ->willReturn(['vendor/']); - $this->gitIgnoreProject->entries() - ->willReturn(['*.log']); - - $result = $this->merger->merge($this->gitIgnoreDevTools->reveal(), $this->gitIgnoreProject->reveal()); + $result = (new Merger())->merge( + new GitIgnore('/canonical', ['', '# comment', 'vendor/']), + new GitIgnore('/project', [' ', '# another comment', '*.log', ' #literal']), + ); - self::assertInstanceOf(GitIgnore::class, $result); + self::assertSame(['vendor/', '*.log', ' #literal'], $result->entries()); } }