Add -fdiagnostics-show-caret to display the context of an error

NEWS

@@ -321,7 +321,7 @@ NEWS - user visible changes				-*- outline -*-
    original version; note: their use will be adjusted where they don't match
    GCC's same options in later versions, including addition of -M and -MD
 
-** New -std options:
+** new -std options:
 
    gcos            GCOS compatibility
    gcos-strict     GCOS compatibility                - strict
@@ -330,6 +330,14 @@ NEWS - user visible changes				-*- outline -*-
    dialect configuration options accompanying each specificity
    introduced by the dialect.
 
+** new diagnostic format for errors: the diagnostics now print the source
+   code context with a left margin showing line numbers, configurable with
+   -fno-diagnostics-show-line-numbers, and possible to disable completely
+   with -fno-diagnostics-show-caret;
+
+   the option -fdiagnostics-plain-output was added to request that diagnostic
+   output look as plain as possible and stay more stable over time
+
 * Important Bugfixes:
 
 ** for dialects other than the GnuCOBOL default different reserved "alias" words

cobc/ChangeLog

@@ -1,4 +1,18 @@
 
+2023-02-16  Fabrice Le Fessant <fabrice.le_fessant@ocamlpro.com>
+
+	* flag.def (cb_diagnostics_show_caret), error.c
+	  (diagnostics_show_caret): add argument -fno-diagnostics-show-caret
+	  to disable the display of the source context of the error/warning,
+	  2 lines before and after the location; add argument
+	  -fno-diagnostics-show-line-numbers to disable printing of
+	  line numbers in carets; rename cb_diagnostic_show_option into
+	  cb_diagnostics_show_option to match the argument name.
+	* error.c (cb_error_kind): replace all occurrences of a
+	  error/warning/note string by a symbolic enum.
+	* cobc.c: new argument -fdiagnostics-plain-output to make
+	  output as plain as possible (disabling carets for example)
+
 2023-05-31  Simon Sobisch <simonsobisch@gnu.org>
 
 	* codegen.c (output_init_comment_and_source_ref) [NO_INIT_SOURCE_LOC]:

cobc/cobc.c

@@ -589,6 +589,7 @@ static const struct option long_options[] = {
 	{"P",			CB_OP_ARG, NULL, 'P'},
 	{"Xref",		CB_NO_ARG, NULL, 'X'},
 	{"use-extfh",		CB_RQ_ARG, NULL, 9},	/* this is used by COBOL-IT; Same is -fcallfh= */
+	{"fdiagnostics-plain-output",	CB_NO_ARG, NULL, '/'},
 	{"Wall",		CB_NO_ARG, NULL, 'W'},
 	{"Wextra",		CB_NO_ARG, NULL, 'Y'},		/* this option used to be called -W */
 #if 1
@@ -3459,6 +3460,12 @@ process_command_line (const int argc, char **argv)
 			}
 			break;
 
+		case '/':
+			/* -fdiagnostics-plain-output */
+			cb_diagnostics_show_caret = 0 ;
+			cb_diagnostics_show_line_numbers = 0;
+			break;
+
 		case 'P':
 			/* -P : Generate preproc listing */
 			if (cob_optarg) {

cobc/error.c

@@ -32,6 +32,13 @@
 #include "cobc.h"
 #include "tree.h"
 
+enum cb_error_kind {
+	CB_KIND_ERROR,
+	CB_KIND_WARNING,
+	CB_KIND_NOTE,
+	CB_KIND_GENERAL
+};
+
 static char		*errnamebuff = NULL;
 static struct cb_label	*last_section = NULL;
 static struct cb_label	*last_paragraph = NULL;
@@ -63,10 +70,80 @@ print_error_prefix (const char *file, int line, const char *prefix)
 	}
 }
 
