From bf84f580deeab73e5a70c1f0c7d55a24f3a1cb99 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sun, 18 Sep 2022 13:07:13 -0700 Subject: [PATCH] NUMERIC, STRING, LEAVE: Improve error messages. --- src/language/dictionary/numeric.c | 103 +++++++++++---------------- tests/automake.mk | 3 + tests/language/dictionary/leave.at | 29 ++++++++ tests/language/dictionary/numeric.at | 60 ++++++++++++++++ tests/language/dictionary/string.at | 60 ++++++++++++++++ 5 files changed, 194 insertions(+), 61 deletions(-) create mode 100644 tests/language/dictionary/leave.at create mode 100644 tests/language/dictionary/numeric.at create mode 100644 tests/language/dictionary/string.at diff --git a/src/language/dictionary/numeric.c b/src/language/dictionary/numeric.c index 18728014f3..4bf1ef94a1 100644 --- a/src/language/dictionary/numeric.c +++ b/src/language/dictionary/numeric.c @@ -37,33 +37,31 @@ int cmd_numeric (struct lexer *lexer, struct dataset *ds) { - size_t i; - - /* Names of variables to create. */ - char **v; - size_t nv; - do { - /* Format spec for variables to create. */ - struct fmt_spec f; - + char **v; + size_t nv; + int vars_start = lex_ofs (lexer); if (!parse_DATA_LIST_vars (lexer, dataset_dict (ds), &v, &nv, PV_NO_DUPLICATE)) return CMD_FAILURE; + int vars_end = lex_ofs (lexer) - 1; + + bool ok = false; /* Get the optional format specification. */ + struct fmt_spec f = var_default_formats (0); if (lex_match (lexer, T_LPAREN)) { if (!parse_format_specifier (lexer, &f)) - goto fail; + goto done; char *error = fmt_check_output__ (&f); if (error) { lex_next_error (lexer, -1, -1, "%s", error); free (error); - goto fail; + goto done; } if (fmt_is_string (f.type)) @@ -72,114 +70,99 @@ cmd_numeric (struct lexer *lexer, struct dataset *ds) lex_next_error (lexer, -1, -1, _("Format type %s may not be used with a numeric " "variable."), fmt_to_string (&f, str)); - goto fail; + goto done; } if (!lex_match (lexer, T_RPAREN)) { lex_error_expecting (lexer, "`)'"); - goto fail; + goto done; } } - else - f = var_default_formats (0); /* Create each variable. */ - for (i = 0; i < nv; i++) + for (size_t i = 0; i < nv; i++) { - struct variable *new_var = dict_create_var (dataset_dict (ds), v[i], 0); + struct variable *new_var = dict_create_var (dataset_dict (ds), + v[i], 0); if (!new_var) - msg (SE, _("There is already a variable named %s."), v[i]); + lex_ofs_error (lexer, vars_start, vars_end, + _("There is already a variable named %s."), v[i]); else var_set_both_formats (new_var, &f); } + ok = true; - /* Clean up. */ - for (i = 0; i < nv; i++) + done: + for (size_t i = 0; i < nv; i++) free (v[i]); free (v); + if (!ok) + return CMD_FAILURE; } while (lex_match (lexer, T_SLASH)); return CMD_SUCCESS; - - /* If we have an error at a point where cleanup is required, - flow-of-control comes here. */ -fail: - for (i = 0; i < nv; i++) - free (v[i]); - free (v); - return CMD_FAILURE; } /* Parses the STRING command. */ int cmd_string (struct lexer *lexer, struct dataset *ds) { - size_t i; - - /* Names of variables to create. */ - char **v; - size_t nv; - - /* Format spec for variables to create. */ - struct fmt_spec f; - - /* Width of variables to create. */ - int width; - do { + char **v; + size_t nv; + int vars_start = lex_ofs (lexer); if (!parse_DATA_LIST_vars (lexer, dataset_dict (ds), &v, &nv, PV_NO_DUPLICATE)) return CMD_FAILURE; + int vars_end = lex_ofs (lexer) - 1; + bool ok = false; + + struct fmt_spec f; if (!lex_force_match (lexer, T_LPAREN) || !parse_format_specifier (lexer, &f)) - goto fail; + goto done; char *error = fmt_check_type_compat__ (&f, NULL, VAL_STRING); if (!error) error = fmt_check_output__ (&f); if (error) { - lex_next_error (lexer, -2, -2, "%s", error); + lex_next_error (lexer, -1, -1, "%s", error); free (error); - goto fail; + goto done; } if (!lex_force_match (lexer, T_RPAREN)) - goto fail; - - width = fmt_var_width (&f); + goto done; /* Create each variable. */ - for (i = 0; i < nv; i++) + int width = fmt_var_width (&f); + for (size_t i = 0; i < nv; i++) { struct variable *new_var = dict_create_var (dataset_dict (ds), v[i], width); if (!new_var) - msg (SE, _("There is already a variable named %s."), v[i]); + lex_ofs_error (lexer, vars_start, vars_end, + _("There is already a variable named %s."), v[i]); else var_set_both_formats (new_var, &f); } + ok = true; - /* Clean up. */ - for (i = 0; i < nv; i++) + done: + for (size_t i = 0; i < nv; i++) free (v[i]); free (v); + if (!ok) + return CMD_FAILURE; } while (lex_match (lexer, T_SLASH)); return CMD_SUCCESS; - - /* If we have an error at a point where cleanup is required, - flow-of-control comes here. */ -fail: - for (i = 0; i < nv; i++) - free (v[i]); - free (v); - return CMD_FAILURE; } /* Parses the LEAVE command. */ @@ -189,11 +172,9 @@ cmd_leave (struct lexer *lexer, struct dataset *ds) struct variable **v; size_t nv; - size_t i; - if (!parse_variables (lexer, dataset_dict (ds), &v, &nv, PV_NONE)) return CMD_CASCADING_FAILURE; - for (i = 0; i < nv; i++) + for (size_t i = 0; i < nv; i++) var_set_leave (v[i], true); free (v); diff --git a/tests/automake.mk b/tests/automake.mk index e1889532e0..6cfb3e0eee 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -376,11 +376,14 @@ TESTSUITE_AT = \ tests/language/dictionary/apply.at \ tests/language/dictionary/delete-variables.at \ tests/language/dictionary/formats.at \ + tests/language/dictionary/leave.at \ tests/language/dictionary/missing-values.at \ tests/language/dictionary/mrsets.at \ + tests/language/dictionary/numeric.at \ tests/language/dictionary/rename-variables.at \ tests/language/dictionary/sort-variables.at \ tests/language/dictionary/split-file.at \ + tests/language/dictionary/string.at \ tests/language/dictionary/sys-file-info.at \ tests/language/dictionary/value-labels.at \ tests/language/dictionary/variable-display.at \ diff --git a/tests/language/dictionary/leave.at b/tests/language/dictionary/leave.at new file mode 100644 index 0000000000..c9d13a2861 --- /dev/null +++ b/tests/language/dictionary/leave.at @@ -0,0 +1,29 @@ +AT_BANNER([LEAVE]) + +AT_SETUP([LEAVE]) +AT_DATA([leave.sps], [dnl +DATA LIST LIST NOTABLE/x y z. +LEAVE x y. +]) +AT_CHECK([pspp -O format=csv leave.sps]) +AT_CLEANUP + +AT_SETUP([LEAVE syntax errors]) +AT_DATA([leave.sps], [dnl +DATA LIST LIST NOTABLE/x y z. +LEAVE **. +LEAVE x **. +]) +AT_DATA([insert.sps], [dnl +INSERT FILE='leave.sps' ERROR=IGNORE. +]) +AT_CHECK([pspp --testing-mode -O format=csv insert.sps], [1], [dnl +"leave.sps:2.7-2.8: error: LEAVE: Syntax error expecting variable name. + 2 | LEAVE **. + | ^~" + +"leave.sps:3.9-3.10: error: LEAVE: Syntax error expecting end of command. + 3 | LEAVE x **. + | ^~" +]) +AT_CLEANUP diff --git a/tests/language/dictionary/numeric.at b/tests/language/dictionary/numeric.at new file mode 100644 index 0000000000..b926ebc4c4 --- /dev/null +++ b/tests/language/dictionary/numeric.at @@ -0,0 +1,60 @@ +AT_BANNER([NUMERIC]) + +AT_SETUP([NUMERIC]) +AT_DATA([numeric.sps], [dnl +DATA LIST LIST NOTABLE/x y z. +NUMERIC n/k(F5). +DISPLAY DICTIONARY. +]) +AT_CHECK([pspp -O format=csv numeric.sps], [0], [dnl +Table: Variables +Name,Position,Measurement Level,Role,Width,Alignment,Print Format,Write Format +x,1,Unknown,Input,8,Right,F8.2,F8.2 +y,2,Unknown,Input,8,Right,F8.2,F8.2 +z,3,Unknown,Input,8,Right,F8.2,F8.2 +n,4,Unknown,Input,8,Right,F8.2,F8.2 +k,5,Unknown,Input,8,Right,F5.0,F5.0 +]) +AT_CLEANUP + +AT_SETUP([NUMERIC syntax errors]) +AT_DATA([numeric.sps], [dnl +DATA LIST LIST NOTABLE/x y z. +NUMERIC **. +NUMERIC n **. +NUMERIC x. +NUMERIC n (**). +NUMERIC n (F50). +NUMERIC n (A8). +NUMERIC n (F8.0 **). +]) +AT_CHECK([pspp -O format=csv numeric.sps], [1], [dnl +"numeric.sps:2.9-2.10: error: NUMERIC: Syntax error expecting variable name. + 2 | NUMERIC **. + | ^~" + +"numeric.sps:3.11-3.12: error: NUMERIC: Syntax error expecting end of command. + 3 | NUMERIC n **. + | ^~" + +"numeric.sps:4.9: error: NUMERIC: There is already a variable named x. + 4 | NUMERIC x. + | ^" + +"numeric.sps:5.12-5.13: error: NUMERIC: Syntax error expecting valid format specifier. + 5 | NUMERIC n (**). + | ^~" + +"numeric.sps:6.12-6.14: error: NUMERIC: Output format F50.0 specifies width 50, but F requires a width between 1 and 40. + 6 | NUMERIC n (F50). + | ^~~" + +"numeric.sps:7.12-7.13: error: NUMERIC: Format type A8 may not be used with a numeric variable. + 7 | NUMERIC n (A8). + | ^~" + +"numeric.sps:8.17-8.18: error: NUMERIC: Syntax error expecting `@:}@'. + 8 | NUMERIC n (F8.0 **). + | ^~" +]) +AT_CLEANUP diff --git a/tests/language/dictionary/string.at b/tests/language/dictionary/string.at new file mode 100644 index 0000000000..febf81f6e6 --- /dev/null +++ b/tests/language/dictionary/string.at @@ -0,0 +1,60 @@ +AT_BANNER([STRING]) + +AT_SETUP([STRING]) +AT_DATA([string.sps], [dnl +DATA LIST LIST NOTABLE/x y z. +STRING s1 (A8)/s2 (A1). +DISPLAY DICTIONARY. +]) +AT_CHECK([pspp -O format=csv string.sps], [0], [dnl +Table: Variables +Name,Position,Measurement Level,Role,Width,Alignment,Print Format,Write Format +x,1,Unknown,Input,8,Right,F8.2,F8.2 +y,2,Unknown,Input,8,Right,F8.2,F8.2 +z,3,Unknown,Input,8,Right,F8.2,F8.2 +s1,4,Nominal,Input,8,Left,A8,A8 +s2,5,Nominal,Input,1,Left,A1,A1 +]) +AT_CLEANUP + +AT_SETUP([STRING syntax errors]) +AT_DATA([string.sps], [dnl +DATA LIST LIST NOTABLE/x y z. +STRING **. +STRING s **. +STRING s (**). +STRING s (F8). +STRING s (AHEX1). +STRING s (A8 **). +STRING x (A8). +]) +AT_CHECK([pspp -O format=csv string.sps], [1], [dnl +"string.sps:2.8-2.9: error: STRING: Syntax error expecting variable name. + 2 | STRING **. + | ^~" + +"string.sps:3.10-3.11: error: STRING: Syntax error expecting `('. + 3 | STRING s **. + | ^~" + +"string.sps:4.11-4.12: error: STRING: Syntax error expecting valid format specifier. + 4 | STRING s (**). + | ^~" + +"string.sps:5.11-5.12: error: STRING: String variables are not compatible with numeric format F8.0. + 5 | STRING s (F8). + | ^~" + +"string.sps:6.11-6.15: error: STRING: Output format AHEX1 specifies width 1, but AHEX requires an even width. + 6 | STRING s (AHEX1). + | ^~~~~" + +"string.sps:7.14-7.15: error: STRING: Syntax error expecting `)'. + 7 | STRING s (A8 **). + | ^~" + +"string.sps:8.8: error: STRING: There is already a variable named x. + 8 | STRING x (A8). + | ^" +]) +AT_CLEANUP -- 2.30.2