From 819129b0d7040b493e9088c57763b5c0d423ce66 Mon Sep 17 00:00:00 2001
From: Ed Santiago <santiago@redhat.com>
Date: Mon, 7 Aug 2023 05:46:13 -0600
Subject: [PATCH] man-page xref: check for duplicate entries

Check for duplicate subcommands, flags, and format specifiers.
I assumed this would never be necessary, that code review would
catch dups, but it happened (#19462). Prevent future ones.

Also, make it a fatal error for a --format to be undocumented,
except for 'podman inspect'. So many exceptions ... :(

Signed-off-by: Ed Santiago <santiago@redhat.com>
---
 hack/xref-helpmsgs-manpages | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/hack/xref-helpmsgs-manpages b/hack/xref-helpmsgs-manpages
index 2335f5dc0a..dbf6d9b981 100755
--- a/hack/xref-helpmsgs-manpages
+++ b/hack/xref-helpmsgs-manpages
@@ -95,6 +95,13 @@ my %Format_Option_Is_Special = map { $_ => 1 } (
     'inspect',                                  # ambiguous (container/image)
  );
 
+# Hardcoded list of existing duplicate-except-for-case format codes,
+# with their associated subcommands. Let's not add any more.
+my %Format_Option_Dup_Allowed = (
+    'podman-images' => { '.id'     => 1 },
+    'podman-stats'  => { '.avgcpu' => 1, '.pids' => 1 },
+);
+
 # Do not cross-reference these.
 my %Skip_Subcommand = map { $_ => 1 } (
     "help",                     # has no man page
@@ -203,8 +210,7 @@ sub xref_by_help {
                 if ($k eq '--format' && ! ref($man->{$k})) {
                     warn "$ME: 'podman @subcommand': --format options are available through autocomplete, but are not documented in $man->{_path}\n";
 
-                    # FIXME: don't make this an error... yet.
-                    # ++$Errs;
+                    ++$Errs   unless "@subcommand" eq "inspect";
                     next OPTION;
                 }
 
@@ -491,6 +497,10 @@ sub podman_man {
                     warn "$ME: $subpath:$.: '$previous_subcmd' and '$subcmd' are out of order\n";
                     ++$Errs;
                 }
+                if ($previous_subcmd eq $subcmd) {
+                    warn "$ME: $subpath:$.: duplicate subcommand '$subcmd'\n";
+                    ++$Errs;
+                }
                 $previous_subcmd = $subcmd;
                 $man{$subcmd} = podman_man($link_name);
 
@@ -529,6 +539,10 @@ sub podman_man {
                         warn "$ME: $subpath:$.: $flag should precede $previous_flag\n";
                         ++$Errs;
                     }
+                    if ($flag eq $previous_flag) {
+                        warn "$ME: $subpath:$.: flag '$flag' is a dup\n";
+                        ++$Errs;
+                    }
                     $previous_flag = $flag if $is_first;
                     push @most_recent_flags, $flag;
 
@@ -607,10 +621,20 @@ sub podman_man {
                     # too many subformats to document individually.
                     $man{$previous_flag}{$format} = $etc || 1;
 
+                    # Sort order check, case-insensitive
                     if (lc($format) lt lc($previous_format)) {
                         warn "$ME: $subpath:$.: format specifier '$format' should precede '$previous_format'\n";
                         ++$Errs;
                     }
+
+                    # Dup check, would've caught #19462.
+                    if (lc($format) eq lc($previous_format)) {
+                        # Sigh. Allow preexisting exceptions, but no new ones.
+                        unless ($Format_Option_Dup_Allowed{$command}{lc $format}) {
+                            warn "$ME: $subpath:$.: format specifier '$format' is a dup\n";
+                            ++$Errs;
+                        }
+                    }
                     $previous_format = $format;
                 }
             }