Fix overzealous arguments-differ for variadics

No message is emitted if the overriding function provides positional or
keyword variadics in its signature that can feasibly accept and pass on
all parameters given by the overridden function.

Closes pylint-dev#1482
Closes pylint-dev#1553

CONTRIBUTORS.txt

@@ -371,3 +371,5 @@ contributors:
 * Anthony Tan: contributor
 
 * Benny Müller: contributor
+
+* Matthew Beckers (mattlbeck): contributor

ChangeLog

@@ -200,6 +200,15 @@ Release date: TBA
 * Fixed ``broad_try_clause`` extension to check try/finally statements and to
   check for nested statements (e.g., inside of an ``if`` statement).
 
+* Fix overzealous `arguments-differ` when overridden function uses variadics
+
+  No message is emitted if the overriding function provides positional or
+  keyword variadics in its signature that can feasibly accept and pass on
+  all parameters given by the overridden function.
+
+  Close #1482
+  Close #1553
+
 What's New in Pylint 2.4.4?
 ===========================
 Release date: 2019-11-13

pylint/checkers/classes.py

@@ -281,11 +281,30 @@ def _different_parameters(original, overridden, dummy_parameter_regex):
     original_parameters = _positional_parameters(original)
     overridden_parameters = _positional_parameters(overridden)
 
+    # Copy kwonlyargs list so that we don't affect later function linting
+    original_kwonlyargs = [v for v in original.args.kwonlyargs]
+
+    # Allow positional/keyword variadic in overridden to match against any
+    # positional/keyword argument in original.
+    # Keep any arguments that are found seperately in overridden to satisfy
+    # later tests
+    if overridden.args.vararg:
+        overidden_names = [v.name for v in overridden_parameters]
+        original_parameters = [
+            v for v in original_parameters if v.name in overidden_names
+        ]
+
+    if overridden.args.kwarg:
+        overidden_names = [v.name for v in overridden.args.kwonlyargs]
+        original_kwonlyargs = [
+            v for v in original.args.kwonlyargs if v.name in overidden_names
+        ]
+
     different_positional = _has_different_parameters(
         original_parameters, overridden_parameters, dummy_parameter_regex
     )
     different_kwonly = _has_different_parameters(
-        original.args.kwonlyargs, overridden.args.kwonlyargs, dummy_parameter_regex
+        original_kwonlyargs, overridden.args.kwonlyargs, dummy_parameter_regex
     )
     if original.name in PYMETHODS:
         # Ignore the difference for special methods. If the parameter
@@ -295,21 +314,12 @@ def _different_parameters(original, overridden, dummy_parameter_regex):
         # be used as keyword arguments anyway.
         different_positional = different_kwonly = False
 
-    # Both or none should have extra variadics, otherwise the method
-    # loses or gains capabilities that are not reflected into the parent method,
-    # leading to potential inconsistencies in the code.
-    different_kwarg = (
-        sum(1 for param in (original.args.kwarg, overridden.args.kwarg) if not param)
-        == 1
-    )
-    different_vararg = (
-        sum(1 for param in (original.args.vararg, overridden.args.vararg) if not param)
-        == 1
-    )
+    # Arguments will only violate LSP if there are variadics in the original
+    # that are then removed from the overridden
+    kwarg_lost = original.args.kwarg and not overridden.args.kwarg
+    vararg_lost = original.args.vararg and not overridden.args.vararg
 
-    return any(
-        (different_positional, different_kwarg, different_vararg, different_kwonly)
-    )
+    return any((different_positional, kwarg_lost, vararg_lost, different_kwonly))
 
 
 def _is_invalid_base_class(cls):

tests/functional/a/arguments_differ.py

@@ -154,8 +154,10 @@ def impl(arg1, arg2, **kwargs):
 
 class MyClass(SuperClass):
 
-    def impl(self, *args, **kwargs): # [arguments-differ]
-
+    def impl(self, *args, **kwargs):
+        """
+        Acceptable use of vararg in subclass because it does not violate LSP.
+        """
         super(MyClass, self).impl(*args, **kwargs)
 
 
@@ -170,6 +172,7 @@ class SecondChangesArgs(FirstHasArgs):
     def test(self, first, second, *args): # [arguments-differ]
         pass
 
