eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
157 stars 125 forks source link

Remove obsoleted code in CompilerOptions #2924

Open iloveeclipse opened 1 week ago

iloveeclipse commented 1 week ago

Found during the review of #2551

Fixes https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2760

iloveeclipse commented 1 week ago

@srikanth-sankaran : you've pointed me to the code in question on original issue #2551, could you please check if that is what you've envisioned or more code could/should be removed?

iloveeclipse commented 1 week ago

Few tests are failing now, investigating:

Test Result (6 failures ) org.eclipse.jdt.core.tests.model.ResolveTests_1_5.test0119 org.eclipse.jdt.core.tests.model.ResolveTests_1_5.test0120 org.eclipse.jdt.core.tests.model.ResolveTests_1_5.test0121 org.eclipse.jdt.core.tests.model.CompletionTests_1_5.test0215 org.eclipse.jdt.core.tests.model.ExternalAnnotations18Test.testProjectDependencyReconcile2 org.eclipse.jdt.core.tests.model.ExternalAnnotations18Test.testProjectDependencyReconcile3

srikanth-sankaran commented 1 week ago

@srikanth-sankaran : you've pointed me to the code in question on original issue #2551, could you please check if that is what you've envisioned or more code could/should be removed?

The changes look reasonable - studying the test failures ...

srikanth-sankaran commented 1 week ago

With this additional fix, these 6 failures go away:

diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/parser/TypeConverter.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/parser/TypeConverter.java
index a8a918f..d5c302d 100644
--- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/parser/TypeConverter.java
+++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/parser/TypeConverter.java
@@ -427,32 +427,22 @@
                    identCount ++;
                    break;
                case '<' :
-                   /* We need to convert and preserve 1.5 specific constructs either if compliance is 1.5 or above,
-                      or the caller has explicitly requested generics to be included. The parameter includeGenericsAnyway
-                      should be used by the caller to signal that in the calling context generics information must be
-                      internalized even when the requesting project is 1.4. But in all cases, we must skip over them to
-                      see if there are any applicable type fragments after the type parameters: i.e we just aren't done
-                      having seen a '<' in 1.4 mode.

-                      Because of the way type signatures are encoded, TypeConverter.decodeType(String, int, int, int) is immune
-                      to this problem. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=325633
-                    */
-                   if (includeGenericsAnyway) {
-                       if (fragments == null) fragments = new ArrayList(2);
-                   }
+                   if (fragments == null) fragments = new ArrayList(2);
+
                    nameFragmentEnd = this.namePos-1;
-                   if (includeGenericsAnyway) {
-                       char[][] identifiers = CharOperation.splitOn('.', typeName, nameFragmentStart, this.namePos);
-                       fragments.add(identifiers);
-                   }
+
+                   char[][] identifiers = CharOperation.splitOn('.', typeName, nameFragmentStart, this.namePos);
+                   fragments.add(identifiers);
+
                    this.namePos++; // skip '<'
                    TypeReference[] arguments = decodeTypeArguments(typeName, length, start, end, includeGenericsAnyway); // positionned on '>' at end
-                   if (includeGenericsAnyway) {
-                       fragments.add(arguments);
-                       identCount = 0;
-                       nameFragmentStart = -1;
-                       nameFragmentEnd = -1;
-                   }
+
+                   fragments.add(arguments);
+                   identCount = 0;
+                   nameFragmentStart = -1;
+                   nameFragmentEnd = -1;
+
                    // next increment will skip '>'
                    break;
            }