From 2d3e277b1e0c0a24c30c611558692f95d14a8470 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Tue, 28 Jun 2016 09:36:24 -0500 Subject: [PATCH] Handle optional parameters that were never supported Previously, pycodestyle never introspected the argument names for classes except to require that ``tree`` be an argument it could pass. For Flake8 3.0, we lifted that restriction, but old plugins seem to have cargo-culted their __init__ signature to be def __init__(self, tree, builtins=None): For some yet unknown reason. This was causing an AttributeError. By updating flake8.utils.parameters_for to return a dictionary that indicates whether the parameter is required or not, we can side-step this by simply ignoring the parameter if it has a default value and we cannot provide it. Closes #151 --- src/flake8/plugins/manager.py | 10 +++++++++- src/flake8/processor.py | 11 ++++++++--- src/flake8/utils.py | 23 +++++++++++++++-------- tests/unit/test_file_processor.py | 14 +++++++++----- tests/unit/test_utils.py | 11 ++++++++--- 5 files changed, 49 insertions(+), 20 deletions(-) diff --git a/src/flake8/plugins/manager.py b/src/flake8/plugins/manager.py index 697b35f..5d9e52b 100644 --- a/src/flake8/plugins/manager.py +++ b/src/flake8/plugins/manager.py @@ -38,6 +38,7 @@ class Plugin(object): self.entry_point = entry_point self._plugin = None self._parameters = None + self._parameter_names = None self._group = None self._plugin_name = None self._version = None @@ -77,6 +78,13 @@ class Plugin(object): self._parameters = utils.parameters_for(self) return self._parameters + @property + def parameter_names(self): + """List of argument names that need to be passed to the plugin.""" + if self._parameter_names is None: + self._parameter_names = list(self.parameters) + return self._parameter_names + @property def plugin(self): """The loaded (and cached) plugin associated with the entry-point. @@ -416,7 +424,7 @@ class Checkers(PluginTypeManager): Find all checker plugins that are expecting a specific argument. """ for plugin in self.plugins.values(): - if argument_name == plugin.parameters[0]: + if argument_name == plugin.parameter_names[0]: yield plugin def register_options(self, optmanager): diff --git a/src/flake8/processor.py b/src/flake8/processor.py index 0c33cc2..1824ed1 100644 --- a/src/flake8/processor.py +++ b/src/flake8/processor.py @@ -206,14 +206,19 @@ class FileProcessor(object): """Generate the keyword arguments for a list of parameters.""" if arguments is None: arguments = {} - for param in parameters: + for param, required in parameters.items(): if param in arguments: continue try: arguments[param] = getattr(self, param) except AttributeError as exc: - LOG.exception(exc) - raise + if required: + LOG.exception(exc) + raise + else: + LOG.warning('Plugin requested optional parameter "%s" ' + 'but this is not an available parameter.', + param) return arguments def check_physical_error(self, error_code, line): diff --git a/src/flake8/utils.py b/src/flake8/utils.py index 9f1189c..fbd15b9 100644 --- a/src/flake8/utils.py +++ b/src/flake8/utils.py @@ -249,7 +249,7 @@ def fnmatch(filename, patterns, default=True): def parameters_for(plugin): - # type: (flake8.plugins.manager.Plugin) -> List[str] + # type: (flake8.plugins.manager.Plugin) -> Dict[str, bool] """Return the parameters for the plugin. This will inspect the plugin and return either the function parameters @@ -261,9 +261,10 @@ def parameters_for(plugin): :type plugin: flake8.plugins.manager.Plugin :returns: - Parameters to the plugin. + A dictionary mapping the parameter name to whether or not it is + required (a.k.a., is positional only/does not have a default). :rtype: - list(str) + dict([(str, bool)]) """ func = plugin.plugin is_class = not inspect.isfunction(func) @@ -271,15 +272,21 @@ def parameters_for(plugin): func = plugin.plugin.__init__ if sys.version_info < (3, 3): - parameters = inspect.getargspec(func)[0] + argspec = inspect.getargspec(func) + start_of_optional_args = len(argspec[0]) - len(argspec[-1] or []) + parameter_names = argspec[0] + parameters = collections.OrderedDict([ + (name, position < start_of_optional_args) + for position, name in enumerate(parameter_names) + ]) else: - parameters = [ - parameter.name + parameters = collections.OrderedDict([ + (parameter.name, parameter.default is parameter.empty) for parameter in inspect.signature(func).parameters.values() if parameter.kind == parameter.POSITIONAL_OR_KEYWORD - ] + ]) if is_class: - parameters.remove('self') + parameters.pop('self', None) return parameters diff --git a/tests/unit/test_file_processor.py b/tests/unit/test_file_processor.py index db17c2e..dec1667 100644 --- a/tests/unit/test_file_processor.py +++ b/tests/unit/test_file_processor.py @@ -114,13 +114,17 @@ def test_check_physical_error(error_code, line, expected_indent_char): @pytest.mark.parametrize('params, args, expected_kwargs', [ - (['blank_before', 'blank_lines'], None, {'blank_before': 0, - 'blank_lines': 0}), - (['noqa', 'fake'], {'fake': 'foo'}, {'noqa': False, 'fake': 'foo'}), - (['blank_before', 'blank_lines', 'noqa'], + ({'blank_before': True, 'blank_lines': True}, + None, + {'blank_before': 0, 'blank_lines': 0}), + ({'noqa': True, 'fake': True}, + {'fake': 'foo'}, + {'noqa': False, 'fake': 'foo'}), + ({'blank_before': True, 'blank_lines': True, 'noqa': True}, {'blank_before': 10, 'blank_lines': 5, 'noqa': True}, {'blank_before': 10, 'blank_lines': 5, 'noqa': True}), - ([], {'fake': 'foo'}, {'fake': 'foo'}), + ({}, {'fake': 'foo'}, {'fake': 'foo'}), + ({'non-existent': False}, {'fake': 'foo'}, {'fake': 'foo'}), ]) def test_keyword_arguments_for(params, args, expected_kwargs): """Verify the keyword args are generated properly.""" diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 5d31e82..d1816bc 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -102,17 +102,22 @@ def test_parameters_for_class_plugin(): plugin = plugin_manager.Plugin('plugin-name', object()) plugin._plugin = FakeCheck - assert utils.parameters_for(plugin) == ['tree'] + assert utils.parameters_for(plugin) == {'tree': True} def test_parameters_for_function_plugin(): """Verify that we retrieve the parameters for a function plugin.""" - def fake_plugin(physical_line, self, tree): + def fake_plugin(physical_line, self, tree, optional=None): pass plugin = plugin_manager.Plugin('plugin-name', object()) plugin._plugin = fake_plugin - assert utils.parameters_for(plugin) == ['physical_line', 'self', 'tree'] + assert utils.parameters_for(plugin) == { + 'physical_line': True, + 'self': True, + 'tree': True, + 'optional': False, + } def read_diff_file(filename):