Community
Participate
Working Groups
Created attachment 123753 [details] example of .ecore model used to generate java code producing errors - Create .ecore model - Create new EClass with the name "Container" - generate model code -> following compilation problems reported by java compiler: 1. The return type is incompatible with MainPackageFactory.createContainer() 2. Type mismatch: cannot convert from ContainerImpl to MinimalEObjectImpl.Container
Hmmm. That's nasty. :-( It's a reminder that all the things in the scope of the base class will be in scope for the derived classes. So it seems I've picked quite a bad name. :-( Probably I'll have to rename it to up an E in front and deprecate the one that's just created (which I'll then delete before the release). Unless I get a more brilliant idea. E.g., one could specify a list of nested names that are in scope in the Root Extends Class.... Gross.
Gross, indeed. I definitely don't think that's a good idea. I think a slightly less gross idea would be to just hardcode the nested names for the classes we know about in GenBaseGeneratorAdapter.createImportManager(). Actually, that doesn't seem particularly worse than just using a less useful name. Both approaches seem fine. Neither does anything to help people using their own custom base class.
Created attachment 125549 [details] Patches to address the issue. Dave, This approach isn't too gross...
We faced the same problem and the patch works fine. Is someone going to commit it soon?
Sorry, I was too fast. The patch is not working when applied against the HEAD, because the generated code references the AccessListener, which is not there.
Yes, unfortunately I'm mixing up my patches and unfortunately time seems to disappear down a rat hole these days. Hopefully Dave will have a chance to review it soon.
Ed, I'm not sure I get it. The patch seems to disallow an un-imported Container or Dynamic completely in the package and factory classes, regardless of the root extends class, while only selectively disallowing them in class interfaces and implementations. So people who aren't even using MinimalEObjectImpl could very well be affected when they regen. I thought we were going to try to minimize the disruption, so shouldn't we take the same care with packages and factories? And, why would we need to worry about interfaces for classes at all? Also, I don't get why we'd use the generated artifact name in the import, rather than the real, full class names (org.eclipse.emf.ecore.impl.MinimalEObjectImpl.Container and org.eclipse.emf.ecore.impl.MinimalEObjectImpl.Container.Dynamic). If ever a template were to refer to one of those MinimalEObjectImpl inner classes, the import manager won't behave properly unless the correct class name has been registered. I realize that's unlikely and we're mostly doing this to reserve the short name, but I don't see any reason to misuse the import manager. Finally, wasn't part of the motivation behind doing this to add pseudo imports for the names of interfaces/classes imported/extended, so that the nested feature type thing would work? I don't see that being done.
Created attachment 126426 [details] Slight improvements to the patch Ed pointed out the obvious to me: that generated factories and packages *always* extend MinimalEObjectImpl.Container (via EFactoryImpl and EPackageImpl), not the root extends class for the model. My points about these pseudo imports not being needed for interfaces and about using the real class names are valid, and I've fixed those in the attached patch. I'm now convinced that we don't need to do anything for the names of interfaces/classes imported/extended. I think that other issue will work just fine as things are.
Oh, I meant to add that I've renamed in the new method to addClassPseudoImports(), since it's conceivable that we might one day need a similar method for some other class-level artifact. Does this look good, Ed?
Dave, It looks good. You'll commit it?
Sure.
The fix is in CVS for EMF 2.5.
Fix available in HEAD: 2.5.0.I200902241800.
*** Bug 279074 has been marked as a duplicate of this bug. ***
Fix available in HEAD: 2.5.0 (R200906151043).
*** Bug 288842 has been marked as a duplicate of this bug. ***