Bug 147516 - [refactoring] warn when refactoring derived files
Summary: [refactoring] warn when refactoring derived files
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.3 M4   Edit
Assignee: Markus Keller CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 111447 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-06-16 15:52 EDT by Jess Garms CLA
Modified: 2006-11-14 03:04 EST (History)
9 users (show)

See Also:


Attachments
Warn when editing derived files (72.41 KB, patch)
2006-11-14 03:03 EST, Markus Keller CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jess Garms CLA 2006-06-16 15:52:22 EDT
In JDT-APT, we create java files during build. If a refactoring occurs later, JDT-UI suggests refactoring those generated files. We would like a way to indicate that those files are generated, so that the user does not have to 
refactor them. When their parent java files are rebuilt post-refactoring, the newly-generated files will have the correct references without ever refactoring the files themselves.
Comment 1 Martin Aeschlimann CLA 2006-06-19 08:52:55 EDT
Micheal, wouldn't operation validation help here? I couldn't find the corresponding API, what's its name?
Comment 2 Michael Valenta CLA 2006-06-19 11:11:19 EDT
I don't think that operation validation will help. The API is ModelProvider#validateChange() but it will only indicate whether there may be undesirable side-effects. The API doesn't provide any means to configure the semantics of the operation (i.e. the user either cancels the operation or decides to go ahead and suffer the consequences if there are any).

What this bug is asking is whether derived Java files can be excluded from a refactoring. My first question is why is this required? Wouldn't the files just be refactored and then replaced by the code generator? Is the problem that the code generator uses timestamps to determine if an overwrite is OK to perform? Or is it just the case that you want to avoid the extra work?

Having said that, the user will want to see the effects of a refactoring at the level they wrote the code. If they used annotations to create some stub classes, they would not want to see those classes in the refactoring but would want to see changes in the class containing the annotations. Perhaps all that is needed is a means to allow higher level models to hide certain files form the user in the refactoring preview.

Comment 3 Jess Garms CLA 2006-06-19 12:32:19 EDT
The files that we generate are re-created on a build of their parent file. So if the user edits their generated files, their edits are wiped out upon the next full build. In order to solve this problem, we originally attempted to make the generated files read-only. This worked well, except that when a refactoring was performed, Eclipse would issue errors that some of the files were read-only.

> Perhaps all that is needed is a means to allow 
> higher level models to hide certain files form
> the user in the refactoring preview.

That certainly would help -- it hides some of the complexity from the user. It would be nice to once again be able to make our generated files read-only as well though. This would require some way to indicate to the refactoring engine that the generated files can either be marked writeable before refactoring, or should be ignored.
Comment 4 Michael Valenta CLA 2006-06-19 12:44:17 EDT
I don't think that making the files read-only is the right answer. By that argument, we should make all the *.class files read-only as well. A better appraoch would be to have some way of identifying to the Java toolng that the file is generated. One way would be to mark is as derived. Another would be to come up with a JDT specific way of indicating that a Java file is generated. This may be preferable if you need to destinguish files that are totally generated vs. those that are partially generated (like EMF).
Comment 5 Jess Garms CLA 2006-06-19 12:49:04 EDT
Yes, marking the files read-only is definitely a hack to get around the fact that eclipse has no way to indicate to the user that the .java file they're editing was generated and any edits they make will be lost. We do already mark the files as derived, but that doesn't prevent the user from mistakenly editing a file, thinking that it's a normal source file.

It's different from the generated .class file case in that there is no native .class file editor that one can get to by control-clicking on user source. With generated source, however, this can easily happen.
Comment 6 Michael Valenta CLA 2006-06-19 13:01:25 EDT
The problem with using the read-only bit is that a reposiory provider will just make it writtable when the user tries to edit the file (i.e. in Eclipse the read-only bit is used by the repository provider to indicate that a file requires a validateEdit before it can be edited). The user may never know that it was marked read-only.

I think what is required here is a JDT level notion of generated source. Then, the Java editor could issue the necessary warnings when a user edits a file and the refactoring could hide the changes from the user.
Comment 7 Martin Aeschlimann CLA 2006-06-20 12:10:13 EDT
I also see no harm when refactorings make changes in generated code and the files are regenerated again.
But I see that we should signal the user that a file is generated in the editor or code generation wizards so the user doesn't start working in generated code.

