expressions: Improve error messages.

Also, introduce tests to trigger the error messages.

Prompted by bug #59697.
Thanks to Stéphane Aulery for reporting the poor error messages.

diff --git a/src/language/expressions/generate.pl b/src/language/expressions/generate.pl
index 228095db9443c098f8929c9da6fcf97e002cbf1f..6f41d7e84215affd05318f4b84068533c043b896 100644 (file)
--- a/src/language/expressions/generate.pl
+++ b/src/language/expressions/generate.pl
@@ -561,8 +561,8 @@ sub parse_arg {
            $arg{IDX} = force ('id');
            if (match ('*')) {
                $arg{TIMES} = force ('int');
-               die "multiplication factor must be positive"
-                 if $arg{TIMES} < 1;
+               die "multiplication factor must be two"
+                 if $arg{TIMES} != 2;
            } else {
                $arg{TIMES} = 1;
            }

diff --git a/src/language/expressions/parse.c b/src/language/expressions/parse.c
index 906423650753a45d3b3fd9328b82858de57f6ce2..79889082ea89dd88018d574ba6161b908d7f8725 100644 (file)
--- a/src/language/expressions/parse.c
+++ b/src/language/expressions/parse.c
@@ -1096,23 +1096,21 @@ coerce_function_args (struct expression *e, const struct operation *f,
 static bool
 validate_function_args (const struct operation *f, int arg_cnt, int min_valid)
 {
+  /* Count the function arguments that go into the trailing array (if any).  We
+     know that there must be at least the minimum number because
+     match_function() already checked. */
   int array_arg_cnt = arg_cnt - (f->arg_cnt - 1);
-  if (array_arg_cnt < f->array_min_elems)
-    {
-      msg (SE, _("%s must have at least %d arguments in list."),
-           f->prototype, f->array_min_elems);
-      return false;
-    }
+  assert (array_arg_cnt >= f->array_min_elems);
 
   if ((f->flags & OPF_ARRAY_OPERAND)
       && array_arg_cnt % f->array_granularity != 0)
     {
-      if (f->array_granularity == 2)
-        msg (SE, _("%s must have an even number of arguments in list."),
-             f->prototype);
-      else
-        msg (SE, _("%s must have multiple of %d arguments in list."),
-             f->prototype, f->array_granularity);
+      /* RANGE is the only case we have so far.  It has paired arguments with
+         one initial argument, and that's the only special case we deal with
+         here. */
+      assert (f->array_granularity == 2);
+      assert (arg_cnt % 2 == 0);
+      msg (SE, _("%s must have an odd number of arguments."), f->prototype);
       return false;
     }
 
@@ -1121,26 +1119,19 @@ validate_function_args (const struct operation *f, int arg_cnt, int min_valid)
       if (f->array_min_elems == 0)
         {
           assert ((f->flags & OPF_MIN_VALID) == 0);
-          msg (SE, _("%s function does not accept a minimum valid "
-                     "argument count."), f->prototype);
+          msg (SE, _("%s function cannot accept suffix .%d to specify the "
+                     "minimum number of valid arguments."),
+               f->prototype, min_valid);
           return false;
         }
       else
         {
           assert (f->flags & OPF_MIN_VALID);
-          if (array_arg_cnt < f->array_min_elems)
-            {
-              msg (SE, _("%s requires at least %d valid arguments in list."),
-                   f->prototype, f->array_min_elems);
-              return false;
-            }
-          else if (min_valid > array_arg_cnt)
+          if (min_valid > array_arg_cnt)
             {
-              msg (SE, _("With %s, "
-                         "using minimum valid argument count of %d "
-                         "does not make sense when passing only %d "
-                         "arguments in list."),
-                   f->prototype, min_valid, array_arg_cnt);
+              msg (SE, _("For %s with %d arguments, at most %d (not %d) may be "
+                         "required to be valid."),
+                   f->prototype, arg_cnt, array_arg_cnt, min_valid);
               return false;
             }
         }

diff --git a/tests/language/expressions/evaluate.at b/tests/language/expressions/evaluate.at
index d9e532e1d33df932a3e2b0605c8368cdd89cfec8..ac4d93e7048f4aad16f1d5309ce2c0347389256e 100644 (file)
--- a/tests/language/expressions/evaluate.at
+++ b/tests/language/expressions/evaluate.at
@@ -15,6 +15,7 @@ dnl You should have received a copy of the GNU General Public License
 dnl along with this program.  If not, see <http://www.gnu.org/licenses/>.
 m4_define([CHECK_EXPR_EVAL],
   [AT_SETUP([expressions - $1])
+   AT_KEYWORDS([expression])
    AT_DATA([evaluate.sps],
      [set mxwarn 1000.
 set mxerr 1000.
@@ -562,11 +563,11 @@ ANY(string, string[, string]...).]],
 RANGE(number, number, number[, number, number]...)
 RANGE(string, string, string[, string, string]...).]],
   [[range(1, 2)], [error],
-   [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an even number of arguments in list.]],
+   [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an odd number of arguments.]],
   [[range(1, 2, 3, 4)], [error],
-   [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an even number of arguments in list.]],
+   [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an odd number of arguments.]],
   [[range(1, 2, 3, 4, 5, 6)], [error],
-   [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an even number of arguments in list.]],
+   [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an odd number of arguments.]],
   [[range('1', 2, 3)], [error],
    [error: DEBUG EVALUATE: Function invocation range(string, number, number) does not match any known function.  Candidates are:
 RANGE(number, number, number[, number, number]...)
@@ -605,11 +606,11 @@ RANGE(string, string, string[, string, string]...).]],
 RANGE(number, number, number[, number, number]...)
 RANGE(string, string, string[, string, string]...).]],
   [[range('1', '2')], [error],
-   [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an even number of arguments in list.]],
+   [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an odd number of arguments.]],
   [[range('1', '2', '3', '4')], [error],
-   [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an even number of arguments in list.]],
+   [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an odd number of arguments.]],
   [[range('1', '2', '3', '4', '5', '6')], [error],
-   [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an even number of arguments in list.]],
+   [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an odd number of arguments.]],
   [[range(1, '2', '3')], [error],
    [error: DEBUG EVALUATE: Function invocation range(number, string, string) does not match any known function.  Candidates are:
 RANGE(number, number, number[, number, number]...)
@@ -635,7 +636,7 @@ MAX(string[, string]...).]],
   [[max(1, 2, 3, $sysmis)], [3.00]],
   [[max.4(1, 2, 3, $sysmis)], [sysmis]],
   [[max.4(1, 2, 3)], [error],
-   [error: DEBUG EVALUATE: With MAX(number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]],
+   [error: DEBUG EVALUATE: For MAX(number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]],
 
   [[max("2", "3", "5", "1", "4")], ["5"]],
   [[max("1", "2")], ["2"]],
@@ -653,7 +654,7 @@ MIN(string[, string]...).]],
   [[min(1, 2, 3, $sysmis)], [1.00]],
   [[min.4(1, 2, 3, $sysmis)], [sysmis]],
   [[min.4(1, 2, 3)], [error],
-   [error: DEBUG EVALUATE: With MIN(number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]],
+   [error: DEBUG EVALUATE: For MIN(number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]],
 
   [[min("2", "3", "5", "1", "4")], ["1"]],
   [[min("1", "2")], ["1"]],
@@ -669,7 +670,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance],
   [[cfvar(1, 2, 3, $sysmis)], [0.50]],
   [[cfvar.4(1, 2, 3, $sysmis)], [sysmis]],
   [[cfvar.4(1, 2, 3)], [error],
-   [error: DEBUG EVALUATE: With CFVAR(number, number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]],
+   [error: DEBUG EVALUATE: For CFVAR(number, number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]],
   [[cfvar('x')], [error],
    [error: DEBUG EVALUATE: Type mismatch invoking CFVAR(number, number[, number]...) as cfvar(string).]],
   [[cfvar('x', 1, 2, 3)], [error],
@@ -685,7 +686,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance],
   [[mean(1, 2, 3, $sysmis)], [2.00]],
   [[mean.4(1, 2, 3, $sysmis)], [sysmis]],
   [[mean.4(1, 2, 3)], [error],
-   [error: DEBUG EVALUATE: With MEAN(number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]],
+   [error: DEBUG EVALUATE: For MEAN(number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]],
 
   [[median(1, 2, 3, 4, 5)], [3.00]],
   [[median(2, 3, 4, 5, 1)], [3.00]],
@@ -716,7 +717,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance],
   [[sd(1, 2, 3, $sysmis)], [1.00]],
   [[sd.4(1, 2, 3, $sysmis)], [sysmis]],
   [[sd.4(1, 2, 3)], [error],
-   [error: DEBUG EVALUATE: With SD(number, number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]],
+   [error: DEBUG EVALUATE: For SD(number, number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]],
   [[sd('x')], [error],
    [error: DEBUG EVALUATE: Type mismatch invoking SD(number, number[, number]...) as sd(string).]],
   [[sd('x', 1, 2, 3)], [error],
@@ -732,7 +733,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance],
   [[sum(1, 2, 3, $sysmis)], [6.00]],
   [[sum.4(1, 2, 3, $sysmis)], [sysmis]],
   [[sum.4(1, 2, 3)], [error],
-   [error: DEBUG EVALUATE: With SUM(number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]],
+   [error: DEBUG EVALUATE: For SUM(number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]],
 
   [[variance(1, 2, 3, 4, 5)], [2.50]],
   [[variance(1, $sysmis, 2, 3, $sysmis, 4, 5)], [2.50]],
@@ -743,7 +744,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance],
   [[variance(1, 2, 3, $sysmis)], [1.00]],
   [[variance.4(1, 2, 3, $sysmis)], [sysmis]],
   [[variance.4(1, 2, 3)], [error],
-   [error: DEBUG EVALUATE: With VARIANCE(number, number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]],
+   [error: DEBUG EVALUATE: For VARIANCE(number, number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]],
   [[variance('x')], [error],
    [error: DEBUG EVALUATE: Type mismatch invoking VARIANCE(number, number[, number]...) as variance(string).]],
   [[variance('x', 1, 2, 3)], [error],
@@ -1986,6 +1987,14 @@ dnl Tests correctness of generic optimizations in optimize_tree().
   [[x ** 1], [5.00], [], [(X = 5.00)]],
   [[x ** 2], [25.00], [], [(X = 5.00)]])
 
+CHECK_EXPR_EVAL([negative checks],
+  [[$nonexistent], [error], [error: DEBUG EVALUATE: Unknown system variable $nonexistent.]],
+  [[RANGE(1, 2)], [error], [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an odd number of arguments.]],
+  [[CONCAT.1('a', 'b')], [error], [error: DEBUG EVALUATE: CONCAT(string[, string]...) function cannot accept suffix .1 to specify the minimum number of valid arguments.]],
+  [[foobar(x)], [error], [error: DEBUG EVALUATE: No function or vector named foobar.]],
+  [[CONCAT.1('a' b)], [error], [error: DEBUG EVALUATE: Syntax error at `b': expecting `,' or `)'.]],
+  [[NCDF.CHISQ(1, 2, 3)], [error], [error: DEBUG EVALUATE: NCDF.CHISQ(number, number, number) is not available in this version of PSPP.]])
+
 AT_SETUP([LAG function])
 AT_DATA([lag.sps], [dnl
 data list /W 1.

diff --git a/tests/language/expressions/parse.at b/tests/language/expressions/parse.at
index cfa1db997e41e5f6dce03384db02a34991859683..e40a4fe873c3b910c53f8754134d1c1f42714b5e 100644 (file)
--- a/tests/language/expressions/parse.at
+++ b/tests/language/expressions/parse.at
@@ -38,6 +38,72 @@ parse.sps:11: error: Stopping syntax file processing here to avoid a cascade of
 ])
 AT_CLEANUP
 
+AT_SETUP([parsing numeric expression with type mismatch])
+AT_DATA([parse.sps], [dnl
+DATA LIST NOTABLE/x 1(A).
+IF 'foo'.
+])
+AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl
+"parse.sps:2: error: IF: Type mismatch: expression has string type, but a numeric value is required here."
+])
+AT_CLEANUP
+
+AT_SETUP([parsing string expression with type mismatch])
+AT_KEYWORDS([expression negative])
+AT_DATA([parse.sps], [dnl
+DATA LIST NOTABLE/x 1(A).
+COMPUTE x=1.
+])
+AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl
+"parse.sps:2: error: COMPUTE: Type mismatch: expression has number type, but a string value is required here."
+])
+AT_CLEANUP
+
+AT_SETUP([parse expression with unknown system variable])
+AT_KEYWORDS([expression negative])
+AT_DATA([parse.sps], [dnl
+DATA LIST NOTABLE/x 1.
+COMPUTE x=$nonexistent.
+])
+AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl
+parse.sps:2: error: COMPUTE: Unknown system variable $nonexistent.
+])
+AT_CLEANUP
+
+AT_SETUP([parse expression with unknown identifier])
+AT_KEYWORDS([expression negative])
+AT_DATA([parse.sps], [dnl
+DATA LIST NOTABLE/x 1.
+COMPUTE x=y.
+])
+AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl
+parse.sps:2: error: COMPUTE: Unknown identifier y.
+])
+AT_CLEANUP
+
+AT_SETUP([parse expression with extension function in compatibility mode])
+AT_KEYWORDS([expression negative])
+AT_DATA([parse.sps], [dnl
+DEBUG EVALUATE/ACOS(0)*0.
+])
+AT_CHECK([pspp --testing-mode --syntax=compatible -O format=csv parse.sps], [0], [dnl
+parse.sps:1: warning: DEBUG EVALUATE: ACOS(number) is a PSPP extension.
+0.00
+])
+AT_CLEANUP
+
+AT_SETUP([LAG expression following TEMPORARY])
+AT_KEYWORDS([expression negative])
+AT_DATA([parse.sps], [dnl
+DATA LIST NOTABLE/x 1.
+TEMPORARY
+COMPUTE y=LAG(x).
+])
+AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl
+parse.sps:3: error: COMPUTE: LAG(num_variable) may not appear after TEMPORARY.
+])
+AT_CLEANUP
+
 AT_SETUP([parse expression with invalid logical expression])
 AT_DATA([parse.sps], [dnl
 INPUT PROGRAM.