vshaxe / vshaxe

Haxe Support for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=nadako.vshaxe
MIT License
325 stars 59 forks source link

Issues with "Rename Symbol" #631

Open dubspeed opened 2 months ago

dubspeed commented 2 months ago

👋 I am working on a larger, commercial Haxe codebase, and we would like to see "Rename Symbol" in the VSHaxe to be our go-to tool for all things renaming in the project.

However, using the tool often yields unpredictable results and crashes, seemingly randomly renaming some symbols completely, some partially, often making it the worse choice over a tedious manual search and replace operations.

All examples have been created with:

I'll try to illustrate with examples, however those are only tiny portions of things I could prepare in a distilled down from our experiences in this bug report format.

Uploaded as well here

package src;

class DemoClassA {
    public var someValue:String;
}

class SomeOtherClass {
    public function isEventFeatureFlag(aString:String):Bool {
        return false;
    }

    public function new() {}
}

class WontRename {
    var TESTRENAME:Bool = false;

    var _view:{
        isAssetsLoaded:Bool
    };
    var _someOtherClass:SomeOtherClass = new SomeOtherClass();

    function _onFeatureEnds(event:DemoClassA) {
        // misses a rename in the next line
        if (_view.isAssetsLoaded && _someOtherClass.isEventFeatureFlag(event.someValue) && TESTRENAME) {
            TESTRENAME = false;
        }
    }
}
  1. Fails to rename local symbols.

Renaming "TESTRENAME" only catches 2 out of 3 cases. As mentioned above, this is one example to show and highlight that we often have cases where local variables are not completely replaced. We would love to have a tool that 100% renames anything local and is at least as good as a search and replace.

  1. Random results on anything "public"

In the example above, renaming "someValue" in "DemoClassA" does not affect WontRename class usage. Renaming public symbols is random at best.

We would love to see that public symbols, e.g. function names, variables, classes, properties etc. can be renamed.

  1. Unclear what it can rename and what it can't

Following up on the above, it seems that the functionality is limited somehow, e.g. when trying to rename the type of _someOtherClass in line 22 there is an error message ("not supported").

In the underlying Haxe rename codebase, which I assume, is the tool doing the job here (and please correct me if I am wrong), it states a list of "features", however misses for example "public functions". Is it supported? If so, then it's broken (see example from HaxeFlixel below). Otherwise, I would wish for some form of error message or user communication.

We would love the tool to either "do what it says on the tin", or, if that can't be achieved, better communication about what works and what not.

  1. Parsing errors on specific files:

Regarding 'haxe-rename', the tool will crash on certain files. An example is here. Using the tool directly, e.g. like node rename.js -s src -l src/LocalVar.hx@221 -n FOOBAR will crash it. I suspect that this will also cause errors in the plugin, but I am not certain how and when the file will be parsed by "Rename Symbol" different to using the 'haxe-rename'.

As the code in the example ForceRenameCrash.hx is valid Haxe code, we wonder if there is a better solution to using haxeparser-library, as it seems faulty.

Overall, we would like to push for a stable, reliable refactoring tool and from our experiences with the tool it currently is not. Since it seems to us that the tool is building its own AST and reparses all haxe files when run, if it wouldn't be better to move the functionality into the compiler itself?


(*1) Example of renaming a public function fail:

AlexHaxe commented 2 months ago

thanks for reporting your issues!

it's a correct assumption that haxe-rename is the underlying library that provides rename functionality. it hasn't seen a lot of progress or work in the last two years mainly due to lack of bug reports.

can you open individual issues in haxe-rename repo so it's easier to track progress and completion? feel free to report any other issue you might have. you can reference this issue as sort of a master document.

dubspeed commented 2 months ago

👋 I quickly opened 3 issues on haxe-rename repo, as advised!

Issue for point 1. from above Issue for point 2. from above Issue for point 4. from above