That's why I thought that the warning that the operation validation can give out is enough: Give a warning as specific as possible and let the user cancel the operation if wanted. Currently operation validtion is in JDT only used by the refactorings, more work is needed here in the JDT tooling and editor if we go that way.
Validate edit would also work, but I think at the moment only one provider can registered per project (and the problems with clients that don't pass a display context)

The advantage of this is that the list of generated files needs only to be known by the generator. If we want the platform to know the concepts of generated files, we first need a way to flag generated files:

ideas:
- extra build path attribute (Java solution only)
- using the 'derived' flag (could be problematic as this might conflict with other usages of derived)
- speical marker on resources
Comment 8 Dani Megert CLA 2006-06-20 12:30:51 EDT
Note that validateEdit is only called when the file is read-only. Using operation validation when starting to type into the editor isn't planned. What we could do is warn if a user types into an editor that works on a derived file.
Comment 9 Michael Valenta CLA 2006-06-26 09:55:47 EDT
I like Dani's suggestion. Both the editor and the refactoring operation could issue a warning that a file that is abut to be modified is marked as derived. Ths should be enough of a warning to the user that whatever they do to that file will be overwritten.
Comment 10 Martin Aeschlimann CLA 2006-06-27 12:38:36 EDT
So you think we should use the 'derived' flag to mark generated source?
I'm for that too, but we always hesitated as other products might use the derived flag as well for slightly different purposes.

Adding John for an opinion from core.resources.
Comment 11 John Arthorne CLA 2006-06-27 15:09:35 EDT
It sounds like the right direction - the derived flag is certainly intended as a way to indicate that a file was system generated and should not be edited manually. I think it should be taken as a hint rather than an absolute rule - if the user really wants to go ahead and edit/refactor a derived resource, they shouldn't be prevented from doing it.  Perhaps there would also be a global "remember my decision" to avoid overloading with user with prompts.
Comment 12 Martin Aeschlimann CLA 2006-06-28 02:23:05 EDT
Ok, nthen I think we should do this. I spawned bug 148931 for the editor, keeping this bug for refactings and code manipulations.

What is a bit unlucky is that this adds another thing that need to be done before changing a file and that will be forgotten by most other plugin contributors.
We habe now validate edit, derived warning and operation validation. 
Comment 13 Dani Megert CLA 2006-06-29 02:31:02 EDT
Please see bug 148931 comment 4. We should continue the discussion in this bug since it contains more details and suggestions already.
Comment 14 Markus Keller CLA 2006-08-14 08:07:42 EDT
Warning the user when he starts editing a derived file would be fine with me.

However, I'm not sure it's a good idea to warn users when a refactoring applies changes to a derived file. I see 3 ways for refactorings to deal with derived files:

a) warn when touched:
   - Users of APT will get unnecessary warnings when generated files contain references to the refactored element.

b) hide from preview and don't warn:
   + Users don't have to see/care about changes in generated files
   - Users can't see all resource changes in preview any more (could be alleviated with a filter for derived resources that is enabled by default)

c) don't refactor APT-generated files
   -> would need a special mechanism for APT files (e.g. bug 111447), since refactorings can not just skip all derived files (see bug 111447 comment 1).
Comment 15 Michael Valenta CLA 2006-08-14 09:35:35 EDT
Option b seems reasonable. However, it wouldn't hurt to have a "don't show me this again" warning as well just to let the user know that derived files are included in the refactoring.
Comment 16 Kim Horne CLA 2006-08-14 10:35:20 EDT
*** Bug 153566 has been marked as a duplicate of this bug. ***
Comment 17 Martin Aeschlimann CLA 2006-08-16 07:14:18 EDT
Add David as cc to also get an opinion for WTP regarding the usage of derived files. Would it be ok to warn in the editor when a derived file is edited (bug 148931)?
Comment 18 Martin Aeschlimann CLA 2006-08-22 05:54:01 EDT
*** Bug 111447 has been marked as a duplicate of this bug. ***
Comment 19 Markus Keller CLA 2006-10-04 11:35:05 EDT
I've enhanced the refactoring preview wizard page to show derived files.
To come:
- warning when refactoring a declaration in a derived file (but stay silent when refactoring just references in a derived file)
- filter in preview page to hide changes in derived files
Comment 20 Markus Keller CLA 2006-10-30 11:34:12 EST
Done for 3.3M3:
 - filter in preview page to hide changes in derived files
Comment 21 Markus Keller CLA 2006-11-14 03:03:21 EST
Created attachment 53796 [details]
Warn when editing derived files
Comment 22 Markus Keller CLA 2006-11-14 03:04:03 EST
Fixed in HEAD.