Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 364539

Summary:

[generate constructor] Generate constructor using fields globally sets access modifier to private

Product:

[Eclipse Project] JDT

Reporter:

Rene Samselnig <rene.samselnig>

Component:

Assignee:

Markus Keller <markus.kell.r>

Status:

RESOLVED FIXED

QA Contact:

Severity:

minor

Priority:

CC:

amj87.iitr, daniel_megert, deepakazad, markus.kell.r, rene.samselnig, samrat.dhillon

Version:

3.8

Target Milestone:

4.3 M2

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Patch to modify SourceActionDialog	none
Patch to modify SourceActionDialog	none

Description Rene Samselnig

2011-11-23 02:52:34 EST

Build Identifier: 20110916-0149

When using 'Generate Constructor using fields...' on an enum with private field the access modifier is disabled and set to private (which is perfectly ok). Afterwards the access modifier is always set to private, even if you use 'Generate Constructor using fields...' on a class. Eclipse should not remember the private access modifier when it is given and disabled.

Reproducible: Always

Steps to Reproduce:
1. Create an Enum
2. Create a private field
3. Use 'Generate Constructor using fields...' to create the constructor
4. Create a class
5. Create a private field
6. Use 'Generate Constructor using fields...' to create the constructor -> the access modifier is now set to private regardless of the previous setting

Comment 1 Samrat Dhillon

2012-05-27 19:40:03 EDT

Created attachment 216340 [details]
Patch to modify SourceActionDialog

Proposing a patch. This patch attempts to not save the constructor visibility modifier in preferences when generating constructor for enum types.

Comment 2 Ayushman Jain

2012-05-28 01:00:30 EDT

Thanks for the patch Samrat. Moving to JDT/UI.

Comment 3 Markus Keller

2012-05-29 08:37:42 EDT

The patch needs some work:
- Code formatting
- e.printStackTrace() is a no-go. Use JavaPlugin.log(e) if really necessary
- Make sure this behaves right for all subclasses of SourceActionDialog. I'm pretty sure the fix is wrong for other source actions, e.g. "Generate Getters and Setters" on an enum type.

Comment 4 Samrat Dhillon

2012-06-04 20:02:14 EDT

Created attachment 216801 [details]
Patch to modify SourceActionDialog

Markus thanks for your comments. I changed the formatting and removed the printStackTrace. I did not realize that "SETTINGS_VISIBILITY_MODIFIER" was also being used to drive the visibility of other things in different dialogs.

Now the code explicitly checks if the dialog is an instanceof GenerateConstructorUsingFieldsSelectionDialog class. This may be a bit ugly, but I think it works for now. Another way could have been moving the close functionality to each of the individual dialog classes that extend SourceActionDialog so that each class can modify the behavior if it wants to. It will require a bit of refactoring.

Comment 5 Samrat Dhillon

2012-08-20 15:07:31 EDT

Just wanted to send out a gentle reminder to review this patch.

Comment 6 Markus Keller

2012-08-22 14:26:00 EDT

Superclasses should generally not refer to individual subclasses.

Fixed in a more generic fashion: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=3f6103cca10d4dcdc9f6da6c91e9226a94a02deb