javascript: Support '%m$' in format strings

Gjs has had support for numbered arguments in format strings since 1.40.
Recognize it as well in format-javascript, so msgfmt -c doesn't fail
when numbered arguments are in msgstr.  Reported by Sean Burke in:
https://lists.gnu.org/archive/html/bug-gettext/2015-10/msg00002.html
* gettext-tools/src/format-javascript.c (struct numbered_arg): New
struct.
(struct spec): Rename format_args_count to numbered_arg_count.  Add
NUMBERED field in place of FORMAT_ARGS.  All callers changed.
(numbered_arg_compare): New function.
(format_parse): Handle numbered arguments.  Based on format-awk.c.
(format_check): Add check for numbered arguments.
* gettext-tools/tests/format-javascript-1: Add tests for numbered
arguments.
* gettext-tools/tests/lang-javascript: Use numbered argument in format
strings.  Add check for Gjs version >= 1.40.

3 files changed, 225 insertions, 43 deletions

diff --git a/gettext-tools/src/format-javascript.c b/gettext-tools/src/format-javascript.c
index c8602c0..9c5ee00 100644
--- a/gettext-tools/src/format-javascript.c
+++ b/gettext-tools/src/format-javascript.c
@@ -34,15 +34,15 @@
 
 #define _(str) gettext (str)
 
-/* Although JavaScript specification itself does not define any format
-   strings, many implementations provide printf-like functions.
-   We provide a permissive parser which accepts commonly used format
-   strings, where:
+/* JavaScript format strings are not in the language specification,
+   but there are several implementations which provide the printf-like
+   feature.  Here, we provide a permissive parser which at least accepts
+   format strings supported by Gjs version 1.40, where:
 
    A directive
-   - starts with '%',
+   - starts with '%' or '%m$' where m is a positive integer,
    - is optionally followed by any of the characters '0', '-', ' ',
-     or, each of which acts as a flag,
+     or 'I', each of which acts as a flag,
    - is optionally followed by a width specification: a nonempty digit
      sequence,
    - is optionally followed by '.' and a precision specification: a nonempty
@@ -65,12 +65,18 @@ enum format_arg_type
   FAT_FLOAT
 };
 
+struct numbered_arg
+{
+  unsigned int number;
+  enum format_arg_type type;
+};
+
 struct spec
 {
   unsigned int directives;
-  unsigned int format_args_count;
+  unsigned int numbered_arg_count;
   unsigned int allocated;
-  enum format_arg_type *format_args;
+  struct numbered_arg *numbered;
 };
 
 /* Locale independent test for a decimal digit.
@@ -80,32 +86,71 @@ struct spec
 #define isdigit(c) ((unsigned int) ((c) - '0') < 10)
 
 
+static int
+numbered_arg_compare (const void *p1, const void *p2)
+{
+  unsigned int n1 = ((const struct numbered_arg *) p1)->number;
+  unsigned int n2 = ((const struct numbered_arg *) p2)->number;
+
+  return (n1 > n2 ? 1 : n1 < n2 ? -1 : 0);
+}
+
 static void *
 format_parse (const char *format, bool translated, char *fdi,
               char **invalid_reason)
 {
   const char *const format_start = format;
   struct spec spec;
+  unsigned int unnumbered_arg_count;
   struct spec *result;
 
   spec.directives = 0;
-  spec.format_args_count = 0;
+  spec.numbered_arg_count = 0;
   spec.allocated = 0;
-  spec.format_args = NULL;
+  spec.numbered = NULL;
+  unnumbered_arg_count = 0;
 
   for (; *format != '\0';)
     if (*format++ == '%')
       {
         /* A directive.  */
+        unsigned int number = 0;
         enum format_arg_type type;
 
         FDI_SET (format - 1, FMTDIR_START);
         spec.directives++;
 
+        if (isdigit (*format))
+          {
+            const char *f = format;
+            unsigned int m = 0;
+
+            do
+              {
+                m = 10 * m + (*f - '0');
+                f++;
+              }
+            while (isdigit (*f));
+
+            if (*f == '$')
+              {
+                if (m == 0)
+                  {
+                    *invalid_reason = INVALID_ARGNO_0 (spec.directives);
+                    FDI_SET (f, FMTDIR_ERROR);
+                    goto bad_format;
+                  }
+                number = m;
+                format = ++f;
+              }
+          }
+
+        /* Parse flags.  */
         while (*format == '-' || *format == '+' || *format == ' '
                || *format == '0' || *format == 'I')
           format++;
 
+        /* Parse width.  */
         while (isdigit (*format))
           format++;
 
@@ -152,15 +197,50 @@ format_parse (const char *format, bool translated, char *fdi,
             goto bad_format;
           }
 
-        if (*format != '%')
+        if (type != FAT_NONE)
           {
-            if (spec.allocated == spec.format_args_count)
+            if (number)
+              {
+                /* Numbered argument.  */
+
+                /* Numbered and unnumbered specifications are exclusive.  */
+                if (unnumbered_arg_count > 0)
+                  {
+                    *invalid_reason = INVALID_MIXES_NUMBERED_UNNUMBERED ();
+                    FDI_SET (format, FMTDIR_ERROR);
+                    goto bad_format;
+                  }
+
+                if (spec.allocated == spec.numbered_arg_count)
+                  {
+                    spec.allocated = 2 * spec.allocated + 1;
+                    spec.numbered = (struct numbered_arg *) xrealloc (spec.numbered, spec.allocated * sizeof (struct numbered_arg));
+                  }
+                spec.numbered[spec.numbered_arg_count].number = number;
+                spec.numbered[spec.numbered_arg_count].type = type;
+                spec.numbered_arg_count++;
+              }
+            else
               {
-                spec.allocated = 2 * spec.allocated + 1;
-                spec.format_args = (enum format_arg_type *) xrealloc (spec.format_args, spec.allocated * sizeof (enum format_arg_type));
+                /* Unnumbered argument.  */
+
+                /* Numbered and unnumbered specifications are exclusive.  */
+                if (spec.numbered_arg_count > 0)
+                  {
+                    *invalid_reason = INVALID_MIXES_NUMBERED_UNNUMBERED ();
+                    FDI_SET (format, FMTDIR_ERROR);
+                    goto bad_format;
+                  }
+
+                if (spec.allocated == spec.numbered_arg_count)
+                  {
+                    spec.allocated = 2 * spec.allocated + 1;
+                    spec.numbered = (struct numbered_arg *) xrealloc (spec.numbered, spec.allocated * sizeof (struct numbered_arg));
+                  }
+                spec.numbered[unnumbered_arg_count].number = unnumbered_arg_count + 1;
+                spec.numbered[unnumbered_arg_count].type = type;
+                unnumbered_arg_count++;
               }
-            spec.format_args[spec.format_args_count] = type;
-            spec.format_args_count++;
           }
 
         FDI_SET (format, FMTDIR_END);
@@ -168,13 +248,63 @@ format_parse (const char *format, bool translated, char *fdi,
         format++;
       }
 
+  /* Convert the unnumbered argument array to numbered arguments.  */
+  if (unnumbered_arg_count > 0)
+    spec.numbered_arg_count = unnumbered_arg_count;
+  /* Sort the numbered argument array, and eliminate duplicates.  */
+  else if (spec.numbered_arg_count > 1)
+    {
+      unsigned int i, j;
+      bool err;
+
+      qsort (spec.numbered, spec.numbered_arg_count,
+             sizeof (struct numbered_arg), numbered_arg_compare);
+
+      /* Remove duplicates: Copy from i to j, keeping 0 <= j <= i.  */
+      err = false;
+      for (i = j = 0; i < spec.numbered_arg_count; i++)
+        if (j > 0 && spec.numbered[i].number == spec.numbered[j-1].number)
+          {
+            enum format_arg_type type1 = spec.numbered[i].type;
+            enum format_arg_type type2 = spec.numbered[j-1].type;
+            enum format_arg_type type_both;
+
+            if (type1 == type2)
+              type_both = type1;
+            else
+              {
+                /* Incompatible types.  */
+                type_both = FAT_NONE;
+                if (!err)
+                  *invalid_reason =
+                    INVALID_INCOMPATIBLE_ARG_TYPES (spec.numbered[i].number);
+                err = true;
+              }
+
+            spec.numbered[j-1].type = type_both;
+          }
+        else
+          {
+            if (j < i)
+              {
+                spec.numbered[j].number = spec.numbered[i].number;
+                spec.numbered[j].type = spec.numbered[i].type;
+              }
+            j++;
+          }
+      spec.numbered_arg_count = j;
+      if (err)
+        /* *invalid_reason has already been set above.  */
+        goto bad_format;
+    }
+
   result = XMALLOC (struct spec);
   *result = spec;
   return result;
 
  bad_format:
-  if (spec.format_args != NULL)
-    free (spec.format_args);
+  if (spec.numbered != NULL)
+    free (spec.numbered);
   return NULL;
 }
 
@@ -183,8 +313,8 @@ format_free (void *descr)
 {
   struct spec *spec = (struct spec *) descr;
 
-  if (spec->format_args != NULL)
-    free (spec->format_args);
+  if (spec->numbered != NULL)
+    free (spec->numbered);
   free (spec);
 }
 
@@ -205,30 +335,68 @@ format_check (void *msgid_descr, void *msgstr_descr, bool equality,
   struct spec *spec2 = (struct spec *) msgstr_descr;
   bool err = false;
 
-  if (spec1->format_args_count + spec2->format_args_count > 0)
+  if (spec1->numbered_arg_count + spec2->numbered_arg_count > 0)
     {
-      unsigned int i;
+      unsigned int i, j;
+      unsigned int n1 = spec1->numbered_arg_count;
+      unsigned int n2 = spec2->numbered_arg_count;
 
-      /* Check the argument types are the same.  */
-      if (spec1->format_args_count != spec2->format_args_count)
+      /* Check the argument names are the same.
+         Both arrays are sorted.  We search for the first difference.  */
+      for (i = 0; i < spec2->numbered_arg_count; i++)
         {
-          if (error_logger)
-            error_logger (_("number of format specifications in '%s' and '%s' does not match"),
-                          pretty_msgid, pretty_msgstr);
-          err = true;
-        }
-      else
-        for (i = 0; i < spec2->format_args_count; i++)
-          if (!(spec1->format_args[i] == spec2->format_args[i]
-                || (!equality
-                    && (spec1->format_args[i] == FAT_ANY
-                        || spec2->format_args[i] == FAT_ANY))))
+          int cmp = (i >= n1 ? 1 :
+                     j >= n2 ? -1 :
+                     spec1->numbered[i].number > spec2->numbered[j].number ? 1 :
+                     spec1->numbered[i].number < spec2->numbered[j].number ? -1 :
+                     0);
+          if (cmp > 0)
             {
               if (error_logger)
-                error_logger (_("format specifications in '%s' and '%s' for argument %u are not the same"),
-                              pretty_msgid, pretty_msgstr, i + 1);
+                error_logger (_("a format specification for argument %u, as in '%s', doesn't exist in '%s'"),
+                              spec2->numbered[j].number, pretty_msgstr,
+                              pretty_msgid);
               err = true;
+              break;
             }
+          else if (cmp < 0)
+            {
+              if (equality)
+                {
+                  if (error_logger)
+                    error_logger (_("a format specification for argument %u doesn't exist in '%s'"),
+                                  spec1->numbered[i].number, pretty_msgstr);
+                  err = true;
+                  break;
+                }
+              else
+                i++;
+            }
+          else
+            j++, i++;
+        }
+      /* Check the argument types are the same.  */
+      if (!err)
+        for (i = 0, j = 0; j < n2; )
+          {
+            if (spec1->numbered[i].number == spec2->numbered[j].number)
+              {
+                if (!equality
+                    && (spec1->numbered[i].type == FAT_ANY
+                        || spec2->numbered[i].type == FAT_ANY))
+                  {
+                    if (error_logger)
+                      error_logger (_("format specifications in '%s' and '%s' for argument %u are not the same"),
+                                    pretty_msgid, pretty_msgstr,
+                                    spec2->numbered[j].number);
+                    err = true;
+                    break;
+                  }
+                j++, i++;
+              }
+            else
+              i++;
+          }
     }
 
   return err;
