Skip to content

Should we restrict the usage of generics in our codebase? #2611

@adangel

Description

@adangel

I have repeatedly problems with eclipse, which can't currently build pmd/7.0.x branch.
There are three main issues:

  • eclipse doesn't resolve inner interfaces names in super base classes... (BaseAntlrInnerNode and others)
  • capturing doesn't work in all cases (these are the cases in IteratorBasedNStream)
  • the refinement of Node.children() in GenericNode doesn't work in eclipse. Eclipse resolves the signature of Node rather than of GenericNode... which means, children().toList() returns List<? extends Node> rather than List<? extends JavaNode> e.g....

So, I asking the question: Should we restrict the usage of generics in our codebase?
Since not even the compilers understand the code, I don't expect anybody else than @oowekyala really understand, what's going on... which essentially renders the project unmaintainable by anybody else...

Wdyt? @oowekyala @jsotuyod

For reference, current patch needed (incomplete):

diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/BaseAntlrInnerNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/BaseAntlrInnerNode.java
index 4daab3ed6e..5833547776 100644
--- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/BaseAntlrInnerNode.java
+++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/BaseAntlrInnerNode.java
@@ -102,7 +102,7 @@ public abstract class BaseAntlrInnerNode<N extends AntlrNode<N>> extends BaseAnt
         // default does nothing
     }
 
-    protected static class PmdAsAntlrInnerNode<N extends AntlrNode<N>> extends ParserRuleContext implements RuleNode, AntlrToPmdParseTreeAdapter<N> {
+    protected static class PmdAsAntlrInnerNode<N extends AntlrNode<N>> extends ParserRuleContext implements RuleNode, BaseAntlrNode.AntlrToPmdParseTreeAdapter<N> {
 
         private final BaseAntlrInnerNode<N> pmdNode;
 
diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/BaseAntlrTerminalNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/BaseAntlrTerminalNode.java
index 8b265ff528..3f7b574edb 100644
--- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/BaseAntlrTerminalNode.java
+++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/BaseAntlrTerminalNode.java
@@ -71,7 +71,7 @@ public abstract class BaseAntlrTerminalNode<N extends AntlrNode<N>>
         throw new IndexOutOfBoundsException("Index " + index + " for terminal node");
     }
 
-    protected static class AntlrTerminalPmdAdapter<N extends AntlrNode<N>> extends TerminalNodeImpl implements AntlrToPmdParseTreeAdapter<N> {
+    protected static class AntlrTerminalPmdAdapter<N extends AntlrNode<N>> extends TerminalNodeImpl implements BaseAntlrNode.AntlrToPmdParseTreeAdapter<N> {
 
         private final BaseAntlrTerminalNode<N> pmdNode;
 
diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/internal/IteratorBasedNStream.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/internal/IteratorBasedNStream.java
index afbf7d3a35..75d475da4d 100644
--- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/internal/IteratorBasedNStream.java
+++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/internal/IteratorBasedNStream.java
@@ -271,8 +271,12 @@ abstract class IteratorBasedNStream<T extends Node> implements NodeStream<T> {
 
         @Override
         public Iterator<S> iterator() {
-            return IteratorUtil.flatMap(upstream.iterator(),
-                                        fun.andThen(walker::apply).andThen(NodeStream::iterator));
+            @SuppressWarnings("unchecked")
+            Function<T, DescendantNodeStream<S>> fun2 =
+                    ((Function<T, DescendantNodeStream<S>>) fun) // casting helps eclipse compiler...
+                    .andThen(walker::apply);
+            Function<T, Iterator<@NonNull S>> it = fun2.andThen(NodeStream::iterator);
+            return IteratorUtil.flatMap(upstream.iterator(), it);
         }
 
         @Override
diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/impl/AbstractNodeTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/impl/AbstractNodeTest.java
index e9b0f3b513..ffacba41eb 100644
--- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/impl/AbstractNodeTest.java
+++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/impl/AbstractNodeTest.java
@@ -124,7 +124,8 @@ public class AbstractNodeTest {
     @Test
     public void testRemoveRootNode() {
         // Check that the root node has the expected properties
-        final List<? extends DummyNode> children = rootNode.children().toList();
+        @SuppressWarnings("unchecked") // cast is needed to support eclipse compiler
+        final List<? extends DummyNode> children = (List<? extends DummyNode>) rootNode.children().toList();
 
         // Do the actual removal
         rootNode.remove();
@@ -163,7 +164,8 @@ public class AbstractNodeTest {
     @Test
     @Parameters(method = "childrenIndexes")
     public void testRemoveRootNodeChildAtIndex(final int childIndex) {
-        final List<? extends DummyNode> originalChildren = rootNode.children().toList();
+        @SuppressWarnings("unchecked") // cast is needed to support eclipse compiler
+        final List<? extends DummyNode> originalChildren = (List<? extends DummyNode>) rootNode.children().toList();
 
         // Do the actual removal
         rootNode.removeChildAtIndex(childIndex);

Metadata

Metadata

Assignees

No one assigned

    Labels

    a:questionRequest for information that doesn't necessarily entail changes to the code basea:suggestionAn idea, with little analysis on feasibility, to be consideredwas:wontfix

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions