changeset 52551:a0587c1b633a

rhg: add a collision detection to complain about duplicated commands The previous commit made it easier (too easy) to define commands with identical names. It turns out `clap` does nothing to detect such collisions, which also leads to very confusing behavior. This change catches that error, and reports where the commands came from.
author Arseniy Alekseyev <aalekseyev@janestreet.com>
date Fri, 13 Dec 2024 16:50:21 +0000
parents 021c1b1671e5
children 66e34bc44280
files rust/rhg/src/main.rs
diffstat 1 files changed, 60 insertions(+), 21 deletions(-) [+]
line wrap: on
line diff
--- a/rust/rhg/src/main.rs	Fri Dec 13 16:19:29 2024 +0000
+++ b/rust/rhg/src/main.rs	Fri Dec 13 16:50:21 2024 +0000
@@ -9,7 +9,7 @@
 use hg::utils::SliceExt;
 use hg::{exit_codes, requirements};
 use std::borrow::Cow;
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::ffi::OsString;
 use std::os::unix::prelude::CommandExt;
 use std::path::PathBuf;
@@ -67,7 +67,8 @@
                 .global(true),
         )
         .version("0.0.1");
-    let app = add_subcommand_args(app);
+    let subcommands = subcommands();
+    let app = subcommands.add_args(app);
 
     let matches = app.try_get_matches_from(argv.iter())?;
 
@@ -98,7 +99,7 @@
             return Err(CommandError::unsupported(msg));
         }
     }
-    let run = subcommand_run_fn(subcommand_name)
+    let run = subcommands.run_fn(subcommand_name)
         .expect("unknown subcommand name from clap despite Command::subcommand_required");
 
     let invocation = CliInvocation {
@@ -542,21 +543,70 @@
     run: RunFn,
     args: clap::Command,
     name: String,
+    /// used for reporting collision
+    origin: String,
 }
 
 macro_rules! subcommand {
     ($command: ident) => {{
         let args = commands::$command::args();
         let name = args.get_name().to_string();
+        let origin = stringify!($command).to_string();
         SubCommand {
             args,
             run: commands::$command::run,
             name,
+            origin,
         }
     }};
 }
-fn subcommands() -> Vec<SubCommand> {
-    vec![
+
+struct Subcommands {
+    commands: Vec<clap::Command>,
+    run: HashMap<String, (String, RunFn)>,
+}
+
+/// `Subcommands` construction
+impl Subcommands {
+    pub fn new() -> Self {
+        Self {
+            commands: vec![],
+            run: HashMap::new(),
+        }
+    }
+
+    pub fn add(&mut self, subcommand: SubCommand) {
+        let name = subcommand.name;
+        if let Some((origin_old, _)) = self
+            .run
+            .insert(name.clone(), (subcommand.origin.clone(), subcommand.run))
+        {
+            panic!(
+                "command `{}` is defined in two places (`{}` and `{}`)",
+                name, origin_old, subcommand.origin
+            )
+        }
+        self.commands.push(subcommand.args)
+    }
+}
+
+/// `Subcommands` querying
+impl Subcommands {
+    pub fn add_args(&self, mut app: clap::Command) -> clap::Command {
+        for cmd in self.commands.iter() {
+            app = app.subcommand(cmd)
+        }
+        app
+    }
+
+    pub fn run_fn(&self, name: &str) -> Option<RunFn> {
+        let (_, run) = self.run.get(name)?;
+        Some(*run)
+    }
+}
+
+fn subcommands() -> Subcommands {
+    let subcommands = vec![
         subcommand!(cat),
         subcommand!(debugdata),
         subcommand!(debugrequirements),
@@ -566,23 +616,12 @@
         subcommand!(root),
         subcommand!(config),
         subcommand!(status),
-    ]
-}
-
-fn add_subcommand_args(mut app: clap::Command) -> clap::Command {
-    for s in subcommands() {
-        app = app.subcommand(s.args)
+    ];
+    let mut commands = Subcommands::new();
+    for cmd in subcommands {
+        commands.add(cmd)
     }
-    app
-}
-
-fn subcommand_run_fn(name: &str) -> Option<RunFn> {
-    for s in subcommands() {
-        if s.name == name {
-            return Some(s.run);
-        }
-    }
-    None
+    commands
 }
 
 pub struct CliInvocation<'a> {