Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 364539 - [generate constructor] Generate constructor using fields globally sets access modifier to private
Summary: [generate constructor] Generate constructor using fields globally sets access...
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.8   Edit
Hardware: All All
: P3 minor (vote)
Target Milestone: 4.3 M2   Edit
Assignee: Markus Keller CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-23 02:52 EST by Rene Samselnig CLA
Modified: 2012-08-22 14:26 EDT (History)
6 users (show)

See Also:

Attachments
Patch to modify SourceActionDialog (1.59 KB, patch)
2012-05-27 19:40 EDT, Samrat Dhillon CLA 		no flags Details | Diff
Patch to modify SourceActionDialog (1.59 KB, patch)
2012-06-04 20:02 EDT, Samrat Dhillon CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rene Samselnig CLA 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 CLA 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 CLA 2012-05-28 01:00:30 EDT 
Thanks for the patch Samrat. Moving to JDT/UI.
Comment 3 Markus Keller CLA 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 CLA 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 CLA 2012-08-20 15:07:31 EDT 
Just wanted to send out a gentle reminder to review this patch.
Comment 6 Markus Keller CLA 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