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

[Zack Weinberg]

In autoconf 2.69, the manual said that you could use “all” and the
empty string as the first argument to m4_warn.  As far as I can tell
this was never actually true, and the manual was corrected in 2.70,
but we still got a bug report from someone who tried it and got a
confusing internal error from the guts of autom4te.

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, m4_warn([error], […]) actually issues an error, because
the argument to m4_warn is passed straight through as the channel
argument to Autom4te::Channels::msg.  Since neither m4_errprintn(…)
nor m4_fatal(…)  does quite the same thing as m4_warn([error], […]),
I propose to leave that alone for now and promote it to a proper,
documented feature (probably spelled m4_error(…)) post-2.72.)

Since this touches code shared between Autoconf and Automake, I’m not
checking it in yet and I’m requesting comments from both sides of the
fence.  Also, there’s a perl 2.14ism in one place (s///a) which I need
to figure out how to make 2.6-compatible before it can land.

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    | 48 +++++++++++++++++++++++++++-
 tests/m4sugar.at            | 63 +++++++++++++++++++++++++++++++++++++
 7 files changed, 123 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..634a61a6 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,46 @@ 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])/"\\x".sprintf("%02x", ord($1))/aeg;
+  $q_channel =~ s/(?=[\"\$\'\@\`])/\\/g;
+  $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..9a6be0d2 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 "!\"%^&*"
+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 "!\"%^&*"
+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 "!\"%^&*"
+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