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

Eclipse property refactoring should update getter / setter references #210

Closed
lombokissues opened this Issue Jul 14, 2015 · 22 comments

Comments

Projects
None yet
5 participants
@lombokissues
Collaborator

lombokissues commented Jul 14, 2015

Migrated from Google Code (issue 137)

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 steve.skrla   🕗 Jul 28, 2010 at 21:05 UTC

What steps will reproduce the problem?

  1. Create a @ Data object with a field named "test"
  2. Create another object that references the getTest or setTest method.
  3. Move the carrot into the test field of the data object and press alt+shift+r
  4. Rename test to testRenamed

What is the expected output? What do you see instead?
expected: The getTest or setTest references should be updated to getTestRenamed / setTestRenamed respectively.

actual: they are not modified and a compiler error results.

What version of the product are you using? On what operating system?
Eclipse build 20100218-1602 with Lombok 0.9.3 on Ubuntu

Collaborator

lombokissues commented Jul 14, 2015

👤 steve.skrla   🕗 Jul 28, 2010 at 21:05 UTC

What steps will reproduce the problem?

  1. Create a @ Data object with a field named "test"
  2. Create another object that references the getTest or setTest method.
  3. Move the carrot into the test field of the data object and press alt+shift+r
  4. Rename test to testRenamed

What is the expected output? What do you see instead?
expected: The getTest or setTest references should be updated to getTestRenamed / setTestRenamed respectively.

actual: they are not modified and a compiler error results.

What version of the product are you using? On what operating system?
Eclipse build 20100218-1602 with Lombok 0.9.3 on Ubuntu

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 steve.skrla   🕗 Jul 28, 2010 at 21:06 UTC

I realize this is a feature request, thanks for all the great work so far =)

Collaborator

lombokissues commented Jul 14, 2015

👤 steve.skrla   🕗 Jul 28, 2010 at 21:06 UTC

I realize this is a feature request, thanks for all the great work so far =)

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 skyewire   🕗 Dec 10, 2010 at 00:59 UTC

I use Refactor > Rename in Eclipse all the time, so unfortunately I can't really use lombok until this issue is fixed :(

Collaborator

lombokissues commented Jul 14, 2015

👤 skyewire   🕗 Dec 10, 2010 at 00:59 UTC

I use Refactor > Rename in Eclipse all the time, so unfortunately I can't really use lombok until this issue is fixed :(

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 reinierz   🕗 Dec 10, 2010 at 17:14 UTC

Re: skyewire: renaming a private field doesn't rename a fetter either. A workaround: generate the getter, then delete it, then rename the field.

Collaborator

lombokissues commented Jul 14, 2015

👤 reinierz   🕗 Dec 10, 2010 at 17:14 UTC

Re: skyewire: renaming a private field doesn't rename a fetter either. A workaround: generate the getter, then delete it, then rename the field.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 skyewire   🕗 Dec 15, 2010 at 02:07 UTC

You can rename the getter/setter when renaming a private field - just press Alt-Shift-R-R and then check the "Rename getter/setter" check boxes. Anyway, thanks for the workaround.

Collaborator

lombokissues commented Jul 14, 2015

👤 skyewire   🕗 Dec 15, 2010 at 02:07 UTC

You can rename the getter/setter when renaming a private field - just press Alt-Shift-R-R and then check the "Rename getter/setter" check boxes. Anyway, thanks for the workaround.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 r.spilker   🕗 Dec 15, 2010 at 09:59 UTC

If you use Alt-Shift-R twice, check the Rename getter/setter checkboxes and press the preview button, you should uncheck the getter and setter checkbox from the class you're working on, but keep the checkboxes of the Update getter occurrence and Update setter occurrence checked.

Collaborator

lombokissues commented Jul 14, 2015

👤 r.spilker   🕗 Dec 15, 2010 at 09:59 UTC

If you use Alt-Shift-R twice, check the Rename getter/setter checkboxes and press the preview button, you should uncheck the getter and setter checkbox from the class you're working on, but keep the checkboxes of the Update getter occurrence and Update setter occurrence checked.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 reinierz   🕗 Jan 22, 2011 at 11:49 UTC

Issue #257 has been merged into this issue.

Collaborator

lombokissues commented Jul 14, 2015

👤 reinierz   🕗 Jan 22, 2011 at 11:49 UTC

Issue #257 has been merged into this issue.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 latchkey   🕗 Oct 28, 2011 at 03:37 UTC

The workarounds with Eclipse 3.7 and lombok 0.10.1 don't work at all. It just errors out.

Collaborator

lombokissues commented Jul 14, 2015

👤 latchkey   🕗 Oct 28, 2011 at 03:37 UTC

The workarounds with Eclipse 3.7 and lombok 0.10.1 don't work at all. It just errors out.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 latchkey   🕗 Jan 17, 2012 at 09:12 UTC

Still definitely not working with latest head of lombok.

Collaborator

lombokissues commented Jul 14, 2015

👤 latchkey   🕗 Jan 17, 2012 at 09:12 UTC