@@ -265,11 +433,11 @@ format_print (void *descr)
     }
 
       printf ("(");
-      for (i = 0; i < spec->format_args_count; i++)
+      for (i = 0; i < spec->numbered_arg_count; i++)
         {
           if (i > 0)
             printf (" ");
-          switch (spec->format_args[i])
+          switch (spec->numbered[i].type)
             {
             case FAT_ANY:
               printf ("*");
diff --git a/gettext-tools/tests/format-javascript-1 b/gettext-tools/tests/format-javascript-1
index d53df29..6e434ea 100755
--- a/gettext-tools/tests/format-javascript-1
+++ b/gettext-tools/tests/format-javascript-1
@@ -42,6 +42,18 @@ cat <<\EOF > f-js-1.data
 "abc%.4.2f"
 # Valid: three arguments
 "abc%d%j%j"
+# Valid: a numbered argument
+"abc%1$d"
+# Invalid: zero
+"abc%0$d"
+# Valid: two-digit numbered arguments
+"abc%11$def%10$dgh%9$dij%8$dkl%7$dmn%6$dop%5$dqr%4$dst%3$duv%2$dwx%1$dyz"
+# Invalid: unterminated number
+"abc%1"
+# Invalid: flags before number
+"abc%+1$d"
+# Invalid: mixing of numbered and unnumbered arguments
+"abc%d%2$x"
 EOF
 
 : ${XGETTEXT=xgettext}
diff --git a/gettext-tools/tests/lang-javascript b/gettext-tools/tests/lang-javascript
index fd018a6..5f502aa 100755
--- a/gettext-tools/tests/lang-javascript
+++ b/gettext-tools/tests/lang-javascript
@@ -44,7 +44,7 @@ msgstr "«Votre commande, s'il vous plait», dit le garçon."
 # Reverse the arguments.
 #, javascript-format
 msgid "%s is replaced by %s."
-msgstr "%s remplace %s."
+msgstr "%2$s remplace %1$s."
 EOF
 
 : ${MSGMERGE=msgmerge}
@@ -66,7 +66,9 @@ ${MSGFMT} -o fr/LC_MESSAGES/prog.mo fr.po
 (gjs -c imports.gettext) >/dev/null 2>/dev/null \
   || { echo "Skipping test: gjs gettext module not found"; exit 77; }
 (gjs -c imports.format) >/dev/null 2>/dev/null \
-  || { echo "Skipping test: gjs format module not found"; exit 77; }
+    || { echo "Skipping test: gjs format module not found"; exit 77; }
+(pkg-config gjs-1.0 --atleast-version=1.40) >/dev/null 2>/dev/null \
+    || { echo "Skipping test: gjs version is older than 1.40"; exit 77; }
 
 # Test which of the fr_FR locales are installed.
 : ${LOCALE_FR=fr_FR}
@@ -99,11 +101,11 @@ fi
 : ${DIFF=diff}
 cat <<\EOF > prog.ok
 «Votre commande, s'il vous plait», dit le garçon.
-FF remplace EUR.
+EUR remplace FF.
 EOF
 cat <<\EOF > prog.oku
 «Votre commande, s'il vous plait», dit le garçon.
-FF remplace EUR.
+EUR remplace FF.
 EOF
 
 : ${LOCALE_FR=fr_FR}