Parameter Documentation checker only works if at least one parameter is documented properly

[luigibertaco]

Steps to reproduce

1. Enable pylint.extensions.docparams
2. Any of the following functions will be rated 10/10

def foobar1(arg1, arg2):
    """function foobar ...
    """
    print(arg1, arg2)

def foobar2(arg1, arg2):
    """function foobar ...

    Parameters
    ----------
    """
    print(arg1, arg2)

def foobar3(arg1, arg2):
    """function foobar ...

    Parameters
    ----------
    arg1: int
    """
    print(arg1, arg2)

3. Run pylint

Current behavior

The checker doesn't raise any warnings related to the parameters on the documentation

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Expected behavior

Something similar to this:

def foobar4(arg1, arg2):
    """function foobar ...

    Parameters
    ----------
    arg1: int
        description
    """
    print(arg1, arg2)

That will return the warnings for any missing parameter:

************* Module pylint_config
pylint_config.py:1:0: W9015: "arg2" missing in parameter documentation (missing-param-doc)
pylint_config.py:1:0: W9016: "arg2" missing in parameter type documentation (missing-type-doc)

------------------------------------------------------------------
Your code has been rated at 0.00/10 (previous run: 0.00/10, +0.00)

pylint --version output

pylint 2.6.0
astroid 2.4.2
Python 3.6.4 (default, Jul 22 2019, 12:18:30)
[GCC 4.2.1 Compatible Apple LLVM 10.0.1 (clang-1001.0.46.4)]

[hippo91]

@luigibertaco thanks for the report.
I agree with you concerning the foobar3 function and even for foobar2 even the situation is a bit less clear. However for foobar1, to my mind, there is no problem with it.
Some people may want to write docstring without parameter doc and i think pylint should not complain about it.
Do you agree with me?

[luigibertaco]

Sure, I think we are on the same page on foobar3, about the other two, I agree that some people will not use Parameters on their docstrings.

However, if that is the case, would they activate the pylint.extensions.docparams plugin? If there are extra checks that will make sense for someone to enable it and leave Parameters out, I have no issue with that (but for my use case I'd like to be able to enforce it)

[hippo91]

@luigibertaco thanks for your explanation.

[ksaketou]

Hello, I would like to work on that :) Are these the tests for those two messages?

[Pierre-Sassoulas]

Hello, glad to hear that, I'll assign the issue to you ! :) The files you linked are the unit test but you can also take example on this to creates new functional tests if you prefer that.

[ksaketou]

Should I create separate functional tests for the missing-param-doc and missing-type-doc or they can be in the same file?

[Pierre-Sassoulas]

Same file is ok unless the file become 100+ lines big :)

[ksaketou]

Hello again, quick question. Will the param doc plugin raise any warnings if there is no parameter documentation?
From this reply I assume that the extension will not accept empty param documentation by default??

[Pierre-Sassoulas]

Hello, yes :

def foobar1(arg1, arg2):
    """function foobar ...
    """
    print(arg1, arg2)

Should raise at least a warning. As @luigibertaco pointed out, if someone is activating this extension it means they want pylint to check that parameters are defined in the documentation in all cases. I see two possibilities:

• raise two missing-param-doc one for each parameters
• raise a single warning missing-any-doc "Missing any documentation in 'foobar1'" (not existing right now) for the whole function

I think the second way to do it will prevent us from having to add an option later for those who want to not have incomplete documentation but want to keep a simple docstring possible if not adding the doc make sense (they would just have to disable this new warning). What do you think ?

[ksaketou]

Okay yes, missing-any-doc makes sense. These will be all in one PR, right?

[Pierre-Sassoulas]

Yes a single PR would work :)

[ksaketou]

Also, when I'm running all the functional tests many of them are failing. Any idea why this happens? I have astroid 2.7.3

[Pierre-Sassoulas]

I think you don't have the latest pylint which should have astroid 2.8.0, you should fetch the main repository and rebase your branch on it.

git remote add upstream git@github.com:PyCQA/pylint.git
git fetch upstream
git rebase upstream/main

Then possibly upgrade your dependencies with pip so they're up to date.

[ksaketou]

Hello, I have another question. For the missing-any-doc message, will we check if the function has parameters, yields, returns and raises doc or just the arguments/parameters doc?

[Pierre-Sassoulas]

I think it should check for yields, returns and raises as it's "some doc" Except if it makes the implementation a lot harder then we could rename it to missing-any-param-doc ?

[ksaketou]

I think this is more complex since missing-any-doc should replace all the messages identified for yields, params, return and raises (if they have been identified) after it makes sure the doc doesn't exist (these checks also happen at different functions each). Missing-any-param-doc would be simpler, what do you think?

