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

Method renaming is failing for instance methods/properties #478

Open
RaptorX opened this issue Feb 21, 2024 · 8 comments
Open

Method renaming is failing for instance methods/properties #478

RaptorX opened this issue Feb 21, 2024 · 8 comments

Comments

@RaptorX
Copy link
Contributor

RaptorX commented Feb 21, 2024

When trying to rename methods/properties of a class I have noticed that when renaming static props/methods everything works fine.

image

But when I try the same with non static members i get this error:

image

@thqby
Copy link
Owner

thqby commented Feb 21, 2024

This operation is not supported and the type of each variable cannot be accurately determined.

@thqby
Copy link
Owner

thqby commented Feb 21, 2024

You posted it last year. #423

@RaptorX
Copy link
Contributor Author

RaptorX commented Feb 21, 2024

Ah, I see, I didnt think they were related because the older one was referring to custom behavior for given properties, while this one referred to properties being clearly defined in the class as having the default behavior.

But i guess you mean you cant derive wether they are properties vs methods?

In any case, if it is not technically possible to do it, dont worry to much about it.

My understanding was that the extension was taking a look at the class definition to load the context for later using the renaming functionality but I guess that is not what is doing.

@RaptorX RaptorX closed this as completed Feb 21, 2024
@RaptorX
Copy link
Contributor Author

RaptorX commented Nov 29, 2024

This operation is not supported and the type of each variable cannot be accurately determined.

Why was this supported before and it is currently supported on other extensions like AHK++ for autohotkey v1?
Is there any technical reason from this not working in your extension specifically?

screen-recording.mp4

@RaptorX RaptorX reopened this Nov 29, 2024
@thqby
Copy link
Owner

thqby commented Nov 29, 2024

class aa {
    Method() {
        
    }
}
class bb {
    Method() {
        
    }
}
c := {Method:fn}
a := new aa()
a.Method()
b := new bb()
b.Method()
c.Method()

What if it's something like this? If you rename the method when the type is ambiguous, it may break the code unless all the methods with the same name are modified?

@RaptorX
Copy link
Contributor Author

RaptorX commented Dec 1, 2024

unless all the methods with the same name are modified?

Thats what the v1 extension does.

Probably the way to go is have the parser have a map/variable that keeps track of instatiation.
When a renaming will happen it will use the map to point to the current class the variable is pointing to.

Pseudocode

tracker := Map()

; when parser finds a class instantiation
tracker.Set('a', 'aa')

; when parser finds the same var name instatiating a different class, it would update the current info
tracker.Set('a', 'bb')

; later when renaming we just need to retrieve the current class for it

target_class := tracker.Get('a') ; should return bb_class
RenameMethod('target_class', 'new_method_name')

I definitely dont know what your constraints are, but that might be one way about it.
Im not sure how that would affect performance of the parser the first time you are reading the files though, so take this whole idea with a grain of salt.

@thqby
Copy link
Owner

thqby commented Dec 1, 2024

The above is only a simple case. When these are function parameters, different types can be passed in, and they will be difficult to analyze in more complex call chains.

@RaptorX
Copy link
Contributor Author

RaptorX commented Dec 2, 2024

The above is only a simple case. When these are function parameters, different types can be passed in, and they will be difficult to analyze in more complex call chains.

Well sure, but the scope of my question is limited to the method names only, I know that parameters are a different beast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants