Rename is not updating all usages.

[as3boyan]

package foo;
import foo.MainClass; //Doesn't make any sense - and it should be marked as unused
class MainClass {
  var main:Main<caret>Class;
}

Class needs to be imported elsewhere. Or just rename without import first.

Anyway, even in this case, plugin should understand that it's the same class, and rename all references.

About Travis CI: it's a great and very interesting opportunity, but currently it spams me with failing cases on each commit in pull request, so I have to disable email notifications.

[as3boyan]

package a;
import a.MainClass;
class MainClass {
    var main:MainClass;
}

EDIT: Find usages works fine.

[as3boyan]

Ok, so, the things is, rename renames class and file, but when it proceeds to usages, it checks if references can be resolved, after renaming class, they are no longer valid anymore of course

[EBatTiVo]

So we have to collect all of the references before you allow the rename to proceed. Right? Don't we have to do this anyway, in order to put up the dialog?

[as3boyan]

I'm not sure now, I don't get it yet, but it seems like HaxeReferenceExpression, has bindToElement, which create new reference to file/package, not sure about class

I think it might be close, basically it changes reference to point to certain file or package, but should it do it for class

[as3boyan]

I think Move File functionality may be similar to this, but still I don't really understand how in old version this compiles and works fine, but in new we have to implement it.

[EBatTiVo]

My solution is to check out an old version (eb94f32 is just before we started the ClassHierarchy work), watch it work, then git bisect until I find the checkin that broke it. Takes about 12 builds and I usually have the change in 20 minutes. Then, I inspect the change (git bisect visualize), check out the immediately previous (working) code and make little independent changes until I figure out which one broke things.

If git bisect doesn't work for you, then you might have to do some more sleuthing to see if it was ever implemented.

[as3boyan]

rename/move works here: 8f9adcb

next commit - doesn't works: 18d9367

[sganapavarapu1]

was reviewing Move functioning on Friday, found that -

before move: findUsages part of Move was returning correct list of usages (be it import statements, or variable type declarations, or someVar.getPropertyOfTypeOfClassBeingMoved() etc.)

after move: I would expect it to correctly fix all those instances from findUsages list

unfortunately, it was only fixing part of findUsages returned list... it wasn't fixing "import" statement occurrences.

There wasn't any implementation for Move earlier - it was using underlying common implementation of open api. But now, there's a need to fix this!

@as3boyan When I look at the diff of the commits you mentioned above... it is not clear how rename/move broke between those 2 commits :(

I'll try to figure out either how Move broke? or implement the Move file/package functionality over next 2-3 days.

[as3boyan]

I was testing commits before these, this one may be breaking be548c1

It seems like this commit broke testMovePackage

If we could build every commit and see when something got broken, that would be awesome.

About git bisect I need to try it sounds good.

[as3boyan]

I got testMovePackage to work and almost implemented some rename/move, but not sure how it worked before.

@sganapavarapu1

There wasn't any implementation for Move earlier - it was using underlying common implementation of open api. But now, there's a need to fix this!

I understand, but I would prefer that common implementation.

[as3boyan]

unfortunately, it was only fixing part of findUsages returned list... it wasn't fixing "import" statement occurrences.

So previous (common) implementation didn't worked for this case?

[sganapavarapu1]

@as3boyan
yes, I too prefer common implementation
I'm not sure whether previous implementation has 'import' statements also working!

Today, I was about to start working on testMovePackage, testMoveFile1, testMoveFile2 - but I will not do that now because you already fixed testMovePackage and you are working on some rename/move. It will be duplication of effort - I will pick up something else.

Thanks,

[as3boyan]

4b82210 may break testMoveFile2

[EBatTiVo]

@as3boyan I see a PR was merged for this. Is this fixed?

[as3boyan]

@EBatTiVo It was for testMovePackage I think

[as3boyan]

858ef3a is indeed broke the move file and package because
HaxeTypeTag typeTag = PsiTreeUtil.getChildOfType(parent, HaxeTypeTag.class); used to throw unimplemented error for PsiPackage something like that

[as3boyan]

But it's different error, and if I revert change in master branch - that doesn't fixes the issue.

[as3boyan]

It has to do something with resolving/checking each usage using isReferenceTo

[as3boyan]

@EBatTiVo indeed PR was for rename, but not sure about it, from some point it may be enough to get tests to work, it might be connected to move functionality. Need to investigate.

Next git bisect bad commit is e1822d7.
Is more likely to break move. Will look into it.

[as3boyan]

I had to revert changes in e1822d7, to make it work

@EBatTiVo Please look into #271

[as3boyan]

#272 should fix this issue

[EBatTiVo]

Fixed with #272. Closing.