Also, the default value for the options accept-no-param-doc, accept-no-raise-doc etc of the checker is True. I think it should be False? If we change that we should also change the default values at the documentation. (https://pylint.pycqa.org/en/latest/technical_reference/extensions.html#parameter-documentation-checker-options )

[Pierre-Sassoulas]

missing-any-doc should replace all the messages identified for yields, params, return and raises

Ho I did not think of that, yes let's change it to missing-any-param-doc and try to keep it simple.

Regarding the default value, I guess it's because often you don't want to be super specific about the params (if they're typed and have a great name docstrings params are a little too much imo) but still want to have a one line description of the function. So the default value is forgiving, and if someone really want to document everything they can change the option. I think we should keep it that way.

[ksaketou]

Great, so the default values don't change and we change the message to missing-any-param-doc.

[ksaketou]

I am trying to commit but pylint gives me this error. Everything runs correctly and the MessageTest class is there, why is that?

[DanielNoord]

I think you might need to run pip install -e . in the pylint directory, although I'm not 100% sure.

[Pierre-Sassoulas]

An outdated pylint installed in your virtual environment could be it like @DanielNoord said. If you did a pip install . it installed an old version of pylint that is still used. pip install -e by contrast will take the current code.

[ksaketou]

I am getting this error with pip install -e . : UnicodeEncodeError: 'charmap' codec can't encode characters in position 23-33: character maps to <undefined> with pylint 2.11.0. From what I read, it has to do with windows encoding?

[DanielNoord]

Can you show the full stack trace? From this error message I can't really determine where this is failing.

[ksaketou]

ERROR: Command errored out with exit status 1:
    command: 'c:\users\kwns\appdata\local\programs\python\python38\python.exe' -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'C:\\Users\\kwns\\OneDrive\\Υπολογιστής\\OSS\\pylint\\setup.py'"'"'; __file__='"'"'C:\\Users\\kwns\\OneDrive\\Υπολογιστής\\OSS\\pylint\\setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' develop --no-deps
        cwd: C:\Users\kwns\OneDrive\Υπολογιστής\OSS\pylint\
   Complete output (44 lines):
   running develop
   running egg_info
   writing pylint.egg-info\PKG-INFO
   writing dependency_links to pylint.egg-info\dependency_links.txt
   writing entry points to pylint.egg-info\entry_points.txt
   writing requirements to pylint.egg-info\requires.txt
   writing top-level names to pylint.egg-info\top_level.txt
   reading manifest file 'pylint.egg-info\SOURCES.txt'
   reading manifest template 'MANIFEST.in'
   warning: no previously-included files matching '*.rst' found under directory 'pylint'
   no previously-included directories found matching '.github'
   no previously-included directories found matching 'doc'
   no previously-included directories found matching 'elisp'
   no previously-included directories found matching 'examples'
   no previously-included directories found matching 'tests'
   no previously-included directories found matching 'script'
   warning: no previously-included files found matching '.*'
   warning: no previously-included files found matching 'ChangeLog'
   warning: no previously-included files found matching 'Dockerfile'
   warning: no previously-included files found matching 'pylintrc'
   warning: no previously-included files found matching 'requirements_*.txt'
   warning: no previously-included files found matching 'tox.ini'
   writing manifest file 'pylint.egg-info\SOURCES.txt'
   running build_ext
   Creating c:\users\kwns\appdata\local\programs\python\python38\lib\site-packages\pylint.egg-link (link to .)
   Traceback (most recent call last):
     File "<string>", line 1, in <module>
     File "C:\Users\kwns\OneDrive\\u03a5\u03c0\u03bf\u03bb\u03bf\u03b3\u03b9\u03c3\u03c4\u03ae\u03c2\OSS\pylint\setup.py", line 3, in <module>
       setup()
     File "c:\users\kwns\appdata\local\programs\python\python38\lib\site-packages\setuptools\__init__.py", line 145, in setup
       return distutils.core.setup(**attrs)
     File "c:\users\kwns\appdata\local\programs\python\python38\lib\distutils\core.py", line 148, in setup
       dist.run_commands()
     File "c:\users\kwns\appdata\local\programs\python\python38\lib\distutils\dist.py", line 966, in run_commands
       self.run_command(cmd)
     File "c:\users\kwns\appdata\local\programs\python\python38\lib\distutils\dist.py", line 985, in run_command
       cmd_obj.run()
     File "c:\users\kwns\appdata\local\programs\python\python38\lib\site-packages\setuptools\command\develop.py", line 38, in run
       self.install_for_development()
     File "c:\users\kwns\appdata\local\programs\python\python38\lib\site-packages\setuptools\command\develop.py", line 153, in install_for_development
       f.write(self.egg_path + "\n" + self.setup_path)
     File "c:\users\kwns\appdata\local\programs\python\python38\lib\encodings\cp1252.py", line 19, in encode
       return codecs.charmap_encode(input,self.errors,encoding_table)[0]
   UnicodeEncodeError: 'charmap' codec can't encode characters in position 23-33: character maps to <undefined>

[DanielNoord]

I feel this might be due to your cwd containing Υπολογιστής. Looking at the trace this might be an issue with setuptools and not necessarily with pylint. Removing the non-standard/Greek letters from the cwd will probably fix this!

[ksaketou]

This fixed it, thank you!

[ksaketou]

One last question 😅 I added my changes in ChangeLog and version.rst but the fix-documentation hook is failing (exit code 9009). I have checked almost everything but I cannot find what's wrong...

[DanielNoord]

Can you post your changes?

[ksaketou]

version.rts :

changelog:

[DanielNoord]

Don't immediately see what's wrong here. Can you post the full error? And are you sure you have installed all dependencies?

[ksaketou]

I ran pre-commit install in the pylint directory to enable the hooks again. Also everything is updated with pip install -e .

[DanielNoord]

I looked into it but code 9009 means that some file can't be found. This means that something is wrong with one of the file paths.
You could do git commit -m "WIP" --no-verify" and push the commit to a branch on your own fork. Don't make a PR to this repository but make a PR on your own fork. If you tag me there I can take a look at the files you are trying to change and if I see anything that might be wrong!