+/* Display a context around the location of the error/warning, only if
+ * cb_diagnostics_show_caret is true
+
+   Only display two lines before and after. No caret yet for the column as
+   we only have the line. Since we directly use the file, source is printed
+   before any REPLACE.
+ */
+
+#define CARET_MAX_COLS 73
+static void
+diagnostics_show_caret (const char *file, int line)
+{
+	FILE* fd = fopen(file, "r");
+	if (fd == NULL) return;
+	char buffer[ CARET_MAX_COLS+1 ];
+	int line_pos = 1;
+	int char_pos = 0;
+	int printed = 0; /* nothing printed */
+	while(1){
+		int c = fgetc (fd);
+		if ( c == EOF ){
+			if (printed){
+				fprintf(stderr, "\n");
+			}
+			fclose(fd);
+			return ;
+		}
+		buffer[char_pos] = c ;
+		if (c == '\n' || char_pos == CARET_MAX_COLS){
+			buffer[char_pos] = 0;
+			if (line_pos > line-3 && line_pos < line+3){
+				if (line_pos == line-2) fprintf(stderr, "\n");
+				printed = 1;
+				fprintf (stderr, "  ");
+				if (cb_diagnostics_show_line_numbers){
+					fprintf (stderr, "%04d ",
+						 line_pos);
+				}
+				fprintf (stderr, "%c %s%s\n",
+					 line == line_pos ? '>' : ' ',
+					 c == '\n' ? "" : ".." ,
+					 buffer);
+				if (line_pos == line+2){
+					fprintf(stderr, "\n");
+					fclose(fd);
+					return;
+				}
+			}
+			while (c != '\n'){
+				/* skip end of line too long */
+				c = fgetc (fd);
+				if( c == EOF ) { fclose(fd); return ; }
+			}
+			line_pos++;
+			char_pos=0;
+		} else {
+			char_pos++;
+		}
+	}
+}
+
 static void
