From 7ec77e29bde205d7cdf30c08cc67707bf6d41be8 Mon Sep 17 00:00:00 2001
From: Francesco Lodolo <flod@lodolo.net>
Date: Wed, 19 Jun 2024 13:59:03 +0200
Subject: [PATCH] Variable checks: check patterns only on specific file
 extensions (#1038)

---
 app/classes/Transvision/AnalyseStrings.php | 22 +++++++++++++++++++-
 tests/units/Transvision/AnalyseStrings.php | 24 ++++++++++++++--------
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/app/classes/Transvision/AnalyseStrings.php b/app/classes/Transvision/AnalyseStrings.php
index 4bae65c5..a2fa8b8f 100644
--- a/app/classes/Transvision/AnalyseStrings.php
+++ b/app/classes/Transvision/AnalyseStrings.php
@@ -57,12 +57,19 @@ public static function differences($tmx_source, $tmx_target, $repo, $ignored_str
             // %1$S or %S. %1$0.S and %0.S are valid too
             'printf'      => '/(%(?:[0-9]+\$){0,1}(?:[0-9].){0,1}([sS]))/',
             // $BrandShortName, but not "My%1$SFeeds-%2$S.opml" or "{ $brandShortName }"
-            'properties'  => '/(?<!%[0-9]|\{\s|\{)(\$[A-Za-z0-9\.]+)\b/',
+            'properties'  => '/(?<!%[0-9]|\{\s|\{)(\$[A-Za-z0-9\._-]+)\b/',
             // %1$s or %s. %d
             'xml_android' => '/(%(?:[0-9]+\$){0,1}([sd]))/',
             // %1, %2, etc.
             'xliff-qt'    => '/(%[0-9])/',
         ];
+        $pattern_extensions = [
+            'ios'         => 'xml',
+            'l10njs'      => 'properties',
+            'printf'      => 'properties',
+            'xml_android' => 'xml',
+            'xliff-qt'    => 'xliff',
+        ];
         $repo_patterns = Project::$repos_info[$repo]['variable_patterns'];
 
         $patterns = array_filter($patterns, function($k) use ($repo_patterns) {
@@ -71,7 +78,20 @@ public static function differences($tmx_source, $tmx_target, $repo, $ignored_str
 
         foreach ($patterns as $pattern_name => $pattern) {
             foreach ($tmx_source as $key => $source) {
+                $file_path = explode(':', $key)[0];
+                $file_extension = explode('.', $file_path);
+                $file_extension = end($file_extension);
+
+                // If not specified, run all patterns
+                $check_pattern = True;
+                if (array_key_exists($file_extension, $patterns) || in_array($file_extension, array_values($pattern_extensions))) {
+                    // Run current pattern only on specific file extensions
+                    $valid_extension = $pattern_extensions[$pattern_name] ?? ".{$pattern_name}";
+                    $check_pattern = Strings::endsWith($file_path, $valid_extension);
+                }
+
                 if (isset($tmx_target[$key])
+                    && $check_pattern
                     && $tmx_target[$key] != ''
                     && ! in_array($key, $ignored_strings)) {
                     /*
diff --git a/tests/units/Transvision/AnalyseStrings.php b/tests/units/Transvision/AnalyseStrings.php
index 3f5af3d1..0d34d247 100644
--- a/tests/units/Transvision/AnalyseStrings.php
+++ b/tests/units/Transvision/AnalyseStrings.php
@@ -171,6 +171,14 @@ public function differencesDP()
                 [],
                 [],
             ],
+            [
+                // .properties checks should not run in FTL messages
+                ['browser.ftl:foobar16a1' => '{ $brandname } installed $properties_var'],
+                ['browser.ftl:foobar16a1' => '{ $brandname } installato'],
+                'gecko_strings',
+                [],
+                [],
+            ],
             [
                 // Difference spacing in variable for FTL (not an error)
                 ['browser:foobar16a2' => '{$brandname} installed'],
@@ -253,24 +261,24 @@ public function differencesDP()
             ],
             [
                 // Mispelled variable
-                ['ios:foobar1' => 'Introductory slide %1$@ of %2$@'],
-                ['ios:foobar1' => 'Introduzione (passaggio %1$@ di %$@)'],
+                ['iostest:foobar1' => 'Introductory slide %1$@ of %2$@'],
+                ['iostest:foobar1' => 'Introduzione (passaggio %1$@ di %$@)'],
                 'firefox_ios',
                 [],
-                ['ios:foobar1'],
+                ['iostest:foobar1'],
             ],
             [
                 // Missing variable
-                ['ios:foobar2' => 'Do you want to save the password on %@?'],
-                ['ios:foobar2' => 'Salvare la password?'],
+                ['iostest:foobar2' => 'Do you want to save the password on %@?'],
+                ['iostest:foobar2' => 'Salvare la password?'],
                 'firefox_ios',
                 [],
-                ['ios:foobar2'],
+                ['iostest:foobar2'],
             ],
             [
                 // Changed order, not an error
-                ['ios:foobar3' => 'Introductory slide %1$@ of %2$@'],
-                ['ios:foobar3' => 'Introduzione (passaggio %2$@ di %1$@)'],
+                ['iostest:foobar3' => 'Introductory slide %1$@ of %2$@'],
+                ['iostest:foobar3' => 'Introduzione (passaggio %2$@ di %1$@)'],
                 'firefox_ios',
                 [],
                 [],