From 0ef5d338bdf90821daac7a6ebd6339bda7b6ad7e Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 15 Sep 2016 15:44:33 +0200 Subject: [PATCH 1/4] make sure keyword-only arguments have a default Fixes #1872. This prevents inspect.Parameter.empty from slipping through to the command. --- qutebrowser/commands/command.py | 4 ++++ tests/unit/commands/test_cmdutils.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index ea1d0d46b..ce9e26eb8 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -246,6 +246,10 @@ class Command: continue if self._inspect_special_param(param): continue + if (param.kind == inspect.Parameter.KEYWORD_ONLY and + param.default is inspect.Parameter.empty): + raise TypeError("{}: handler has keyword only argument " + "without default!".format(self.name)) typ = self._get_type(param) is_bool = typ is bool kwargs = self._param_to_argparse_kwargs(param, is_bool) diff --git a/tests/unit/commands/test_cmdutils.py b/tests/unit/commands/test_cmdutils.py index 917b94e3e..1aacf3c33 100644 --- a/tests/unit/commands/test_cmdutils.py +++ b/tests/unit/commands/test_cmdutils.py @@ -349,6 +349,24 @@ class TestRegister: with pytest.raises(IndexError): cmd.get_pos_arg_info(2) + def test_keyword_only_without_default(self): + # https://github.com/The-Compiler/qutebrowser/issues/1872 + def fun(*, target): + """Blah.""" + pass + + with pytest.raises(TypeError): + fun = cmdutils.register()(fun) + + def test_typed_keyword_only_without_default(self): + # https://github.com/The-Compiler/qutebrowser/issues/1872 + def fun(*, target: int): + """Blah.""" + pass + + with pytest.raises(TypeError): + fun = cmdutils.register()(fun) + class TestArgument: From b6d9d3f955b8b50c22bec765c6a3800508de401d Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 15 Sep 2016 15:46:26 +0200 Subject: [PATCH 2/4] fix tests for new "default required" policy This test had a keyword only parameter without a default, which is now disallowed. This caused the test to fail. --- tests/unit/commands/test_cmdutils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/commands/test_cmdutils.py b/tests/unit/commands/test_cmdutils.py index 1aacf3c33..6f533798f 100644 --- a/tests/unit/commands/test_cmdutils.py +++ b/tests/unit/commands/test_cmdutils.py @@ -324,7 +324,7 @@ class TestRegister: # https://github.com/The-Compiler/qutebrowser/issues/1871 @cmdutils.register() @cmdutils.argument('arg', choices=['foo', 'bar']) - def fun(*, arg): + def fun(*, arg='foo'): """Blah.""" pass From 794eb84805394a5f2540aa1de6279b8286ff976e Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 15 Sep 2016 16:38:18 +0200 Subject: [PATCH 3/4] add parameter name in error message --- qutebrowser/commands/command.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index ce9e26eb8..ff4b2768a 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -247,9 +247,10 @@ class Command: if self._inspect_special_param(param): continue if (param.kind == inspect.Parameter.KEYWORD_ONLY and - param.default is inspect.Parameter.empty): + param.default is inspect.Parameter.empty): raise TypeError("{}: handler has keyword only argument " - "without default!".format(self.name)) + "{!r} without default!".format(self.name, + param.name)) typ = self._get_type(param) is_bool = typ is bool kwargs = self._param_to_argparse_kwargs(param, is_bool) From eabfdb3c165b133b666f222b1f2f8147f4bf650c Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Thu, 15 Sep 2016 16:42:02 +0200 Subject: [PATCH 4/4] tests: make sure the type error is the one we want --- tests/unit/commands/test_cmdutils.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/unit/commands/test_cmdutils.py b/tests/unit/commands/test_cmdutils.py index 6f533798f..f672a6768 100644 --- a/tests/unit/commands/test_cmdutils.py +++ b/tests/unit/commands/test_cmdutils.py @@ -355,18 +355,27 @@ class TestRegister: """Blah.""" pass - with pytest.raises(TypeError): + with pytest.raises(TypeError) as excinfo: fun = cmdutils.register()(fun) + expected = ("fun: handler has keyword only argument 'target' without " + "default!") + assert str(excinfo.value) == expected + + def test_typed_keyword_only_without_default(self): # https://github.com/The-Compiler/qutebrowser/issues/1872 def fun(*, target: int): """Blah.""" pass - with pytest.raises(TypeError): + with pytest.raises(TypeError) as excinfo: fun = cmdutils.register()(fun) + expected = ("fun: handler has keyword only argument 'target' without " + "default!") + assert str(excinfo.value) == expected + class TestArgument: