From 943dc564b2dfb522b71be7b9e4310165f696d0e1 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <git@the-compiler.org>
Date: Tue, 23 Aug 2016 22:01:21 +0200
Subject: [PATCH] Fix choices validation with unannotated args

Something like:

    @cmdutils.argument('foo', choices=['one', 'two'])
    def func(foo):
        # ...

didn't actually validate the foo argument, since the inferred type of
the argument is None, and that skipped all conversion (and thus
validation).

Fixes #1871
See #1885

This is a reworked version of 12061b8bb12e9b385d80e83b4ba7411639c666a6
which lets special parameters (count/win_id/flags) through correctly.
---
 qutebrowser/commands/command.py      | 13 +++++++++----
 tests/unit/commands/test_cmdutils.py | 28 ++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py
index 59b690cbd..e871a1970 100644
--- a/qutebrowser/commands/command.py
+++ b/qutebrowser/commands/command.py
@@ -340,12 +340,17 @@ class Command:
         Args:
             param: The inspect.Parameter to look at.
         """
+        arginfo = self.get_arg_info(param)
         if param.annotation is not inspect.Parameter.empty:
             return param.annotation
-        elif param.default is None or param.default is inspect.Parameter.empty:
+        elif param.default not in [None, inspect.Parameter.empty]:
+            return type(param.default)
+        elif arginfo.count or arginfo.win_id or param.kind in [
+                inspect.Parameter.VAR_POSITIONAL,
+                inspect.Parameter.VAR_KEYWORD]:
             return None
         else:
-            return type(param.default)
+            return str
 
     def _get_self_arg(self, win_id, param, args):
         """Get the self argument for a function call.
@@ -425,10 +430,10 @@ class Command:
         elif typ is str:
             choices = self.get_arg_info(param).choices
             value = argparser.type_conv(param, typ, value, str_choices=choices)
-        elif typ is None:
-            pass
         elif typ is bool:  # no type conversion for flags
             assert isinstance(value, bool)
+        elif typ is None:
+            pass
         else:
             value = argparser.type_conv(param, typ, value)
 
diff --git a/tests/unit/commands/test_cmdutils.py b/tests/unit/commands/test_cmdutils.py
index 1bef17d86..a90da79fc 100644
--- a/tests/unit/commands/test_cmdutils.py
+++ b/tests/unit/commands/test_cmdutils.py
@@ -295,6 +295,34 @@ class TestRegister:
         else:
             assert cmd._get_call_args(win_id=0) == ([expected], {})
 
+    def test_choices_no_annotation(self):
+        # https://github.com/The-Compiler/qutebrowser/issues/1871
+        @cmdutils.register()
+        @cmdutils.argument('arg', choices=['foo', 'bar'])
+        def fun(arg):
+            """Blah."""
+            pass
+
+        cmd = cmdutils.cmd_dict['fun']
+        cmd.namespace = cmd.parser.parse_args(['fish'])
+
+        with pytest.raises(cmdexc.ArgumentTypeError):
+            cmd._get_call_args(win_id=0)
+
+    def test_choices_no_annotation_kwonly(self):
+        # https://github.com/The-Compiler/qutebrowser/issues/1871
+        @cmdutils.register()
+        @cmdutils.argument('arg', choices=['foo', 'bar'])
+        def fun(*, arg):
+            """Blah."""
+            pass
+
+        cmd = cmdutils.cmd_dict['fun']
+        cmd.namespace = cmd.parser.parse_args(['--arg=fish'])
+
+        with pytest.raises(cmdexc.ArgumentTypeError):
+            cmd._get_call_args(win_id=0)
+
     def test_pos_arg_info(self):
         @cmdutils.register()
         @cmdutils.argument('foo', choices=('a', 'b'))