[PATCH v2, committed] Clarify error messages for misuse of m4_warn and --help for -W.

[Zack Weinberg]

m4_warn([category], [message]) passes its arguments directly to
Autom4te::Channels::msg.  If the category argument is not a recognized
“channel”, that function will crash and emit a *Perl* stack trace,
which makes it look like there’s something wrong with autoconf or
autom4te, rather than something wrong with the script.

Making matters worse, in autoconf 2.69, the manual said you could
use “all” and the empty string as the first argument to m4_warn.
As far as I can tell, neither of those was ever a valid message
channel, and the manual was corrected in 2.70, but we still got
a bug report from someone who tried it.

This patch makes us issue a nice helpful user error, instead of a
confusing internal error, when Autom4te::Channels::msg is called with
a bogus channel argument.  If the bogus channel is “all” or the empty
string, that error is demoted to a -Wobsolete warning.  If it is one
of the other special tokens recognized by -W, we customize the error
message, since someone might’ve thought that “none” being acceptable
to -W meant it was also acceptable to m4_warn.  The --help output for
autoconf, autoheader, autom4te, and autoreconf is also adjusted to
clarify that not all of the tokens recognized by -W count as
warning categories.

(Oddly enough, the lack of filtration at the autom4te level means
m4_warn([error], […]) actually does issue an error.  There’s no other
way to get exactly that effect — m4_errprintn(…)  and m4_fatal(…)
both do something a little different — so I I propose to leave that
alone for now and promote it to a proper, documented feature, probably
spelled m4_error(…), post-2.72.)

Channels.pm and ChannelDefs.pm are shared with Automake.  I believe
there is nothing you can write in a Makefile.am that will cause
Automake::Channels::msg to be called with an arbitrary user-supplied
channel argument, so these changes should have no visible effect on
that side of the fence.

Addresses <https://savannah.gnu.org/support/?110872>.

 * lib/Autom4te/Channels.pm (msg): If the channel argument is invalid,
   don’t crash; report the mistake and use the ‘syntax’ channel.
   (report_bad_channel): New function for reporting invalid channels.

 * lib/Autom4te/ChannelDefs.pm (usage): Clarify that the list of
   warning categories is exhaustive, and that “all”, “none”,
   “no-CATEGORY”, and “error” are not warning categories.

 * bin/autoconf.in, bin/autoheader.in, bin/autom4te.in,
   bin/autoreconf.in: Tweak --help text.

 * tests/m4sugar.at (m4@&t@_warn (bad categories)): New test.
---
 bin/autoconf.in             |  1 +
 bin/autoheader.in           |  1 +
 bin/autom4te.in             |  1 +
 bin/autoreconf.in           | 10 +++---
 lib/Autom4te/ChannelDefs.pm | 10 +++---
 lib/Autom4te/Channels.pm    | 47 ++++++++++++++++++++++++++-
 tests/m4sugar.at            | 63 +++++++++++++++++++++++++++++++++++++
 7 files changed, 122 insertions(+), 11 deletions(-)

diff --git a/bin/autoconf.in b/bin/autoconf.in
index 060a9a04..5f4a2e5c 100644
--- a/bin/autoconf.in
+++ b/bin/autoconf.in
@@ -64,6 +64,7 @@ Operation modes:
   -f, --force               consider all files obsolete
   -o, --output=FILE         save output in FILE (stdout is the default)
   -W, --warnings=CATEGORY   report the warnings falling in CATEGORY
+                            (comma-separated list accepted)
 
 " . Autom4te::ChannelDefs::usage . "
 
diff --git a/bin/autoheader.in b/bin/autoheader.in
index 4afa5f13..f5af0df2 100644
--- a/bin/autoheader.in
+++ b/bin/autoheader.in
@@ -74,6 +74,7 @@ or else 'configure.in'.
   -d, --debug              don\'t remove temporary files
   -f, --force              consider all files obsolete
   -W, --warnings=CATEGORY  report the warnings falling in CATEGORY
+                           (comma-separated list accepted)
 
 " . Autom4te::ChannelDefs::usage . "
 
diff --git a/bin/autom4te.in b/bin/autom4te.in
index 8957a6c9..2e4e7d52 100644
--- a/bin/autom4te.in
+++ b/bin/autom4te.in
@@ -160,6 +160,7 @@ Operation modes:
   -o, --output=FILE        save output in FILE (defaults to '-', stdout)
   -f, --force              don't rely on cached values
   -W, --warnings=CATEGORY  report the warnings falling in CATEGORY
+                           (comma-separated list accepted)
   -l, --language=LANG      specify the set of M4 macros to use
   -C, --cache=DIRECTORY    preserve results for future runs in DIRECTORY
       --no-cache           disable the cache
