Use Java Generics where possible

[azoitl]

GEF Classic's dates back to a time without generics. While this is in general nothing bad it makes maintaining GEF Classic hard. Also clients and GEF Classic have to perform more instanceof checks and cast then necessary. Therefore GEF Classic is also hard to use.

However just type all generics used is also not the solution as it may break client code. See for example the discussion in PR #81.

In the other side there are partly hidden issues in GEF Classic that a cleanup can reveal (e.g., Issue #154, or the planned upcoming PR azoitl@69194fb).

As now several people are interested in improving the GEF Classic code base in this respect this issue is here to coordinate any effort. But also discuss how to best test and evaluate all proposed changes to make the transition for our clients as smooth as possible.

Furthermore the starting point for this has already been done. There are already several PRs merged for the 3.15 release but there is still some way ahead of us.

For starting this I've created a first PR #149 and based on that there are several commits in the pipeline that can be moved to PRs as soon the first one is merged:
- azoitl@7d1ad00
- azoitl@588d877
- azoitl@19c84c8
- azoitl@69194fb

First set of Validation and Check rules
In order to ensure a smooth transition for our users I would suggest at least the following tests and rules to be performed:

• PRs should be as small as possible each addressing one distinct feature
• It should pass our unit tests
• Our examples (e.g., logic editor) do behave as expected (e.g., for PR#149 I found an issue by doing this step)
• Compile with additional clients (e.g., I do at least Eclipse GMF runtime and 4diac IDE). The more the merrier.

[Destrolaric]

@azoitl We discussed this with our team. We can accept the #149. And we are planning to split other parts of our Destrolaric#2 into several smaller prs. Is it acceptable?

[vogella]

And we are planning to split other parts of our Destrolaric#2 into several smaller prs. Is it acceptable?

Yes

[azoitl]

@Destrolaric first of all thx a lot for your ongoing support and contributions. I'm very grateful for these. I'm also sorry that I'm causing here extra work and a burden. So also thx for that. I was thinking it would be good idea to meet in some online meeting so we can better sync on our interests. What do you think?

[Destrolaric]

@azoitl Thanks for the suggestion. It looks like a good idea. I think we may have the meeting next week, is that ok?

[azoitl]

@azoitl Thanks for the suggestion. It looks like a good idea. I think we may have the meeting next week, is that ok?

Next week sounds good. May I send you a few suggestions via email?

[Destrolaric]

@azoitl Yes, of course.

[nitind]

gef-classic/org.eclipse.draw2d/src/org/eclipse/draw2d/IFigure.java

Line 295 in 49d0515

List<? extends IFigure> getChildren();

returning a list of <? extends IFigure> instead of IFigure breaks our builds (https://ci.eclipse.org/webtools/view/webtools_CI/job/webtools-jsf_master/2353/console):

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:2.7.5:compile (default-compile) on project org.eclipse.jst.jsf.facesconfig.ui: Compilation failure: Compilation failure:
[ERROR] /home/jenkins/agent/workspace/webtools-jsf_master/jsf/plugins/org.eclipse.jst.jsf.facesconfig.ui/src/org/eclipse/jst/jsf/facesconfig/ui/preference/BaseNodeFigure.java:[824]
[ERROR] parent.getChildren().add(child);
[ERROR] ^^^
[ERROR] The method add(capture#5-of ? extends IFigure) in the type List<capture#5-of ? extends IFigure> is not applicable for the arguments (IFigure)
[ERROR] /home/jenkins/agent/workspace/webtools-jsf_master/jsf/plugins/org.eclipse.jst.jsf.facesconfig.ui/src/org/eclipse/jst/jsf/facesconfig/ui/preference/BaseNodeFigure.java:[839]
[ERROR] parent.getChildren().add(child);
[ERROR] ^^^
[ERROR] The method add(capture#7-of ? extends IFigure) in the type List<capture#7-of ? extends IFigure> is not applicable for the arguments (IFigure)
[ERROR] 2 problems (2 errors)
[ERROR] -> [Help 1]

[azoitl]

@nitind sorry that this breaks your build. I just pushed a fix with a small clean-up to webtools-jsf that fixes the issue. The reason we use the <? extends IFigure> version is documented in the discussion of PR #81.

But looking at your code I noticed that that what you like to do should be in the end offered by the IFigure interface, namely reordering a child. Because adding children by directly manipulating the children list is somewhat dangerous and may break the figure. What do you think about adding a reorderChild method to IFigure?

[nitind]

I don't think it's needed, given the various overloaded add() methods that already exist. I've pushed a small change atop yours in consideration of this. The more complicated scenario was the Dali build, with this error:

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:2.7.5:compile (default-compile) on project org.eclipse.jpt.jpa.ui: Compilation failure: Compilation failure:
[ERROR] /home/jenkins/agent/workspace/webtools-dali_master/jpa/plugins/org.eclipse.jpt.jpa.ui/src/org/eclipse/jpt/jpa/ui/internal/wizards/gen/AssociationsListComposite.java:[90]
[ERROR] List associationFigures = figure.getChildren();
[ERROR] ^^^^^^^^^^^^^^^^^^^^
[ERROR] Type mismatch: cannot convert from List<capture#1-of ? extends IFigure> to List
[ERROR] /home/jenkins/agent/workspace/webtools-dali_master/jpa/plugins/org.eclipse.jpt.jpa.ui/src/org/eclipse/jpt/jpa/ui/internal/wizards/gen/AssociationsListComposite.java:[105]
[ERROR] List associationFigures = figure.getChildren();
[ERROR] ^^^^^^^^^^^^^^^^^^^^
[ERROR] Type mismatch: cannot convert from List<capture#2-of ? extends IFigure> to List
[ERROR] 2 problems (2 errors)
[ERROR] -> [Help 1]

[azoitl]

Thx for the feedback. Damn I really hoped that my changes would break less. Have to be more considerate for the upcoming changes.
\cc @Destrolaric

[github-actions]

This issue is stale because it has been open for 90 days with no activity.

[github-actions]

This issue is stale because it has been open for 90 days with no activity.

[github-actions]

This issue is stale because it has been open for 90 days with no activity.

[github-actions]

This issue is stale because it has been open for 90 days with no activity.