paxdiablo
paxdiablo

Reputation: 881553

Pylint 2.12.1 different behaviour from 2.11.1

We in the process of upgrading from Pylint 2.11.1 to 2.12.1 (released recently) and we're seeing strange behaviour in checking code that passes with the older version. Specifically, we have the following method (sans code but otherwise exactly the code we have so that there can be no doubt there's a cut'n'paste error):

async def run_callback(callback: common_types.AnyCallback) -> None:
    """Run the callback, handles sync and async functions.

    This WILL block the event loop if a sync function is called this way.
    IF a sync callback needs to be called, it should be wrapped in an
    async function and then called with run in executor. This cannot be
    done at this level because run_in_executor is a separate thread.
    Most async stuff is not thread safe and vice versa, so this is the
    minimal abstraction which won't introduce race conditions, the
    developer needs to handle it by manually doing a run_in_executor.

    Example:
        def sync_thing():
            pass

        async def async_thing():
            pass

        from cmnlibpy import utils

        await util.run_callback(sync_thing)
        await util.run_callback(async_thing)

    Args:
        callback:
            sync or async function which will be called
    """

While this is perfectly acceptable to 2.11.1, the newer version barfs on the parameter despite the fact it's in the docstring:

************* Module blah.blah.blah
blah/blah/blah.py:20:0: W9015: "callback" missing in parameter documentation (missing-param-doc)

I've tried moving it within the docstring, renaming it, and various other things to no avail. If you're concerned about the type, it's defined in an imported item from common_types.py in the same package:

from typing import Union, Awaitable, Callable

SyncCallback = Callable[[], None]
AsyncCallback = Callable[[], Awaitable[None]]
AnyCallback = Union[SyncCallback, AsyncCallback]

Why is the new PyLint complaining about this? Has it decided to deprecate Google-style parameter specifications?


Further investigation: it appears that the problem doesn't manifest itself when parameter and description are on a single line, such as:

callback: description goes here.

A diff of the GoogleDocstring regular expressions for parameter lines in extensions/_check_docs_utils.py seems to indicate it should either be a colon with the description on the same line, or no colon with description on following line:

- \s*  (\w+)                                                          # identifier
- \s*  :
- \s*  (?:({GoogleDocstring.re_multiple_type})(?:,\s+optional)?)?     # optional type declaration
- \n                                                                  # description starts on a new line
- \s* (.*)                                                            # description

+ \s*  (\*{{0,2}}\w+)(\s?(:|\n))                                      # identifier with potential asterisks
+ \s*  (?:({GoogleDocstring.re_multiple_type})(?:,\s+optional)?\n)?   # optional type declaration
+ \s* (.*)                                                            # optional description

However, the multi-line version didn't seem to work even if I left the : off the end of the line.


At the request of PyLint developer, I have opened a bug report (see https://github.com/PyCQA/pylint/issues/5452 for detail).

Upvotes: 2

Views: 207

Answers (1)

paxdiablo
paxdiablo

Reputation: 881553

It turns out there was an issue with the handling of parameter specifications that crossed multiple lines.

The code uses a regex that always delivers the three components (name, type, and description) but the code had some strangeness that resulted in a situation where neither type nor description was set, meaning the parameter wasn't recorded.

The code responsible was in extensions/_check_docs_utils.py:

re_only_desc = re.search(":\n", entry)
if re_only_desc:
    param_name = match.group(1)
    param_desc = match.group(2)
    param_type = None
else:
    param_name = match.group(1)
    param_type = match.group(2)
    param_desc = match.group(3)

I'm not sure why the presence of:\n is used to decide that the type isn't available but that doesn't seem right. Its presence means that the description is on the following line (nothing to do with missing type) but the fact that you're using \s means line breaks are treated the same as any other white space in the section so it's irrelevant.

Because the regex used for this captures all three groups whether the type is there or not, if it's not there, the second group (the parameter type) is set to None and the third group is still the description (it doesn't move to group 2). So the first part of the if sets both type and description to None, stopping the variable from being added.

If the code is instead changed into (no if needed):

param_name = match.group(1)
param_type = match.group(2)
param_desc = match.group(3)

then the first test case below still works, and the second one starts working:

Args:
    first_param: description
    second_param:
        description

Full details at the bug report and pull request, the maintainers were a pleasure to work with to get the fix done and it will hopefully be in 2.12.2, whenever that appears.

As a workaround, you can just ensure that the entire parameter specification exists on one line, such as with:

""" Doc string goes here.

    Args:
        param: The parameter.
"""

Upvotes: 2

Related Questions