diff --git a/lib/Controller/API/App/GetSegmentsController.php b/lib/Controller/API/App/GetSegmentsController.php index c4db3e7b70..505f7f6851 100644 --- a/lib/Controller/API/App/GetSegmentsController.php +++ b/lib/Controller/API/App/GetSegmentsController.php @@ -25,6 +25,7 @@ use ReflectionException; use Utils\TaskRunner\Exceptions\EndQueueException; use Utils\TaskRunner\Exceptions\ReQueueException; +use Utils\LQA\ICUSourceSegmentDetector; use Utils\Tools\CatUtils; class GetSegmentsController extends KleinController @@ -127,7 +128,7 @@ public function segments(): void language: $job->source, patternString: $seg['segment'] ); - $string_contains_icu = $analyzer->containsComplexSyntax() && $analyzer->isValidSyntax(); + $string_contains_icu = ICUSourceSegmentDetector::sourceContainsIcu($analyzer, $icu_enabled); } /** @var MateCatFilter $Filter */ diff --git a/lib/Model/ProjectCreation/ProjectManager.php b/lib/Model/ProjectCreation/ProjectManager.php index bccb47f2f3..e7fa49f1d1 100644 --- a/lib/Model/ProjectCreation/ProjectManager.php +++ b/lib/Model/ProjectCreation/ProjectManager.php @@ -32,6 +32,7 @@ use Plugins\Features\SecondPassReview; use ReflectionException; use Throwable; +use TypeError; use Utils\ActiveMQ\AMQHandler; use Utils\ActiveMQ\WorkerClient; use Utils\AsyncTasks\Workers\ActivityLogWorker; @@ -215,7 +216,11 @@ protected function getJobCreationService(): JobCreationService */ protected function getQAProcessor(): QAProcessor { - return $this->qaProcessor ??= new QAProcessor($this->filter, $this->features); + return $this->qaProcessor ??= new QAProcessor( + $this->filter, + $this->features, + (bool) ($this->projectStructure->metadata[ProjectsMetadataMarshaller::ICU_ENABLED->value] ?? false), + ); } /** @@ -1017,6 +1022,7 @@ protected function storeSegments(int $fid): void /** * @throws Exception + * @throws TypeError */ private function insertSegmentNotesForFile(): void { diff --git a/lib/Model/ProjectCreation/QAProcessor.php b/lib/Model/ProjectCreation/QAProcessor.php index 301fd64a40..d1257623c2 100644 --- a/lib/Model/ProjectCreation/QAProcessor.php +++ b/lib/Model/ProjectCreation/QAProcessor.php @@ -2,8 +2,12 @@ namespace Model\ProjectCreation; +use Exception; +use Matecat\ICU\MessagePatternComparator; +use Matecat\ICU\MessagePatternValidator; use Matecat\SubFiltering\MateCatFilter; use Model\FeaturesBase\FeatureSet; +use Utils\LQA\ICUSourceSegmentDetector; use Utils\LQA\QA; /** @@ -13,6 +17,10 @@ * selects the appropriate target string, converts back to Layer 0, and writes the results * onto each {@see TranslationTuple} in place. * + * When ICU is enabled for the project, the processor also detects ICU MessageFormat + * patterns in each segment and passes a {@see MessagePatternComparator} to QA so that + * ICU consistency is validated alongside tag checks. + * * Produces 4 scalars per tuple: * - translationLayer0: the QA-processed target (Layer 0) * - suggestionLayer0: same as translationLayer0 (may diverge in future) @@ -24,6 +32,7 @@ class QAProcessor public function __construct( private readonly MateCatFilter $filter, private readonly FeatureSet $features, + private readonly bool $icuEnabled = false, ) { } @@ -35,6 +44,8 @@ public function __construct( * @param ProjectStructure $projectStructure contains translations to process * @param string $sourceLang source language code (e.g. 'en-US') * @param string $targetLang target language code (e.g. 'it-IT') + * + * @throws Exception */ public function process( ProjectStructure $projectStructure, @@ -50,7 +61,14 @@ public function process( $source = $this->filter->fromLayer0ToLayer1($tuple->source); $target = $this->filter->fromLayer0ToLayer1($tuple->target); - $check = $this->createQA($source, $target); + [$comparator, $sourceContainsIcu] = $this->detectIcu( + $sourceLang, + $targetLang, + $tuple->source, + $tuple->target, + ); + + $check = $this->createQA($source, $target, $comparator, $sourceContainsIcu); $check->setFeatureSet($this->features); $check->setSourceSegLang($sourceLang); $check->setTargetSegLang($targetLang); @@ -70,12 +88,62 @@ public function process( } } + /** + * Detect ICU MessageFormat patterns in the source segment. + * + * Uses the raw Layer 0 content for detection because curly-bracket filter + * handlers are disabled by default at project-creation time, so ICU syntax + * survives the L0→L1 round-trip. + * + * @param string $sourceLang source language code + * @param string $targetLang target language code + * @param string $rawSource raw Layer 0 source content + * @param string $rawTarget raw Layer 0 target content + * + * @return array{0: ?MessagePatternComparator, 1: bool} + * [comparator (null when ICU is not detected), sourceContainsIcu flag] + */ + private function detectIcu( + string $sourceLang, + string $targetLang, + string $rawSource, + string $rawTarget, + ): array { + if (!$this->icuEnabled) { + return [null, false]; + } + + $sourceValidator = new MessagePatternValidator($sourceLang, $rawSource); + + $sourceContainsIcu = ICUSourceSegmentDetector::sourceContainsIcu($sourceValidator, $this->icuEnabled); + + if (!$sourceContainsIcu) { + return [null, false]; + } + + $targetValidator = new MessagePatternValidator($targetLang, $rawTarget); + + return [ + MessagePatternComparator::fromValidators($sourceValidator, $targetValidator), + true, + ]; + } + /** * Create a new QA instance. * Protected so test subclasses can override to inject stubs. + * + * @param string $source Layer 1 source segment + * @param string $target Layer 1 target segment + * @param MessagePatternComparator|null $comparator ICU pattern comparator (null when ICU not detected) + * @param bool $sourceContainsIcu whether the source contains ICU patterns */ - protected function createQA(string $source, string $target): QA - { - return new QA($source, $target); + protected function createQA( + string $source, + string $target, + ?MessagePatternComparator $comparator = null, + bool $sourceContainsIcu = false, + ): QA { + return new QA($source, $target, $comparator, $sourceContainsIcu); } } diff --git a/lib/Utils/LQA/ICUSourceSegmentChecker.php b/lib/Utils/LQA/ICUSourceSegmentChecker.php index 8a99d9760e..229664aa3b 100644 --- a/lib/Utils/LQA/ICUSourceSegmentChecker.php +++ b/lib/Utils/LQA/ICUSourceSegmentChecker.php @@ -44,9 +44,10 @@ private function sourceContainsIcu(ProjectStruct $projectStruct, JobStruct $chun $sourceSegment, ); - if ($this->icuEnabled($projectStruct)) { - $this->sourceContainsIcu = $this->icuSourcePatternValidator->containsComplexSyntax() && $this->icuSourcePatternValidator->isValidSyntax(); - } + $this->sourceContainsIcu = ICUSourceSegmentDetector::sourceContainsIcu( + $this->icuSourcePatternValidator, + $this->icuEnabled($projectStruct) + ); return $this->sourceContainsIcu; @@ -66,4 +67,4 @@ private function icuEnabled(ProjectStruct $projectStruct): bool return $this->icuEnabled = $projectStruct->getMetadataValue(ProjectsMetadataMarshaller::ICU_ENABLED->value) ?? false; } -} \ No newline at end of file +} diff --git a/lib/Utils/LQA/ICUSourceSegmentDetector.php b/lib/Utils/LQA/ICUSourceSegmentDetector.php new file mode 100644 index 0000000000..0352f755ec --- /dev/null +++ b/lib/Utils/LQA/ICUSourceSegmentDetector.php @@ -0,0 +1,26 @@ +containsComplexSyntax() + && $validator->isValidSyntax(); + } +} diff --git a/tests/unit/Model/ProjectCreation/QAProcessorTest.php b/tests/unit/Model/ProjectCreation/QAProcessorTest.php index fd2f7a4424..c33bd3fcf8 100644 --- a/tests/unit/Model/ProjectCreation/QAProcessorTest.php +++ b/tests/unit/Model/ProjectCreation/QAProcessorTest.php @@ -50,7 +50,7 @@ protected function setUp(): void * * @throws MockException */ - private function buildProcessor(?QA $qa = null): TestableQAProcessor + private function buildProcessor(?QA $qa = null, bool $icuEnabled = false): TestableQAProcessor { $filter = $this->createStub(MateCatFilter::class); $filter->method('fromLayer0ToLayer1') @@ -61,6 +61,7 @@ private function buildProcessor(?QA $qa = null): TestableQAProcessor $processor = new TestableQAProcessor( $filter, $this->createStub(FeatureSet::class), + $icuEnabled, ); if ($qa !== null) { @@ -247,4 +248,97 @@ public function appliesLayerConversions(): void // fromLayer1ToLayer0('L1:target text') → 'L0:L1:target text' $this->assertSame('L0:L1:target text', $tuple->translationLayer0); } + + // ── Test 6: ICU enabled + ICU source → comparator passed to createQA ─ + + /** + * @throws MockException + */ + #[Test] + public function passesComparatorWhenIcuEnabledAndSourceContainsIcu(): void + { + $qa = $this->buildQAStub(normalizedTarget: 'norm'); + + $icuSource = '{count, plural, one{# item} other{# items}}'; + $icuTarget = '{count, plural, one{# elemento} other{# elementi}}'; + + $tuple = $this->makeTuple(source: $icuSource, target: $icuTarget); + $this->projectStructure->translations = [ + 'tu-1' => [0 => $tuple], + ]; + + $processor = $this->buildProcessor($qa, icuEnabled: true); + $processor->process($this->projectStructure, 'en-US', 'it-IT'); + + $this->assertNotNull($processor->lastComparator); + $this->assertTrue($processor->lastSourceContainsIcu); + } + + // ── Test 7: ICU disabled + ICU source → no comparator ──────────── + + /** + * @throws MockException + */ + #[Test] + public function skipsIcuDetectionWhenIcuDisabled(): void + { + $qa = $this->buildQAStub(normalizedTarget: 'norm'); + + $icuSource = '{count, plural, one{# item} other{# items}}'; + $icuTarget = '{count, plural, one{# elemento} other{# elementi}}'; + + $tuple = $this->makeTuple(source: $icuSource, target: $icuTarget); + $this->projectStructure->translations = [ + 'tu-1' => [0 => $tuple], + ]; + + $processor = $this->buildProcessor($qa, icuEnabled: false); + $processor->process($this->projectStructure, 'en-US', 'it-IT'); + + $this->assertNull($processor->lastComparator); + $this->assertFalse($processor->lastSourceContainsIcu); + } + + // ── Test 8: ICU enabled + non-ICU source → no comparator ───────── + + /** + * @throws MockException + */ + #[Test] + public function skipsIcuDetectionWhenSourceHasNoIcuPatterns(): void + { + $qa = $this->buildQAStub(normalizedTarget: 'norm'); + + $tuple = $this->makeTuple(source: 'plain source text', target: 'plain target text'); + $this->projectStructure->translations = [ + 'tu-1' => [0 => $tuple], + ]; + + $processor = $this->buildProcessor($qa, icuEnabled: true); + $processor->process($this->projectStructure, 'en-US', 'it-IT'); + + $this->assertNull($processor->lastComparator); + $this->assertFalse($processor->lastSourceContainsIcu); + } + + // ── Test 9: ICU enabled + broken target → QA reports ICU errors ── + + #[Test] + public function reportsIcuValidationErrorsForBrokenTarget(): void + { + $icuSource = '{count, plural, one{# item} other{# items}}'; + $brokenTarget = '{count, plural, one{# elemento}}'; + + $tuple = $this->makeTuple(source: $icuSource, target: $brokenTarget); + $this->projectStructure->translations = [ + 'tu-1' => [0 => $tuple], + ]; + + // No QA stub — uses real QA so ICU validation actually runs + $processor = $this->buildProcessor(icuEnabled: true); + $processor->process($this->projectStructure, 'en-US', 'it-IT'); + + $this->assertSame(1, $tuple->warning); + $this->assertNotEmpty($tuple->serializedErrors); + } } diff --git a/tests/unit/Model/ProjectCreation/TestableQAProcessor.php b/tests/unit/Model/ProjectCreation/TestableQAProcessor.php index 03dbefff55..3f27f0c8f0 100644 --- a/tests/unit/Model/ProjectCreation/TestableQAProcessor.php +++ b/tests/unit/Model/ProjectCreation/TestableQAProcessor.php @@ -2,6 +2,7 @@ namespace unit\Model\ProjectCreation; +use Matecat\ICU\MessagePatternComparator; use Model\ProjectCreation\QAProcessor; use Utils\LQA\QA; @@ -12,19 +13,26 @@ class TestableQAProcessor extends QAProcessor { private ?QA $qaInstance = null; - /** - * Inject a QA stub/mock for tests. - */ + /** @var ?MessagePatternComparator Captures the comparator passed to the last createQA() call */ + public ?MessagePatternComparator $lastComparator = null; + + /** @var ?bool Captures the sourceContainsIcu flag passed to the last createQA() call */ + public ?bool $lastSourceContainsIcu = null; + public function setQA(QA $qa): void { $this->qaInstance = $qa; } - /** - * Override to return the injected QA instance instead of creating a real one. - */ - protected function createQA(string $source, string $target): QA - { - return $this->qaInstance ?? parent::createQA($source, $target); + protected function createQA( + string $source, + string $target, + ?MessagePatternComparator $comparator = null, + bool $sourceContainsIcu = false, + ): QA { + $this->lastComparator = $comparator; + $this->lastSourceContainsIcu = $sourceContainsIcu; + + return $this->qaInstance ?? parent::createQA($source, $target, $comparator, $sourceContainsIcu); } }