instance-method-first-arg-name triggers on unrelated global funcs

[amogorkon]

Checklist

• [x] I have searched the Sourcery documentation for the issue, and found nothing
• [x] I have checked there are no open bugs referencing the same bug or problem

Description

In some cases when a global function follows a class with methods, sourcery suggests to add self to the arg or change the first parameter to "self", although it's not a method. I attached a screenshot with the issue.

Code snippet that reproduces issue

class Test:
    def __init__(self, bla):
        self.bla = bla
    def foo(self):
        print("foo")
    def bar(self):
        print("bar")

def bar(bla):
    print(bla.bla)

Debug Information

IDE Version: Version: 1.76.2 (user setup) Commit: ee2b180d582a7f601fa6ecfdad8d9fd269ab1884 Datum: 2023-03-14T17:55:54.936Z Electron: 19.1.11 Chromium: 102.0.5005.196 Node.js: 16.14.2 V8: 10.2.154.26-electron.0 Betriebssystem: Windows_NT x64 10.0.19045 Sandkasten: No

Sourcery Version: v1.0.9

Operating system and Version: Windows 10

[reka]

Hi @amogorkon ,

Thanks a lot for reporting this issue.

Unfortunately, I haven't managed to reproduce it yet. When I copy the code with the Test class and bar function, Sourcery doesn't show me an error for bar. :thinking:

Do you have an idea what might influence this? Is there anything peculiar about retrieve_task_by_id?

Which IDE are you using? Does Sourcery also show this suggestion with self when you run sourcery review in the CLI?

Thanks for your help.

Reka

[amogorkon]

When I copy the code with the Test class and bar function, Sourcery doesn't show me an error for bar.

Weird.

Is there anything peculiar about retrieve_task_by_id?

Nothing. It's remarkably unremarkable. Here it is:

def retrieve_task_by_id(db, ID: int) -> Task:
    query = db.execute(
        f"""
SELECT id, do, notes, deleted, draft, inactive, done, primary_activity_id, secondary_activity_id, space_id, priority, level_id, adjust_time_spent, difficulty, fear, embarrassment, last_checked, workload, ilk FROM tasks WHERE id == {ID};
    """
    )
    res = query.fetchone()
    assert res is not None, breakpoint()
    return Task(*res)

Maybe sourcery is confused by its placement directly below a class with the first parameter not being annotated?

Which IDE are you using?

VSC.

Does Sourcery also show this suggestion with self when you run sourcery review in the CLI?

I haven't tried that.

I turned off the sourcery suggestion for this error after reporting the issue.. if you tell me how to re-enable it, I can do more experiments to help you out.

[wardy3]

Which IDE are you using? Does Sourcery also show this suggestion with self when you run sourcery review in the CLI?

I'm also using VSCode and get the same issue. It must be related to previous code, as I copied my class and the global function below it into a new file but the error is now gone.

I can't work it out. It seems to appear if I keep all my imports at the top but when I delete one, it goes away. There's a mix of standard. 3rd party, and local imports. No idea ...

It does not occur if I use sourcery review mydir where the file resides

[bm424]

I've had another look at this and I can't reproduce it. If someone has a complete .py file they could share and some process they've found that reliably creates the refactoring in the wrong place, we have a shot at solving this.

[Estrangeling]

I have found a simple way to perfectly reproduce the issue on my setup, without failure.

Steps to reproduce:

Create a new text file in Visual Studio Code.

Select Python as the language.

Add a dummy function to the empty text file, like the following:

def func(foo):
    return foo

Add a dummy class before the function, like so:

class Dummy:
    def __init__(self):
        pass

def func(foo):
    return foo

And the bug will be triggered.

Please fix this stupid bug. It is very annoying.

[Estrangeling]

This is a bit more complicated example, I have defined a function first, and added a dummy class before it, the function has its parameters annotated, but the bug still triggers. This would rule out that annotations are related in anyway.

class Dummy:
    def __init__(self):
        pass

def power(base: float, exp: int):
    if not exp:
        return 1.0

    if exp < 0:
        base = 1 / base
        exp = -exp

    p = 1.0
    while exp:
        if exp & 1:
            p = base * p

        base *= base
        exp >>= 1

    return p

Closing the editor and reopening it, the bug goes away.

Note to reproduce the issue, you can't just copy paste the full code, or paste the function first and the class second, that won't trigger the bug.

You have to type the class definition by hand above the definition of an existing function in a script, in order to trigger the bug.

If you paste a function, and then type a class definition with a method above the function definition in a script, the bug will be triggered.

[bm424]

Thanks @Estrangeling, with that I've been able to reproduce and track it down. There are two issues working in tandem here: first, the refactoring is triggered specifically when this code is written (note the syntax error)

class Example:
    def
def something(a):
    ...

So something about the parser is misattributing the function when it sees this syntax error.

The second problem is that we cache refactorings to avoid re-computing them, and in this case the cached refactoring persists until you restart the IDE.

Thanks for your help, and I'll take a look at this today - look out for a fix in the next release.

[bm424]

A quick update - I'm still working on this (turns out to be quite a low-level bug) so unfortunately it didn't make it into yesterday's release.