From 5dd0629478c9abf56908c4500e9b6690efbb4616 Mon Sep 17 00:00:00 2001
From: Jun Wu <quark@fb.com>
Date: Mon, 29 Jul 2019 19:30:30 -0700
Subject: [PATCH] cliparser: add an error type about illegal alias

Summary:
When the alias cannot be split via shlex::split, raise an error directly
instead of silently ignoring it and fallback to Python error handling.

Reviewed By: kulshrax

Differential Revision: D16530519

fbshipit-source-id: d81534b198555b256f062dc4e6520fa40ace7700
---
 edenscm/mercurial/dispatch.py                    |  3 +++
 edenscm/mercurial/rust/bindings/src/cliparser.rs |  5 +++++
 lib/cliparser/src/alias.rs                       | 12 +++++-------
 lib/cliparser/src/parser.rs                      |  2 ++
 tests/test-alias.t                               |  2 +-
 5 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/edenscm/mercurial/dispatch.py b/edenscm/mercurial/dispatch.py
index 1ccf0b9279..bc3d25a7e2 100644
--- a/edenscm/mercurial/dispatch.py
+++ b/edenscm/mercurial/dispatch.py
@@ -1058,6 +1058,9 @@ def _parse(ui, args):
         except cliparser.CircularReference as e:
             alias = e.args[1]
             raise error.Abort(_("circular alias: %s") % alias)
+        except cliparser.IllformedAlias as e:
+            msg = e.args[0]
+            raise error.Abort(msg)
 
     else:
         replacement = []
diff --git a/edenscm/mercurial/rust/bindings/src/cliparser.rs b/edenscm/mercurial/rust/bindings/src/cliparser.rs
index 05a8a572c2..035c960c97 100644
--- a/edenscm/mercurial/rust/bindings/src/cliparser.rs
+++ b/edenscm/mercurial/rust/bindings/src/cliparser.rs
@@ -16,6 +16,7 @@ mod exceptions {
 
     py_exception!(cliparser, AmbiguousCommand);
     py_exception!(cliparser, CircularReference);
+    py_exception!(cliparser, IllformedAlias);
     py_exception!(cliparser, OptionNotRecognized);
     py_exception!(cliparser, OptionRequiresArgument);
     py_exception!(cliparser, OptionArgumentInvalid);
@@ -58,6 +59,7 @@ pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
         use exceptions::*;
         m.add(py, "AmbiguousCommand", AmbiguousCommand::type_object(py))?;
         m.add(py, "CircularReference", CircularReference::type_object(py))?;
+        m.add(py, "IllformedAlias", IllformedAlias::type_object(py))?;
         m.add(
             py,
             "OptionNotRecognized",
@@ -256,5 +258,8 @@ fn map_to_python_err(py: Python, err: ParseError) -> PyErr {
         ParseError::CircularReference { command_name } => {
             return PyErr::new::<exceptions::CircularReference, _>(py, (msg, command_name))
         }
+        ParseError::IllformedAlias { name, value } => {
+            return PyErr::new::<exceptions::IllformedAlias, _>(py, (msg, name, value));
+        }
     }
 }
diff --git a/lib/cliparser/src/alias.rs b/lib/cliparser/src/alias.rs
index 187b338571..cc0c59b2e1 100644
--- a/lib/cliparser/src/alias.rs
+++ b/lib/cliparser/src/alias.rs
@@ -43,13 +43,11 @@ pub fn expand_aliases(
     loop {
         match cfg.get(&arg) {
             Some(alias) => {
-                let parts: Vec<String> = match split(alias) {
-                    Some(v) => v,
-                    None => {
-                        expanded.push(arg);
-                        break;
-                    }
-                };
+                let parts: Vec<String> =
+                    split(alias).ok_or_else(|| ParseError::IllformedAlias {
+                        name: arg.clone(),
+                        value: alias.to_string(),
+                    })?;
 
                 if !visited.insert(arg.clone()) {
                     return Err(ParseError::CircularReference { command_name: arg });
diff --git a/lib/cliparser/src/parser.rs b/lib/cliparser/src/parser.rs
index f755259c9b..e977cdfa32 100644
--- a/lib/cliparser/src/parser.rs
+++ b/lib/cliparser/src/parser.rs
@@ -68,6 +68,8 @@ pub enum ParseError {
     },
     #[fail(display = "Alias {} resulted in a circular reference", command_name)]
     CircularReference { command_name: String },
+    #[fail(display = "alias definition {} = {:?} cannot be parsed", name, value)]
+    IllformedAlias { name: String, value: String },
 }
 
 #[derive(Debug, PartialEq, Eq, Hash, Clone)]
diff --git a/tests/test-alias.t b/tests/test-alias.t
index 0e5340bbc0..e1f6ffbb6d 100644
--- a/tests/test-alias.t
+++ b/tests/test-alias.t
@@ -231,7 +231,7 @@ no definition
 no closing quotation
 
   $ hg noclosing
-  abort: error in definition for alias 'noclosingquotation': No closing quotation
+  abort: alias definition noclosingquotation = "\'" cannot be parsed
   [255]
   $ hg help noclosing
   error in definition for alias 'noclosingquotation': No closing quotation