-print_error (const char *file, int line, const char *prefix,
+print_error (const char *file, int line, enum cb_error_kind kind,
 	     const char *fmt, va_list ap, const char *diagnostic_option)
 {
+	const char *prefix;
+
+	switch( kind ){
+	case CB_KIND_ERROR: prefix = _("error: "); break;
+	case CB_KIND_WARNING: prefix = _("warning: "); break;
+	case CB_KIND_NOTE: prefix = _("note: "); break;
+	case CB_KIND_GENERAL: prefix = ""; break;
+	}
+
 	if (!file) {
 		file = cb_source_file;
 	}
@@ -119,12 +196,26 @@ print_error (const char *file, int line, const char *prefix,
 		}
 		cb_add_error_to_listing (file, line, prefix, errmsg);
 	}
+
+	static const char* last_caret_file = NULL ;
+	static int last_caret_line = -1 ;
+	if (cb_diagnostics_show_caret
+	    && file != NULL
+	    && strcmp (file, COB_DASH) != 0
+	    && line
+	    && (last_caret_file != file || last_caret_line != line)
+		){
+		/* remember last printed location to avoid reprinting it */
+		last_caret_file = file;
+		last_caret_line = line;
+		diagnostics_show_caret (file, line);
+	}
 }
 
 static void
 cobc_too_many_errors (void)
 {
-	if (!cb_diagnostic_show_option) {
+	if (!cb_diagnostics_show_option) {
 		fprintf (stderr, "cobc: %s\n",
 			_("too many errors"));
 	} else
@@ -306,7 +397,7 @@ static char *warning_option_text (const enum cb_warn_opt opt, const enum cb_warn
 {
 	const char *opt_name;
 
-	if (!cb_diagnostic_show_option) {
+	if (!cb_diagnostics_show_option) {
 		return NULL;
 	}
 	switch (opt) {
@@ -343,9 +434,9 @@ cb_warning_internal (const enum cb_warn_opt opt, const char *fmt, va_list ap)
 	}
 
 	if (pref != COBC_WARN_AS_ERROR) {
-		print_error (NULL, 0, _("warning: "),  fmt, ap, warning_option_text (opt, pref));
+		print_error (NULL, 0, CB_KIND_WARNING,  fmt, ap, warning_option_text (opt, pref));
 	} else {
-		print_error (NULL, 0, _("error: "),  fmt, ap, warning_option_text (opt, pref));
+		print_error (NULL, 0, CB_KIND_ERROR,  fmt, ap, warning_option_text (opt, pref));
 	}
 
 	if (sav_lst_file) {
@@ -379,7 +470,7 @@ cb_error_always (const char *fmt, ...)
 
 	cobc_in_repository = 0;
 	va_start (ap, fmt);
-	print_error (NULL, 0, _("error: "), fmt, ap, NULL);
+	print_error (NULL, 0, CB_KIND_ERROR, fmt, ap, NULL);
 	va_end (ap);
 
 	if (sav_lst_file) {
@@ -405,12 +496,12 @@ cb_error_internal (const char *fmt, va_list ap)
 	}
 
 	if (!ignore_error) {
-		print_error (NULL, 0, _("error: "), fmt, ap, NULL);
+		print_error (NULL, 0, CB_KIND_ERROR, fmt, ap, NULL);
 		ret = COBC_WARN_AS_ERROR;
 	} else if (pref == COBC_WARN_AS_ERROR) {
-		print_error (NULL, 0, _("error: "), fmt, ap, warning_option_text (opt, pref));
+		print_error (NULL, 0, CB_KIND_ERROR, fmt, ap, warning_option_text (opt, pref));
 	} else {
-		print_error (NULL, 0, _("warning: "), fmt, ap, warning_option_text (opt, pref));
+		print_error (NULL, 0, CB_KIND_WARNING, fmt, ap, warning_option_text (opt, pref));
 	}
 
 	if (sav_lst_file) {
@@ -447,7 +538,7 @@ cb_perror (const int config_error, const char *fmt, ...)
 	}
 
 	va_start (ap, fmt);
-	print_error (NULL, 0, "", fmt, ap, NULL);
+	print_error (NULL, 0, CB_KIND_GENERAL, fmt, ap, NULL);
 	va_end (ap);
 
 
@@ -472,9 +563,9 @@ cb_plex_warning (const enum cb_warn_opt opt, const size_t sline, const char *fmt
 
 	va_start (ap, fmt);
 	if (pref != COBC_WARN_AS_ERROR) {
-		print_error (NULL, cb_source_line + (int)sline, _("warning: "), fmt, ap, warning_option_text (opt, pref));
+		print_error (NULL, cb_source_line + (int)sline, CB_KIND_WARNING, fmt, ap, warning_option_text (opt, pref));
 	} else {
-		print_error (NULL, cb_source_line + (int)sline, _("error: "), fmt, ap, warning_option_text (opt, pref));
+		print_error (NULL, cb_source_line + (int)sline, CB_KIND_ERROR, fmt, ap, warning_option_text (opt, pref));
 	}
 	va_end (ap);
 
@@ -496,7 +587,7 @@ cb_plex_error (const size_t sline, const char *fmt, ...)
 	va_list ap;
 
 	va_start (ap, fmt);
-	print_error (NULL, cb_source_line + (int)sline, ("error: "), fmt, ap, NULL);
+	print_error (NULL, cb_source_line + (int)sline, CB_KIND_ERROR, fmt, ap, NULL);
 	va_end (ap);
 
 	if (sav_lst_file) {
@@ -628,7 +719,7 @@ cb_warning_x_internal (const enum cb_warn_opt opt, cb_tree x, const char *fmt, v
 	}
 
 	print_error (x->source_file, x->source_line,
-		pref == COBC_WARN_AS_ERROR ? _("error: ") : _("warning: "),
+		pref == COBC_WARN_AS_ERROR ? CB_KIND_ERROR : CB_KIND_WARNING,
 		fmt, ap, warning_option_text (opt, pref));
 
 	if (sav_lst_file) {
@@ -672,7 +763,7 @@ cb_warning_dialect_x (const enum cb_support tag, cb_tree x, const char *fmt, ...
 
 	va_start (ap, fmt);
 	print_error (x->source_file, x->source_line,
-		ret == COBC_WARN_AS_ERROR ? _("error: ") : _("warning: "),
+		ret == COBC_WARN_AS_ERROR ? CB_KIND_ERROR : CB_KIND_WARNING,
 		fmt, ap, NULL);
 	va_end (ap);
 
@@ -725,10 +816,10 @@ cb_note_x (const enum cb_warn_opt opt, cb_tree x, const char *fmt, ...)
 	listprint_suppress ();
 	va_start (ap, fmt);
 	if (opt != COB_WARNOPT_NONE) {
-		print_error (x->source_file, x->source_line, _("note: "),
+		print_error (x->source_file, x->source_line, CB_KIND_NOTE,
 			fmt, ap, warning_option_text (opt, pref));
 	} else {
-		print_error (x->source_file, x->source_line, _("note: "),
+		print_error (x->source_file, x->source_line, CB_KIND_NOTE,
 			fmt, ap, NULL);
 	}
 	va_end (ap);
@@ -752,10 +843,10 @@ cb_note (const enum cb_warn_opt opt, const int suppress_listing, const char *fmt
 	}
 	va_start (ap, fmt);
 	if (opt != COB_WARNOPT_NONE) {
-		print_error (NULL, 0, _("note: "),
+		print_error (NULL, 0, CB_KIND_NOTE,
 			fmt, ap, warning_option_text (opt, pref));
 	} else {
-		print_error (NULL, 0, _("note: "),
+		print_error (NULL, 0, CB_KIND_NOTE,
 			fmt, ap, NULL);
 	}
 	va_end (ap);
@@ -776,13 +867,13 @@ cb_error_x_internal (cb_tree x, const char *fmt, va_list ap)
 	}
 
 	if (!ignore_error) {
-		print_error (x->source_file, x->source_line, _("error: "),
+		print_error (x->source_file, x->source_line, CB_KIND_ERROR,
 			fmt, ap, NULL);
 	} else if (pref == COBC_WARN_AS_ERROR) {
-		print_error (x->source_file, x->source_line, _("error: "),
+		print_error (x->source_file, x->source_line, CB_KIND_ERROR,
 			fmt, ap, warning_option_text (opt, pref));
 	} else {
-		print_error (x->source_file, x->source_line, _("warning: "),
+		print_error (x->source_file, x->source_line, CB_KIND_WARNING,
 			fmt, ap, warning_option_text (opt, pref));
 		ret = COBC_WARN_ENABLED;
 	}

cobc/flag.def

@@ -232,6 +232,12 @@ CB_FLAG (cb_listing_symbols, 1, "tsymbols",
 CB_FLAG (cb_listing_cmd, 1, "tcmd",
 	_("  -ftcmd                specify command line in listing"))
 
-CB_FLAG_ON (cb_diagnostic_show_option, 1, "diagnostics-show-option",
+CB_FLAG_ON (cb_diagnostics_show_option, 1, "diagnostics-show-option",
 	_("  -fno-diagnostics-show-option\tsuppress output of option that directly\n"
 	  "                        controls the diagnostic"))
+
+CB_FLAG_ON (cb_diagnostics_show_caret, 1, "diagnostics-show-caret",
+	_("  -fno-diagnostics-show-caret\tdo not display source context on warning/error diagnostic"))
+
+CB_FLAG_ON (cb_diagnostics_show_line_numbers, 1, "diagnostics-show-line-numbers",
+	_("  -fno-diagnostics-show-line-numbers\tsuppress display of line numbers in diagnostics"))

cobc/help.c

@@ -162,6 +162,7 @@ cobc_print_usage_warnings (void)
 #undef	CB_ONWARNDEF
 #undef	CB_NOWARNDEF
 #undef	CB_ERRWARNDEF
+	puts (_("  -fdiagnostics-plain-output\tmake diagnostic output as plain as possible"));
 	puts (_("  -Werror               treat all warnings as errors"));
 	puts (_("  -Wno-error            don't treat warnings as errors"));
 	puts (_("  -Werror=<warning>     treat specified <warning> as error"));

tests/atlocal.in

@@ -72,6 +72,7 @@ fi
 # FLAGS="-debug -Wall ${COBOL_FLAGS}"
 FLAGS="-debug -Wall ${COBOL_FLAGS} -fno-diagnostics-show-option"
 
+COBC="${COBC} -fdiagnostics-plain-output"
 COMPILE="${COBC} -x ${FLAGS}"
 COMPILE_ONLY="${COBC} -fsyntax-only ${FLAGS} -Wno-unsupported"
 COMPILE_MODULE="${COBC} -m ${FLAGS}"

tests/testsuite.src/listings.at

@@ -2143,8 +2143,8 @@ LINE    PG/LN  A...B............................................................
 GnuCOBOL V.R.P          prog.cob             DDD MMM dd HH:MM:SS YYYY  Page 0002
 
 command line:
-  cobc -q -std=default -Wall -fno-tmessages -fsyntax-only -t prog.lst
-+ -fno-tsymbols -ftcmd prog.cob
+  cobc -fdiagnostics-plain-output -q -std=default -Wall -fno-tmessages
++ -fsyntax-only -t prog.lst -fno-tsymbols -ftcmd prog.cob
 ])
 
 AT_CHECK([$UNIFY_LISTING prog.lst prog.lis], [0], [], [])