Make a smarter check to avoid 'useless-type-doc' false positive

7 files changed, 51 insertions, 21 deletions

diff --git a/ChangeLog b/ChangeLog
index 9afa5be21..2ee8bc0af 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -11,6 +11,12 @@ Release date: TBA
 
 * astroid has been upgraded to 2.6.0
 
+* Fix false positive ``useless-type-doc`` on ignored argument using ``pylint.extensions.docparams``
+  when a function was typed using pep484 but not inside the docstring.
+
+  Closes #4117
+  Closes #4593
+
 * ``setuptools_scm`` has been removed and replaced by ``tbump`` in order to not
   have hidden runtime dependencies to setuptools
 
diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst
index c8aebf86d..ec89eb51a 100644
--- a/doc/whatsnew/2.9.rst
+++ b/doc/whatsnew/2.9.rst
@@ -72,3 +72,6 @@ Other Changes
 * ``ignore-paths`` configuration directive has been added. Defined regex patterns are matched against file path.
 
 * Added handling of floating point values when parsing configuration from pyproject.toml
+
+* Fix false positive ``useless-type-doc`` on ignored argument using ``pylint.extensions.docparams`` when a function
+  was typed using pep484 but not inside the docstring.
diff --git a/pylint/extensions/_check_docs_utils.py b/pylint/extensions/_check_docs_utils.py
index 5f1e8ea0b..de2f427bf 100644
--- a/pylint/extensions/_check_docs_utils.py
+++ b/pylint/extensions/_check_docs_utils.py
@@ -25,7 +25,6 @@ import re
 from typing import List
 
 import astroid
-from astroid import AssignName
 
 from pylint.checkers import utils
 
@@ -207,9 +206,6 @@ class Docstring:
     def __repr__(self) -> str:
         return f"<{self.__class__.__name__}:'''{self.doc}'''>"
 
-    def arg_is_documented(self, arg_name: AssignName) -> bool:
-        return arg_name.name in self.doc
-
     def is_valid(self):
         return False
 
diff --git a/pylint/extensions/docparams.py b/pylint/extensions/docparams.py
index 032e993a6..42dfd2b87 100644
--- a/pylint/extensions/docparams.py
+++ b/pylint/extensions/docparams.py
@@ -531,7 +531,6 @@ class DocstringParameterChecker(BaseChecker):
             expected_argument_names.add(arguments_node.kwarg)
             not_needed_type_in_docstring.add(arguments_node.kwarg)
         params_with_doc, params_with_type = doc.match_param_docs()
-
         # Tolerate no parameter documentation at all.
         if not params_with_doc and not params_with_type and accept_no_param_doc:
             tolerate_missing_params = True
@@ -546,13 +545,20 @@ class DocstringParameterChecker(BaseChecker):
                 warning_node,
             )
 
+        # This is before the update of param_with_type because this must check only
+        # the type documented in a docstring, not the one using pep484
+        # See #4117 and #4593
+        self._compare_ignored_args(
+            params_with_type,
+            "useless-type-doc",
+            expected_but_ignored_argument_names,
+            warning_node,
+        )
         for index, arg_name in enumerate(arguments_node.args):
-            if arguments_node.annotations[index] and doc.arg_is_documented(arg_name):
+            if arguments_node.annotations[index]:
                 params_with_type.add(arg_name.name)
         for index, arg_name in enumerate(arguments_node.kwonlyargs):
-            if arguments_node.kwonlyargs_annotations[index] and doc.arg_is_documented(
-                arg_name
-            ):
+            if arguments_node.kwonlyargs_annotations[index]:
                 params_with_type.add(arg_name.name)
 
         if not tolerate_missing_params:
@@ -584,12 +590,6 @@ class DocstringParameterChecker(BaseChecker):
             expected_but_ignored_argument_names,
             warning_node,
         )
-        self._compare_ignored_args(
-            params_with_type,
-            "useless-type-doc",
-            expected_but_ignored_argument_names,
-            warning_node,
-        )
 
     def check_single_constructor_params(self, class_doc, init_doc, class_node):
         if class_doc.has_params() and init_doc.has_params():
