From 36218f05ea6bf9a0ea14713a63342926d3869d21 Mon Sep 17 00:00:00 2001
From: Valentin Gatien-Baron <valentin.gatienbaron@gmail.com>
Date: Sun, 22 Jul 2018 02:24:08 -0400
Subject: [PATCH 1/2] Arg module sometimes misbehaved instead of rejecting
 invalid -keyword=arg inputs

When -a is defined as Unit, accepted -a=1.
When -a is defined as Tuple, accepted -a=1 b c as -a b=1 c=1.
When -a is defined as Rest, looped infinitely on -a=1.
---
 Changes       | 4 ++++
 stdlib/arg.ml | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Changes b/Changes
index 1cd4fced3167..433f8df19266 100644
--- a/Changes
+++ b/Changes
@@ -50,6 +50,10 @@ Working version
   a deprecation warning since 4.02.
   (whitequark)
 
+- GPR#1923: Arg module sometimes misbehaved instead of rejecting invalid
+  -keyword=arg inputs
+  (Valentin Gatien-Baron, review by ..)
+
 ### Other libraries:
 
 - GPR#1061: Add ?follow parameter to Unix.link. This allows hardlinking
diff --git a/stdlib/arg.ml b/stdlib/arg.ml
index e9322f6aa813..64e63d77e2d3 100644
--- a/stdlib/arg.ml
+++ b/stdlib/arg.ml
@@ -188,7 +188,7 @@ let parse_and_expand_argv_dynamic_aux allow_expand current argv speclist anonfun
           | Some _ -> ()
         in
         let rec treat_action = function
-        | Unit f -> f ();
+        | Unit f -> no_arg (); f ();
         | Bool f ->
             let arg = get_arg () in
             begin match bool_of_string_opt arg with
@@ -243,8 +243,10 @@ let parse_and_expand_argv_dynamic_aux allow_expand current argv speclist anonfun
             end;
             consume_arg ();
         | Tuple specs ->
+            no_arg ();
             List.iter treat_action specs;
         | Rest f ->
+            no_arg ();
             while !current < (Array.length !argv) - 1 do
               f !argv.(!current + 1);
               consume_arg ();

From 57301e7e409f5fdbdcd6c0dd1f908e86b31b17fc Mon Sep 17 00:00:00 2001
From: Valentin Gatien-Baron <valentin.gatienbaron@gmail.com>
Date: Sun, 22 Jul 2018 11:26:17 -0400
Subject: [PATCH 2/2] tests

---
 Changes                                     |  2 +-
 testsuite/tests/lib-arg/testerror.ml        |  6 ++++
 testsuite/tests/lib-arg/testerror.reference | 34 ++++++++++++++++-----
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/Changes b/Changes
index 433f8df19266..a2cd9c9112dd 100644
--- a/Changes
+++ b/Changes
@@ -52,7 +52,7 @@ Working version
 
 - GPR#1923: Arg module sometimes misbehaved instead of rejecting invalid
   -keyword=arg inputs
-  (Valentin Gatien-Baron, review by ..)
+  (Valentin Gatien-Baron, review by Gabriel Scherer)
 
 ### Other libraries:
 
diff --git a/testsuite/tests/lib-arg/testerror.ml b/testsuite/tests/lib-arg/testerror.ml
index cfe4a2993610..94409452c5a6 100644
--- a/testsuite/tests/lib-arg/testerror.ml
+++ b/testsuite/tests/lib-arg/testerror.ml
@@ -38,6 +38,12 @@ let tests = [
    Arg.Unit (fun () -> raise @@ Arg.Bad("User-raised error bis")),
    "user raised error"]
 , ignore, [ "-error" ]
+
+(* bad keyword in various places*)
+; [ "-rest", Arg.Rest ignore, "help"], ignore, [ "-rest=1" ]
+; [ "-tuple", Arg.Tuple [Arg.Int print_int; Arg.Int print_int ], "help" ]
+  , ignore, [ "-tuple=1" ]
+; [ "-unit", Arg.Unit ignore, "" ], ignore, [ "-unit=1" ]
 ]
 
 let () =
diff --git a/testsuite/tests/lib-arg/testerror.reference b/testsuite/tests/lib-arg/testerror.reference
index 3608e11c209c..c061b32f153d 100644
--- a/testsuite/tests/lib-arg/testerror.reference
+++ b/testsuite/tests/lib-arg/testerror.reference
@@ -1,45 +1,65 @@
-(1/7) Bad:
+(1/10) Bad:
 testerror: option '-s' needs an argument.
 Arg module testing
   -s missing arg
   -help  Display this list of options
   --help  Display this list of options
 
-(2/7) Bad:
+(2/10) Bad:
 testerror: wrong argument 'true'; option '-set=true' expects no argument.
 Arg module testing
   -set no argument expected
   -help  Display this list of options
   --help  Display this list of options
 
-(3/7) Help:
+(3/10) Help:
 Arg module testing
   -help  Display this list of options
   --help  Display this list of options
 
-(4/7) Bad:
+(4/10) Bad:
 testerror: wrong argument 'not_an_int'; option '-int' expects an integer.
 Arg module testing
   -int wrong argument type
   -help  Display this list of options
   --help  Display this list of options
 
-(5/7) Bad:
+(5/10) Bad:
 testerror: unknown option '-an-unknown-option'.
 Arg module testing
   -help  Display this list of options
   --help  Display this list of options
 
-(6/7) Bad:
+(6/10) Bad:
 testerror: User-raised error.
 Arg module testing
   -help  Display this list of options
   --help  Display this list of options
 
-(7/7) Bad:
+(7/10) Bad:
 testerror: User-raised error bis.
 Arg module testing
   -error user raised error
   -help  Display this list of options
   --help  Display this list of options
 
+(8/10) Bad:
+testerror: wrong argument '1'; option '-rest=1' expects no argument.
+Arg module testing
+  -rest help
+  -help  Display this list of options
+  --help  Display this list of options
+
+(9/10) Bad:
+testerror: wrong argument '1'; option '-tuple=1' expects no argument.
+Arg module testing
+  -tuple help
+  -help  Display this list of options
+  --help  Display this list of options
+
+(10/10) Bad:
+testerror: wrong argument '1'; option '-unit=1' expects no argument.
+Arg module testing
+  -help  Display this list of options
+  --help  Display this list of options
+