+
 class Positional(object):
 
     def test(self, first, second):
@@ -178,13 +181,35 @@ def test(self, first, second):
 
 class PositionalChild(Positional):
 
-    def test(self, *args): # [arguments-differ]
-        """Accepts too many.
-
-        Why subclassing in the first case if the behavior is different?
+    def test(self, *args):
+        """
+        Acceptable use of vararg in subclass because it does not violate LSP.
         """
         super(PositionalChild, self).test(args[0], args[1])
 
+class Mixed(object):
+
+    def mixed(self, first, second, *, third, fourth):
+        pass
+
+
+class MixedChild1(Mixed):
+
+    def mixed(self, first, *args, **kwargs):
+        """
+        Acceptable use of vararg in subclass because it does not violate LSP.
+        """
+        super(MixedChild1, self).mixed(first, *args, **kwargs)
+
+
+class MixedChild2(Mixed):
+
+    def mixed(self, first, *args, third, **kwargs):
+        """
+        Acceptable use of vararg in subclass because it does not violate LSP.
+        """
+        super(MixedChild2, self).mixed(first, *args, third, **kwargs)
+
 
 class HasSpecialMethod(object):
 

tests/functional/a/arguments_differ.txt

@@ -3,6 +3,4 @@ arguments-differ:23:ChildDefaults.test:Parameters differ from overridden 'test'
 arguments-differ:41:ClassmethodChild.func:Parameters differ from overridden 'func' method
 arguments-differ:68:VarargsChild.has_kwargs:Parameters differ from overridden 'has_kwargs' method
 arguments-differ:71:VarargsChild.no_kwargs:Parameters differ from overridden 'no_kwargs' method
-arguments-differ:157:MyClass.impl:Parameters differ from overridden 'impl' method
-arguments-differ:170:SecondChangesArgs.test:Parameters differ from overridden 'test' method
-arguments-differ:181:PositionalChild.test:Parameters differ from overridden 'test' method
+arguments-differ:172:SecondChangesArgs.test:Parameters differ from overridden 'test' method

tests/functional/a/arguments_differ_py3.py

@@ -16,6 +16,9 @@ def kwonly_4(self, *, first, second=None):
     def kwonly_5(self, *, first, **kwargs):
         "Keyword only and keyword variadics."
 
+    def kwonly_6(self, first, second, *, third):
+        "Two positional and one keyword"
+
 
 class Foo(AbstractFoo):
 
@@ -33,4 +36,12 @@ def kwonly_4(self, first, second): # [arguments-differ]
 
     def kwonly_5(self, *, first): # [arguments-differ]
         "Keyword only, but no variadics."
-
+
+    def kwonly_6(self, *args, **kwargs): # valid override
+        "Positional and keyword variadics to pass through parent params"
+
+
+class Foo2(AbstractFoo):
+
+    def kwonly_6(self, first, *args, **kwargs): # valid override
+        "One positional with the rest variadics to pass through parent params"

tests/functional/a/arguments_differ_py3.txt

@@ -1,5 +1,5 @@
-arguments-differ:22:Foo.kwonly_1:Parameters differ from overridden 'kwonly_1' method
-arguments-differ:25:Foo.kwonly_2:Parameters differ from overridden 'kwonly_2' method
-arguments-differ:28:Foo.kwonly_3:Parameters differ from overridden 'kwonly_3' method
-arguments-differ:31:Foo.kwonly_4:Parameters differ from overridden 'kwonly_4' method
-arguments-differ:34:Foo.kwonly_5:Parameters differ from overridden 'kwonly_5' method
+arguments-differ:25:Foo.kwonly_1:Parameters differ from overridden 'kwonly_1' method
+arguments-differ:28:Foo.kwonly_2:Parameters differ from overridden 'kwonly_2' method
+arguments-differ:31:Foo.kwonly_3:Parameters differ from overridden 'kwonly_3' method
+arguments-differ:34:Foo.kwonly_4:Parameters differ from overridden 'kwonly_4' method
+arguments-differ:37:Foo.kwonly_5:Parameters differ from overridden 'kwonly_5' method