Still definitely not working with latest head of lombok.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 saurabhnarain   🕗 May 16, 2013 at 09:03 UTC

Pressing Shift-Alt-R twice gives me an option but it errors out org.eclipse.core.runtime.AssertionFailedException: assertion failed:

Is this issue still not fixed?

Using eclipse 4.2 lombok 0.11.6 ubuntu 10.04

Collaborator

lombokissues commented Jul 14, 2015

👤 saurabhnarain   🕗 May 16, 2013 at 09:03 UTC

Pressing Shift-Alt-R twice gives me an option but it errors out org.eclipse.core.runtime.AssertionFailedException: assertion failed:

Is this issue still not fixed?

Using eclipse 4.2 lombok 0.11.6 ubuntu 10.04

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 Simeon.Zverinski   🕗 May 20, 2013 at 20:28 UTC

I also experienced the same problem in Eclipse 4.2 and Lombok v0.11.8. Looking at the eclipse error log, the error comes from the following exception

org.eclipse.core.runtime.AssertionFailedException: assertion failed:
at org.eclipse.core.runtime.Assert.isTrue(Assert.java:110)
at org.eclipse.core.runtime.Assert.isTrue(Assert.java:96)
at org.eclipse.jdt.internal.corext.refactoring.rename.RenameFieldProcessor.checkAccessorDeclarations(RenameFieldProcessor.java:509)
at org.eclipse.jdt.internal.corext.refactoring.rename.RenameFieldProcessor.checkAccessor(RenameFieldProcessor.java:487)
at org.eclipse.jdt.internal.corext.refactoring.rename.RenameFieldProcessor.doCheckFinalConditions(RenameFieldProcessor.java:461)
at org.eclipse.jdt.internal.corext.refactoring.rename.JavaRenameProcessor.checkFinalConditions(JavaRenameProcessor.java:48)
at org.eclipse.ltk.core.refactoring.participants.ProcessorBasedRefactoring.checkFinalConditions(ProcessorBasedRefactoring.java:224)
at org.eclipse.ltk.core.refactoring.CheckConditionsOperation.run(CheckConditionsOperation.java:85)
at org.eclipse.ltk.core.refactoring.CreateChangeOperation.run(CreateChangeOperation.java:121)
at org.eclipse.ltk.core.refactoring.PerformChangeOperation.run(PerformChangeOperation.java:209)
at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:2344)
at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run(WorkbenchRunnableAdapter.java:87)
at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)

I've managed to overcome this issue by patching eclipse's RenameFieldProcessor.java and replacing "Assert.isTrue(groupDeclarations.length > 0);" on the line 509 with the "if (groupDeclarations.length > 0) {". Surprisingly renaming started working just fine (using Alt-Shift-R twice)after rebuilding and replacing the plugin org.eclipse.jdt.ui_3.8.2.v20130107-165834.jar.

Collaborator

lombokissues commented Jul 14, 2015

👤 Simeon.Zverinski   🕗 May 20, 2013 at 20:28 UTC

I also experienced the same problem in Eclipse 4.2 and Lombok v0.11.8. Looking at the eclipse error log, the error comes from the following exception

org.eclipse.core.runtime.AssertionFailedException: assertion failed:
at org.eclipse.core.runtime.Assert.isTrue(Assert.java:110)
at org.eclipse.core.runtime.Assert.isTrue(Assert.java:96)
at org.eclipse.jdt.internal.corext.refactoring.rename.RenameFieldProcessor.checkAccessorDeclarations(RenameFieldProcessor.java:509)
at org.eclipse.jdt.internal.corext.refactoring.rename.RenameFieldProcessor.checkAccessor(RenameFieldProcessor.java:487)
at org.eclipse.jdt.internal.corext.refactoring.rename.RenameFieldProcessor.doCheckFinalConditions(RenameFieldProcessor.java:461)
at org.eclipse.jdt.internal.corext.refactoring.rename.JavaRenameProcessor.checkFinalConditions(JavaRenameProcessor.java:48)
at org.eclipse.ltk.core.refactoring.participants.ProcessorBasedRefactoring.checkFinalConditions(ProcessorBasedRefactoring.java:224)
at org.eclipse.ltk.core.refactoring.CheckConditionsOperation.run(CheckConditionsOperation.java:85)
at org.eclipse.ltk.core.refactoring.CreateChangeOperation.run(CreateChangeOperation.java:121)
at org.eclipse.ltk.core.refactoring.PerformChangeOperation.run(PerformChangeOperation.java:209)
at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:2344)
at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run(WorkbenchRunnableAdapter.java:87)
at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)

I've managed to overcome this issue by patching eclipse's RenameFieldProcessor.java and replacing "Assert.isTrue(groupDeclarations.length > 0);" on the line 509 with the "if (groupDeclarations.length > 0) {". Surprisingly renaming started working just fine (using Alt-Shift-R twice)after rebuilding and replacing the plugin org.eclipse.jdt.ui_3.8.2.v20130107-165834.jar.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 shollander1   🕗 Jun 03, 2013 at 14:43 UTC