diff --git a/tests/extensions/test_check_docs.py b/tests/extensions/test_check_docs.py
index a65bf9316..42b145fc8 100644
--- a/tests/extensions/test_check_docs.py
+++ b/tests/extensions/test_check_docs.py
@@ -2236,8 +2236,8 @@ class TestParamDocChecker(CheckerTestCase):
         """
         )
         with self.assertAddsMessages(
-            Message(msg_id="useless-param-doc", node=node, args=("_, _ignored",)),
             Message(msg_id="useless-type-doc", node=node, args=("_",)),
+            Message(msg_id="useless-param-doc", node=node, args=("_, _ignored",)),
         ):
             self.checker.visit_functiondef(node)
 
@@ -2260,8 +2260,8 @@ class TestParamDocChecker(CheckerTestCase):
         """
         )
         with self.assertAddsMessages(
-            Message(msg_id="useless-param-doc", node=node, args=("_, _ignored",)),
             Message(msg_id="useless-type-doc", node=node, args=("_",)),
+            Message(msg_id="useless-param-doc", node=node, args=("_, _ignored",)),
         ):
             self.checker.visit_functiondef(node)
 
@@ -2290,8 +2290,8 @@ class TestParamDocChecker(CheckerTestCase):
         """
         )
         with self.assertAddsMessages(
-            Message(msg_id="useless-param-doc", node=node, args=("_, _ignored",)),
             Message(msg_id="useless-type-doc", node=node, args=("_",)),
+            Message(msg_id="useless-param-doc", node=node, args=("_, _ignored",)),
         ):
             self.checker.visit_functiondef(node)
 
diff --git a/tests/functional/u/useless/useless_type_doc.py b/tests/functional/u/useless/useless_type_doc.py
index 88dedb479..6714755fd 100644
--- a/tests/functional/u/useless/useless_type_doc.py
+++ b/tests/functional/u/useless/useless_type_doc.py
@@ -1,5 +1,4 @@
 """demonstrate FP with useless-type-doc"""
-# line-too-long
 
 
 def function(public_param: int, _some_private_param: bool = False) -> None:
@@ -16,6 +15,20 @@ def function(public_param: int, _some_private_param: bool = False) -> None:
         ...
 
 
+def smart_function(public_param: int, _some_private_param: bool = False) -> None:
+    """We're speaking about _some_private_param without really documenting it.
+
+    Args:
+        public_param: an ordinary parameter
+    """
+    for _ in range(public_param):
+        ...
+    if _some_private_param:
+        ...
+    else:
+        ...
+
+
 # +1: [useless-type-doc,useless-param-doc]
 def function_useless_doc(public_param: int, _some_private_param: bool = False) -> None:
     """does things
@@ -33,7 +46,7 @@ def function_useless_doc(public_param: int, _some_private_param: bool = False) -
         ...
 
 
-def test(_new: str) -> str:  # We don't expect useless-type-doc here
+def test(_new: str) -> str:
     """foobar
 
     :return: comment
@@ -41,8 +54,16 @@ def test(_new: str) -> str:  # We don't expect useless-type-doc here
     return ""
 
 
+def smarter_test(_new: str) -> str:
+    """We're speaking about _new without really documenting it.
+
+    :return: comment
+    """
+    return ""
+
+
 # +1: [useless-type-doc,useless-param-doc]
-def test_two(_new: str) -> str:  # We don't expect useless-type-doc here
+def test_two(_new: str) -> str:
     """foobar
 
     :param str _new:
diff --git a/tests/functional/u/useless/useless_type_doc.txt b/tests/functional/u/useless/useless_type_doc.txt
new file mode 100644
index 000000000..20083ed79
--- /dev/null
+++ b/tests/functional/u/useless/useless_type_doc.txt
@@ -0,0 +1,4 @@
+useless-param-doc:33:0:function_useless_doc:"""_some_private_param"" useless ignored parameter documentation":HIGH
+useless-type-doc:33:0:function_useless_doc:"""_some_private_param"" useless ignored parameter type documentation":HIGH
+useless-param-doc:66:0:test_two:"""_new"" useless ignored parameter documentation":HIGH
+useless-type-doc:66:0:test_two:"""_new"" useless ignored parameter type documentation":HIGH