diff --git a/bin/autoreconf.in b/bin/autoreconf.in
index c6077efe..285d0f49 100644
--- a/bin/autoreconf.in
+++ b/bin/autoreconf.in
@@ -84,20 +84,18 @@ Operation modes:
       --no-recursive       don't rebuild sub-packages
   -s, --symlink            with -i, install symbolic links instead of copies
   -m, --make               when applicable, re-run ./configure && make
-  -W, --warnings=CATEGORY  report the warnings falling in CATEGORY [syntax]
+  -W, --warnings=CATEGORY  report the warnings falling in CATEGORY
+                           (comma-separated list accepted)
 
 " . Autom4te::ChannelDefs::usage . "
 
-The environment variable 'WARNINGS' is honored.  Some subtools might
-support other warning types, using 'all' is encouraged.
-
 Library directories:
   -B, --prepend-include=DIR  prepend directory DIR to search path
   -I, --include=DIR          append directory DIR to search path
 
 The environment variables AUTOCONF, ACLOCAL, AUTOHEADER, AUTOM4TE,
-AUTOMAKE, AUTOPOINT, GTKDOCIZE, INTLTOOLIZE, LIBTOOLIZE, M4, and MAKE
-are honored.
+AUTOMAKE, AUTOPOINT, GTKDOCIZE, INTLTOOLIZE, LIBTOOLIZE, M4, MAKE,
+and WARNINGS are honored.
 
 Report bugs to <bug-autoconf\@gnu.org>.
 GNU Autoconf home page: <https://www.gnu.org/software/autoconf/>.
diff --git a/lib/Autom4te/ChannelDefs.pm b/lib/Autom4te/ChannelDefs.pm
index 85ea6f82..9682e8db 100644
--- a/lib/Autom4te/ChannelDefs.pm
+++ b/lib/Autom4te/ChannelDefs.pm
@@ -197,7 +197,7 @@ Return the warning category descriptions.
 
 sub usage ()
 {
-  return "Warning categories include:
+  return "Warning categories are:
   cross                  cross compilation issues
   gnu                    GNU coding standards (default in gnu and gnits modes)
   obsolete               obsolete features or constructions (default)
@@ -207,10 +207,12 @@ sub usage ()
   extra-portability      extra portability issues related to obscure tools
   syntax                 dubious syntactic constructs (default)
   unsupported            unsupported or incomplete features (default)
-  all                    all the warnings
-  no-CATEGORY            turn off warnings in CATEGORY
+
+-W also understands:
+  all                    turn on all the warnings
   none                   turn off all the warnings
-  error                  treat warnings as errors";
+  no-CATEGORY            turn off warnings in CATEGORY
+  error                  treat all enabled warnings as errors";
 }
 
 =item C<prog_error ($MESSAGE, [%OPTIONS])>
diff --git a/lib/Autom4te/Channels.pm b/lib/Autom4te/Channels.pm
index a2ed0a0b..cd77be95 100644
--- a/lib/Autom4te/Channels.pm
+++ b/lib/Autom4te/Channels.pm
@@ -628,7 +628,13 @@ sub msg ($$;$%)
       $location = '';
     }
 
-  confess "unknown channel $channel" unless exists $channels{$channel};
+  if (!exists $channels{$channel})
+    {
+      # This can happen as a result of e.g. m4_warn([nonsense], [message])
+      # so it should not crash.
+      report_bad_channel($channel, $location);
+      $channel = 'syntax';
+    }
 
   my %opts = %{$channels{$channel}};
   _merge_options (%opts, %options);
@@ -662,6 +668,45 @@ sub msg ($$;$%)
     }
 }
 
+sub report_bad_channel ($$)
+{
+  my ($channel, $location) = @_;
+  my $message;
+  my $report_as = 'error';
+
+  # quotemeta is both too aggressive (e.g. it escapes '-') and
+  # too generous (it turns control characters into \ + themselves,
+  # not into symbolic escapes).
+  my $q_channel = $channel;
+  $q_channel =~ s/(?=[\"\$\'\@\`\\])/\\/g;
+  $q_channel =~ s/([^\x20-\x7e])/sprintf('\\x%02X', ord $1)/eg;
+  $q_channel = '"' . $q_channel . '"';
+
+  if ($channel eq '' || $channel eq 'all')
+    {
+      # Prior to version 2.70, the Autoconf manual said it was valid to use
+      # "all" and the empty string as the category argument to m4_warn, so
+      # don't treat those cases as errors.
+      $report_as = 'obsolete';
+      $message = "use of $q_channel as a diagnostic category is obsolete\n";
+      $message .= "(see autom4te --help for a list of valid categories)";
+    }
+  elsif ($channel eq 'none'
+         || ($channel =~ /^no-/ && exists $channels{substr($channel, 3)}))
+    {
+      # Also recognize "none" and "no-[category]", as someone might have
+      # thought anything acceptable to -W is also acceptable to m4_warn.
+      # Note: m4_warn([error], [...]) does actually issue an error.
+      $message = "-W accepts $q_channel, but it is not a diagnostic category";
+    }
+  else
+    {
+      $message = "unknown diagnostic category " . $q_channel;
+    }
+
+  msg $report_as, $location, $message;
+}
+
 
 =item C<setup_channel ($channel, %options)>
 
diff --git a/tests/m4sugar.at b/tests/m4sugar.at
index 2f1dbe37..c7e87228 100644
--- a/tests/m4sugar.at
+++ b/tests/m4sugar.at
@@ -244,6 +244,7 @@ AT_CLEANUP
 ## --------- ##
 
 AT_SETUP([m4@&t@_warn])
+AT_KEYWORDS([m4@&t@_warn])
 
 AT_DATA_M4SUGAR([script.4s],
 [[m4_init
@@ -297,6 +298,68 @@ script.4s:8: the top level
 
 AT_CLEANUP
 
+## ----------------------------------------------- ##
+## Bad m4_warn categories (Autoconf bug #110872).  ##
+## ----------------------------------------------- ##
+
+AT_SETUP([m4@&t@_warn (bad categories)])
+AT_KEYWORDS([m4@&t@_warn])
+
+AT_DATA_M4SUGAR([script.4s],
+[[m4_init
+m4_warn([],            [empty category])dnl
+m4_warn([bogus],       [invalid category])dnl
+m4_warn([!"\   %^&*], [line noise category])dnl " unconfuse editor
+m4_warn([all],         ['all' used as a category])dnl
+m4_warn([none],        ['none' used as a category])dnl
+m4_warn([no-syntax],   ['no-syntax' used as a category])dnl
+m4_warn([no-bogus],    ['no-bogus' used as a category])dnl
+m4_warn([error],       [oddly enough, this works])dnl
+]])
+
+AT_CHECK_M4SUGAR([-o- -Wnone], 1, [],
+[script.4s:3: error: unknown diagnostic category "bogus"
+script.4s:4: error: unknown diagnostic category "!\"\\\x09%^&*"
+script.4s:6: error: -W accepts "none", but it is not a diagnostic category
+script.4s:7: error: -W accepts "no-syntax", but it is not a diagnostic category
+script.4s:8: error: unknown diagnostic category "no-bogus"
+script.4s:9: error: oddly enough, this works
+])
+
+AT_CHECK_M4SUGAR([-o- -Wnone,obsolete], 1, [],
+[script.4s:2: warning: use of "" as a diagnostic category is obsolete
+script.4s:2: (see autom4te --help for a list of valid categories)
+script.4s:3: error: unknown diagnostic category "bogus"
+script.4s:4: error: unknown diagnostic category "!\"\\\x09%^&*"
+script.4s:5: warning: use of "all" as a diagnostic category is obsolete
+script.4s:5: (see autom4te --help for a list of valid categories)
+script.4s:6: error: -W accepts "none", but it is not a diagnostic category
+script.4s:7: error: -W accepts "no-syntax", but it is not a diagnostic category
+script.4s:8: error: unknown diagnostic category "no-bogus"
+script.4s:9: error: oddly enough, this works
+])
+
+AT_CHECK_M4SUGAR([-o- -Wnone,obsolete,syntax], 1, [],
+[script.4s:2: warning: use of "" as a diagnostic category is obsolete
+script.4s:2: (see autom4te --help for a list of valid categories)
+script.4s:2: warning: empty category
+script.4s:3: error: unknown diagnostic category "bogus"
+script.4s:3: warning: invalid category
+script.4s:4: error: unknown diagnostic category "!\"\\\x09%^&*"
+script.4s:4: warning: line noise category
+script.4s:5: warning: use of "all" as a diagnostic category is obsolete
+script.4s:5: (see autom4te --help for a list of valid categories)
+script.4s:5: warning: 'all' used as a category
+script.4s:6: error: -W accepts "none", but it is not a diagnostic category
+script.4s:6: warning: 'none' used as a category
+script.4s:7: error: -W accepts "no-syntax", but it is not a diagnostic category
+script.4s:7: warning: 'no-syntax' used as a category
+script.4s:8: error: unknown diagnostic category "no-bogus"
+script.4s:8: warning: 'no-bogus' used as a category
+script.4s:9: error: oddly enough, this works
+])
+
+AT_CLEANUP
 
 ## ----------------- ##
 ## m4_divert_stack.  ##
-- 
2.41.0