Simeon.Z : Sounds like you tracked it down to a bug in eclipse. How about submitting a bug report to eclipse and getting it fixed so we all can benefit! Thanks!

Collaborator

lombokissues commented Jul 14, 2015

👤 shollander1   🕗 Jun 03, 2013 at 14:43 UTC

Simeon.Z : Sounds like you tracked it down to a bug in eclipse. How about submitting a bug report to eclipse and getting it fixed so we all can benefit! Thanks!

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 Simeon.Zverinski   🕗 Jun 04, 2013 at 21:14 UTC

@ sholland: actually I don't think it's a bug in Eclipse. The assert could make sense from the standard eclipse rename workflow perspective. I would rather suggest that the lombok project should include this patch into the list of other eclipse patches that get applied when it's configured for eclipse.

Anyway, if you feel that eclipse guys would accept it as a bug, please feel free to report it to eclipse.

Collaborator

lombokissues commented Jul 14, 2015

👤 Simeon.Zverinski   🕗 Jun 04, 2013 at 21:14 UTC

@ sholland: actually I don't think it's a bug in Eclipse. The assert could make sense from the standard eclipse rename workflow perspective. I would rather suggest that the lombok project should include this patch into the list of other eclipse patches that get applied when it's configured for eclipse.

Anyway, if you feel that eclipse guys would accept it as a bug, please feel free to report it to eclipse.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 cbeams   🕗 Nov 08, 2013 at 11:19 UTC

I just tried the basic field renaming workflow described here, and ran into the same "assertion failed" errors. This is against Spring Tool Suite 3.4.0.RELEASE + Lombok 1.12.2. Wondering if anyone has had luck solving this problem since the last comments in June?

Collaborator

lombokissues commented Jul 14, 2015

👤 cbeams   🕗 Nov 08, 2013 at 11:19 UTC

I just tried the basic field renaming workflow described here, and ran into the same "assertion failed" errors. This is against Spring Tool Suite 3.4.0.RELEASE + Lombok 1.12.2. Wondering if anyone has had luck solving this problem since the last comments in June?

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 r.spilker   🕗 Jun 11, 2014 at 09:33 UTC

Issue #726 has been merged into this issue.

Collaborator

lombokissues commented Jul 14, 2015

👤 r.spilker   🕗 Jun 11, 2014 at 09:33 UTC

Issue #726 has been merged into this issue.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 marton.dinnyes   🕗 Aug 21, 2014 at 12:20 UTC

I raised a bug with Eclipse, we'll see the end of it.
https://bugs.eclipse.org/bugs/show_bug.cgi?id=442070

Collaborator

lombokissues commented Jul 14, 2015

👤 marton.dinnyes   🕗 Aug 21, 2014 at 12:20 UTC

I raised a bug with Eclipse, we'll see the end of it.
https://bugs.eclipse.org/bugs/show_bug.cgi?id=442070

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 marton.dinnyes   🕗 Aug 22, 2014 at 15:05 UTC

Seems dead end on Eclipse side, but feel free to argue in comments there.

Collaborator

lombokissues commented Jul 14, 2015

👤 marton.dinnyes   🕗 Aug 22, 2014 at 15:05 UTC

Seems dead end on Eclipse side, but feel free to argue in comments there.

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

End of migration

Collaborator

lombokissues commented Jul 14, 2015

End of migration

@ractive

This comment has been minimized.

Show comment
Hide comment
@ractive

ractive Oct 28, 2015

Any news on this issue? I think it's a very common thing to rename a field which is now only possible with workarounds... 😞

ractive commented Oct 28, 2015

Any news on this issue? I think it's a very common thing to rename a field which is now only possible with workarounds... 😞

@rgra

This comment has been minimized.

Show comment
Hide comment
@rgra

rgra Nov 12, 2015

Contributor

I have a fix ready and will create a pull request after some clean-up. This will fix refactoring problems with @DaTa and @Getter/@Setter.

@rzwitserloot: did you try to add test cases in lombok for eclipse refactoring? would be good to test if it's broken again in later eclipse releases.

Contributor

rgra commented Nov 12, 2015

I have a fix ready and will create a pull request after some clean-up. This will fix refactoring problems with @DaTa and @Getter/@Setter.

@rzwitserloot: did you try to add test cases in lombok for eclipse refactoring? would be good to test if it's broken again in later eclipse releases.

@nicoschl

This comment has been minimized.

Show comment
Hide comment
@nicoschl

nicoschl Nov 27, 2016

Any news on when the pull request will be considered?

nicoschl commented Nov 27, 2016

Any news on when the pull request will be considered?

rspilker added a commit that referenced this issue Jan 19, 2017

Merge pull request #1060 from rgra/Issue210
Patch for renaming fields with Getter/Setter/Data in eclipse #210
@rzwitserloot

This comment has been minimized.

Show comment
Hide comment
@rzwitserloot

rzwitserloot Jan 19, 2017

Owner

Addressed in the next release due to a patch by @rgra: #1060

Owner

rzwitserloot commented Jan 19, 2017

Addressed in the next release due to a patch by @rgra: #1060

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment