Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 tasks done
amogorkon opened this issue Mar 28, 2023 · 8 comments
Closed
2 tasks done

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

amogorkon opened this issue Mar 28, 2023 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@amogorkon
Copy link

Checklist

  • I have searched the Sourcery documentation for the issue, and found nothing
  • 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

Unbenannt

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

@amogorkon amogorkon added the bug Something isn't working label Mar 28, 2023
@reka
Copy link
Contributor

reka commented Mar 28, 2023

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. 🤔

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
Copy link
Author

amogorkon commented Mar 29, 2023

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
Copy link

wardy3 commented Sep 21, 2023

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
Copy link
Contributor

bm424 commented Oct 16, 2023

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
Copy link

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.

2023-10-17_005317

Please fix this stupid bug. It is very annoying.

@Estrangeling
Copy link

Estrangeling commented Oct 16, 2023

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

2023-10-17_010641

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
Copy link
Contributor

bm424 commented Oct 17, 2023

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 bm424 self-assigned this Oct 17, 2023
@bm424
Copy link
Contributor

bm424 commented Oct 19, 2023

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.

@bm424 bm424 closed this as completed Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants