From 444d4b4e66928ec2ca0d52528dbf3eb4add7730c Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Thu, 7 May 2026 18:25:39 +0530 Subject: [PATCH] refactor: standalone regex matching for commit skip patterns Replace directive-extraction approach with word-boundary regex matching so plain-word patterns like "skip appwrite" and "appwrite skip" work alongside bracket directives. Use \s+ between word tokens (required space) and \s* only after ":" tokens (git trailer flexibility). Add tests for "skip appwrite" and "appwrite skip" with case insensitivity. Co-Authored-By: Claude Sonnet 4.6 --- .../Vcs/Validator/CommitSkipPatterns.php | 107 +++++++----------- .../Vcs/Validator/CommitSkipPatternsTest.php | 61 ++++++++++ 2 files changed, 101 insertions(+), 67 deletions(-) diff --git a/src/Appwrite/Vcs/Validator/CommitSkipPatterns.php b/src/Appwrite/Vcs/Validator/CommitSkipPatterns.php index 7243808bf8..9dbe45ba83 100644 --- a/src/Appwrite/Vcs/Validator/CommitSkipPatterns.php +++ b/src/Appwrite/Vcs/Validator/CommitSkipPatterns.php @@ -12,8 +12,16 @@ class CommitSkipPatterns extends Validator /** * Returns false (skip deployment) when the commit message contains any of the - * configured skip directives. + * configured patterns as a standalone directive (case-insensitive). * Returns true (proceed) when no patterns are configured or none match. + * + * Matching rules: + * - Case-insensitive + * - The directive must be surrounded by whitespace or string boundaries, so + * "prefix[skip deploy]suffix" does NOT accidentally skip + * - Internal whitespace in the pattern is normalised: tokens are split on \s+ + * and rejoined with \s* in the regex, so "[skip deploy]" matches + * "[skip deploy]" and "skip-checks: true" matches "skip-checks:true" */ public function isValid($value): bool { @@ -21,13 +29,38 @@ class CommitSkipPatterns extends Validator return false; } - $patterns = $this->normalizePatterns($this->patterns); - if (empty($patterns)) { - return true; - } + foreach ($this->patterns as $pattern) { + if (!is_string($pattern)) { + continue; + } - foreach ($this->extractDirectives($value) as $directive) { - if (isset($patterns[$directive])) { + $pattern = trim($pattern); + if ($pattern === '') { + continue; + } + + // Split on whitespace; each token is regex-quoted. Tokens are rejoined + // with \s+ (required space) so that "skipappwrite" does NOT match the + // pattern "skip appwrite". The only exception: when the preceding token + // ends with ":" (git trailer style), \s* is used so that + // "skip-checks:true" still matches the pattern "skip-checks: true". + $tokens = preg_split('/\s+/', $pattern); + $regexParts = []; + $count = count($tokens); + for ($i = 0; $i < $count; $i++) { + $regexParts[] = preg_quote($tokens[$i], '~'); + if ($i < $count - 1) { + $regexParts[] = str_ends_with($tokens[$i], ':') ? '\s*' : '\s+'; + } + } + $regexBody = implode('', $regexParts); + + // (? $patterns - * @return array - */ - private function normalizePatterns(array $patterns): array - { - $normalized = []; - - foreach ($patterns as $pattern) { - if (!\is_string($pattern)) { - continue; - } - - $pattern = $this->normalizeDirective($pattern); - if ($pattern === '') { - continue; - } - - $normalized[$pattern] = true; - } - - return $normalized; - } - - /** - * @return array - */ - private function extractDirectives(string $message): array - { - $directives = []; - - if (\preg_match_all('/\[[^\]\r\n]+\]/u', $message, $matches) > 0) { - foreach ($matches[0] as $match) { - $directives[] = $this->normalizeDirective($match); - } - } - - foreach (\preg_split("/\r\n|\n|\r/", $message) ?: [] as $line) { - $line = \trim($line); - if ($line === '' || !\str_contains($line, ':')) { - continue; - } - - $directives[] = $this->normalizeDirective($line); - } - - return \array_values(\array_filter(\array_unique($directives))); - } - - private function normalizeDirective(string $value): string - { - $value = \trim($value); - if ($value === '') { - return ''; - } - - $value = (string) \preg_replace('/\s+/u', ' ', $value); - return \mb_strtolower($value); - } } diff --git a/tests/unit/Vcs/Validator/CommitSkipPatternsTest.php b/tests/unit/Vcs/Validator/CommitSkipPatternsTest.php index efc88bbf6a..b546199d22 100644 --- a/tests/unit/Vcs/Validator/CommitSkipPatternsTest.php +++ b/tests/unit/Vcs/Validator/CommitSkipPatternsTest.php @@ -151,4 +151,65 @@ class CommitSkipPatternsTest extends TestCase $msg = "feat: add new stuff\n\nMore detail here.\n\nskip-checks:true"; $this->assertFalse($validator->isValid($msg)); } + + // ------------------------------------------------------------------------- + // Plain-word patterns: "skip appwrite" and "appwrite skip" + // ------------------------------------------------------------------------- + + public function testSkipAppwritePatternSkips(): void + { + $validator = new CommitSkipPatterns(['skip appwrite']); + $this->assertFalse($validator->isValid('docs: update readme skip appwrite')); + $this->assertFalse($validator->isValid('skip appwrite')); + } + + public function testSkipAppwritePatternCaseInsensitive(): void + { + $validator = new CommitSkipPatterns(['skip appwrite']); + $this->assertFalse($validator->isValid('SKIP APPWRITE')); + $this->assertFalse($validator->isValid('Skip Appwrite')); + $this->assertFalse($validator->isValid('SKIP appwrite')); + } + + public function testSkipAppwritePatternNoMatchProceeds(): void + { + $validator = new CommitSkipPatterns(['skip appwrite']); + $this->assertTrue($validator->isValid('feat: real feature')); + $this->assertTrue($validator->isValid('skipappwrite')); // no space — not standalone + $this->assertTrue($validator->isValid('appwrite is great')); + } + + public function testAppwriteSkipPatternSkips(): void + { + $validator = new CommitSkipPatterns(['appwrite skip']); + $this->assertFalse($validator->isValid('appwrite skip ci')); + $this->assertFalse($validator->isValid('docs appwrite skip')); + $this->assertFalse($validator->isValid('appwrite skip')); + } + + public function testAppwriteSkipPatternCaseInsensitive(): void + { + $validator = new CommitSkipPatterns(['appwrite skip']); + $this->assertFalse($validator->isValid('APPWRITE SKIP')); + $this->assertFalse($validator->isValid('Appwrite Skip')); + $this->assertFalse($validator->isValid('appwrite SKIP')); + } + + public function testAppwriteSkipPatternNoMatchProceeds(): void + { + $validator = new CommitSkipPatterns(['appwrite skip']); + $this->assertTrue($validator->isValid('feat: deploy appwrite changes')); + $this->assertTrue($validator->isValid('appwriteskip')); // no space — not standalone + $this->assertTrue($validator->isValid('skip the appwrite stuff')); + } + + public function testBothAppwritePatternsInArray(): void + { + $validator = new CommitSkipPatterns(['skip appwrite', 'appwrite skip']); + $this->assertFalse($validator->isValid('skip appwrite')); + $this->assertFalse($validator->isValid('appwrite skip')); + $this->assertFalse($validator->isValid('SKIP APPWRITE')); + $this->assertFalse($validator->isValid('APPWRITE SKIP')); + $this->assertTrue($validator->isValid('feat: deploy appwrite changes')); + } }