arguments-differ is triggered if just arg names changed

[socketpair]

class fruit:
    def _do_actions(self, fruit_name: str):
        pass

class orange(fruit):
    def _do_actions(self, orange_name: str):   # will trigger arguments-differ
        pass

I think we should differentiate errors:

1. arguments-differ if something changed except the names (i.e., number of args, their types, or args become kwargs, or so.).
2. a new error: arguments-renamed should be raised only if a name was changed.

But arguments changed in a way that old arguments change their positions (i.e. swapped) arguments-differ should be raise anyway:

class fruit:
    def xxx(self, a: str, b: str):
        pass

class orange(fruit):
    def xxx(self, b: str, a:str):   # MUST trigger arguments-differ
        pass

Rationale:
If I just add # pylint: disable=arguments-differ it would hide other arguments-related errors in this line. As a workaround I can write something like:

class orange(fruit):
    def _do_actions(self, fruit_name: str):   # will trigger arguments-differ
        super()._do_actions(fruit_name)
        orange_name = fruit_name
        # use orange_name in the code.

but this is slightly stupid. I would not like to do such a trick just to silence the linter.

[PCManticore]

Thanks, separating this behaviour into two checks sounds like a good idea.

[ksaketou]

Hello, I would like to work on that. If I understood correctly, the arguments-renamed will be triggered only when the attribute names have changed. In any other case, the arguments-differ will be triggered. Right? @Pierre-Sassoulas @PCManticore

[Pierre-Sassoulas]

Hey, nice to see you're becoming a regular 😄 I assigned you the issue !

Regarding the task itself I think what @socketpair proposed is nice:

1. arguments-differ if something changed except the names (i.e., number of args, their types, or args become kwargs, or so.).
2. a new error: arguments-renamed should be raised only if a name was changed.

If I understood correctly will be triggered only when the attribute names have changed but also without the type changing, ie:

import enum


class Condiment(enum.Enum):
    CINAMON = 1
    SUGAR = 2

class Fruit:
    def brew(self, fruit_name: str):
        print(f"Brewing a fruit named {fruit_name}")

    def eat_with_condiment(self, fruit_name:str, condiment: Condiment):
        print(f"Eating a fruit named {fruit_name} with {condiment}")

class Orange(Fruit):
    def brew(self, orange_name: str):   # [arguments-renamed]
        print(f"Brewing an orange named {orange_name}")

    def eat_with_condiment(self, condiment: Condiment, orange_name:str): # [argument-differ]
        print(f"Eating a fruit named {orange_name} with {condiment}")

class Banana(Fruit):
    def brew(self, fruit_name: str): # No warning
        print(f"Brewing a banana named {fruit_name}")

    def eat_with_condiment(self, fruit_name:str, condiment: Condiment, error:str): # [argument-differ]
        print(f"Eating a fruit named {fruit_name} with {condiment}")
        
    # ... Maybe we can create other fruit for type changing, or args becoming kwargs depending 
   # on the existing functional test covering that already ?
        

[ksaketou]

Nice to see you again 😃 Thank you for the example. I think for the arguments-differ there are some tests for *args and **kwargs already but they could cover some more cases. The tests for the arguments-renamed will just check the name and the type of the arguments.

[ksaketou]

Hello @Pierre-Sassoulas , I have a question. I'm having difficulty understanding what this line checks. What regex does the dummy_parameter_regex contain? https://github.com/PyCQA/pylint/blob/5e1928b325bc798f5be1ab94031bf6816d058d9f/pylint/checkers/classes.py#L276

[Pierre-Sassoulas]

It seem to be coming from the call line 1763:
https://github.com/PyCQA/pylint/blob/5e1928b325bc798f5be1ab94031bf6816d058d9f/pylint/checkers/classes.py#L1763

Dummy regex is defined here in the class:

    @astroid.decorators.cachedproperty
    def _dummy_rgx(self):
        return get_global_option(self, "dummy-variables-rgx", default=None)

So the value come straight from the configuration file where it can be defined this way:

# A regular expression matching the name of dummy variables (i.e. expected to
# not be used).
dummy-variables-rgx=_+$|(_[a-zA-Z0-9_]*[a-zA-Z0-9]+?$)|dummy|^ignored_|^unused_

To be fair I don't think we have too pass the regex variable around because it seem to be a constant (?). Maybe I did not understand that part well.

[ksaketou]

Do you think that I can compare the parameter types using type() and isinstance()? I'm using those to find the types of original_param and overridden_param but It doesn't seem to make the comparison correctly. The issue is that I cannot identify the type of the arguments and compare them. When I use the _get_node_type method, isinstance() raises an error. 😕

[Pierre-Sassoulas]

I don't know if this answer your question specifically but about how to use isinstance, in LenChecker here's how we test if a node is function_name(...):

# is_call_of_name(parent, "function_name")

def is_call_of_name(node: astroid.node_classes.NodeNG, name: str) -> bool:
    """Checks if node is a function call with the given name"""
    return (
        isinstance(node, astroid.Call)
        and isinstance(node.func, astroid.Name)
        and node.func.name == name
    )

[ksaketou]

Ok I get it. However, the parameters I want to compare are AssignName objects so I cannot get their type through this node. Should I use the .parent attribute?

[Pierre-Sassoulas]

I think you need to use infer() (be careful it can raise InferenceError`` if we do not manage to infer the type. I'm not absolutely sure of it.

[ksaketou]

Hello, I have a few questions regarding the arguments-renamed error. The output message will be the same as the one below which was added to the arguments-differ?https://github.com/PyCQA/pylint/blob/6b3c49895f5e1750e2cc6c4e0c4e0c5460056cd2/tests/functional/a/arguments_differ.txt#L5
Also, should I create tests for Python3 separately? (since that's what happens for the arguments-differ)

[Pierre-Sassoulas]

Yes I think the message would be the same, but not the name of the message. I did not understand the question about tests for python 3, I think we're going only test for python 3 since we do not have test for python 2 now ? The python 2 tests are historical, don't worry about them :)

[ksaketou]

I am asking because on my initial PR the checks for pyton3 had failed. There are two test files for arguments-differ, the first one is arguments_differ.py and the second one is arguments_differ_py3.py.

[Pierre-Sassoulas]

Hum ok, I think they could be merged as implicitly every tests if py3 now.

[ksaketou]

You mean to merge these two files in one? There is also this file that specifies the Python version.

[Pierre-Sassoulas]

Yes merging the two file, the .rc file can be removed, it was the configuration for arguments_differ_py3.py but now we only have python > 3.0.