Fix FPs around enumerators in UnusedRoutine and UnusedProperty

Author: cirras

CHANGELOG.md

@@ -7,13 +7,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- **API:** `EnumeratorOccurrence` type.
+- **API:** `EnumeratorOccurrence::getGetEnumerator` method.
+- **API:** `EnumeratorOccurrence::getMoveNext` method.
+- **API:** `EnumeratorOccurrence::getCurrent` method.
+- **API:** `ForInStatementNode.getEnumeratorOccurrence` method.
+
 ### Changed
 
 - Detect tab-indented multiline strings in `TabulationCharacter`.
 - Improve support for evaluating name references in compiler directive expressions.
 
+### Deprecated
+
+- **API:** `ForInStatementNode::getGetEnumeratorDeclaration` method, get the declaration through
+  `getEnumeratorOccurrence` instead.
+- **API:** `ForInStatementNode::getMoveNextDeclaration` method, get the declaration through
+  `getEnumeratorOccurrence` instead.
+- **API:** `ForInStatementNode::getCurrentDeclaration` method, get the declaration through
+  `getEnumeratorOccurrence` instead.
+
 ### Fixed
 
+- False positives on enumerable method `GetEnumerator` in `UnusedRoutine`.
+- False positives on enumerator method `MoveNext` in `UnusedRoutine`.
+- False positives on enumerator property `Current` in `UnusedProperty`.
 - Name resolution failures in legacy initialization sections referencing the implementation section.
 - Incorrect file position calculation for multiline string tokens.
 

delphi-checks/src/test/java/au/com/integradev/delphi/checks/UnusedPropertyCheckTest.java

@@ -265,4 +265,32 @@ void testUnusedImplementationPropertyWithExcludeApiShouldAddIssue() {
                 .appendImpl("end;"))
         .verifyIssues();
   }
+
+  @Test
+  void testUsedEnumeratorPropertiesShouldNotAddIssue() {
+    CheckVerifier.newVerifier()
+        .withCheck(new UnusedPropertyCheck())
+        .onFile(
+            new DelphiTestUnitBuilder()
+                .appendDecl("type")
+                .appendDecl("  TFooEnumerator = record")
+                .appendDecl("    function MoveNext: Boolean;")
+                .appendDecl("    property Current: string read FCurrent;")
+                .appendDecl("  end;")
+                .appendDecl("  TFoo = record")
+                .appendDecl("  private")
+                .appendDecl("    FCurrent: string;")
+                .appendDecl("  public")
+                .appendDecl("    function GetEnumerator: TFooEnumerator;")
+                .appendDecl("  end;")
+                .appendImpl("procedure Bar(Foo: TFoo);")
+                .appendImpl("var")
+                .appendImpl("  Baz: string;")
+                .appendImpl("begin")
+                .appendImpl("  for Baz in Foo do begin")
+                .appendImpl("    WriteLn(Baz);")
+                .appendImpl("  end;")
+                .appendImpl("end;"))
+        .verifyNoIssues();
+  }
 }

delphi-checks/src/test/java/au/com/integradev/delphi/checks/UnusedRoutineCheckTest.java

@@ -409,4 +409,31 @@ void testUnusedImplementationRoutineWithExcludeApiShouldAddIssue() {
                 .appendImpl("end;"))
         .verifyIssues();
   }
+
+  @Test
+  void testUsedEnumeratorMethodsShouldNotAddIssue() {
+    CheckVerifier.newVerifier()
+        .withCheck(new UnusedRoutineCheck())
+        .onFile(
+            new DelphiTestUnitBuilder()
+                .appendDecl("type")
+                .appendDecl("  TFooEnumerator = record")
+                .appendDecl("    function MoveNext: Boolean;")
+                .appendDecl("    property Current: string read FCurrent;")
+                .appendDecl("  end;")
+                .appendDecl("  TFoo = record")
+                .appendDecl("  private")
+                .appendDecl("    FCurrent: string;")
+                .appendDecl("  public")
+                .appendDecl("    function GetEnumerator: TFooEnumerator;")
+                .appendDecl("  end;")
+                .appendImpl("var")
+                .appendImpl("  Foo: TFoo;")
+                .appendImpl("  Bar: string;")
+                .appendImpl("begin")
+                .appendImpl("  for Bar in Foo do begin")
+                .appendImpl("    WriteLn(Bar);")
+                .appendImpl("  end;"))
+        .verifyNoIssues();
+  }
 }

delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/ForInStatementNodeImpl.java

@@ -18,47 +18,19 @@
  */
 package au.com.integradev.delphi.antlr.ast.node;
 
-import static java.util.Collections.emptyList;
-
 import au.com.integradev.delphi.antlr.ast.visitors.DelphiParserVisitor;
-import au.com.integradev.delphi.symbol.resolve.NameResolutionHelper;
-import com.google.common.base.Suppliers;
-import java.util.function.Supplier;
 import javax.annotation.Nullable;
 import org.antlr.runtime.Token;
 import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
 import org.sonar.plugins.communitydelphi.api.ast.ForInStatementNode;
 import org.sonar.plugins.communitydelphi.api.ast.StatementNode;
+import org.sonar.plugins.communitydelphi.api.symbol.EnumeratorOccurrence;
 import org.sonar.plugins.communitydelphi.api.symbol.declaration.PropertyNameDeclaration;
 import org.sonar.plugins.communitydelphi.api.symbol.declaration.RoutineNameDeclaration;
-import org.sonar.plugins.communitydelphi.api.type.Type;
-import org.sonar.plugins.communitydelphi.api.type.TypeFactory;
 
 public final class ForInStatementNodeImpl extends ForStatementNodeImpl
     implements ForInStatementNode {
-  private final Supplier<NameResolutionHelper> nameResolutionHelper =
-      Suppliers.memoize(() -> new NameResolutionHelper(getAst().getDelphiFile().getTypeFactory()));
-
-  private final Supplier<RoutineNameDeclaration> getEnumeratorDeclaration =
-      Suppliers.memoize(
-          () ->
-              nameResolutionHelper
-                  .get()
-                  .findMethodMember(this, getEnumerable().getType(), "GetEnumerator", emptyList()));
-
-  private final Supplier<RoutineNameDeclaration> moveNextDeclaration =
-      Suppliers.memoize(
-          () ->
-              nameResolutionHelper
-                  .get()
-                  .findMethodMember(this, getEnumeratorType(), "MoveNext", emptyList()));
-
-  private final Supplier<PropertyNameDeclaration> currentDeclaration =
-      Suppliers.memoize(
-          () ->
-              nameResolutionHelper
-                  .get()
-                  .findPropertyMember(this, getEnumeratorType(), "Current", emptyList()));
+  private EnumeratorOccurrence enumeratorOccurrence;
 
   public ForInStatementNodeImpl(Token token) {
     super(token);
@@ -69,30 +41,35 @@ public <T> T accept(DelphiParserVisitor<T> visitor, T data) {
     return visitor.visit(this, data);
   }
 
+  @SuppressWarnings("removal")
   @Override
   @Nullable
   public RoutineNameDeclaration getGetEnumeratorDeclaration() {
-    return getEnumeratorDeclaration.get();
+    if (getEnumeratorOccurrence() == null) {
+      return null;
+    }
+    return (RoutineNameDeclaration)
+        getEnumeratorOccurrence().getGetEnumerator().getNameDeclaration();
   }
 
+  @SuppressWarnings("removal")
   @Override
   @Nullable
   public RoutineNameDeclaration getMoveNextDeclaration() {
-    return moveNextDeclaration.get();
+    if (getEnumeratorOccurrence() == null) {
+      return null;
+    }
+    return (RoutineNameDeclaration) getEnumeratorOccurrence().getMoveNext().getNameDeclaration();
   }
 
+  @SuppressWarnings("removal")
   @Override
   @Nullable
   public PropertyNameDeclaration getCurrentDeclaration() {
-    return currentDeclaration.get();
-  }
-
-  private Type getEnumeratorType() {
-    RoutineNameDeclaration enumerator = getGetEnumeratorDeclaration();
-    if (enumerator != null) {
-      return enumerator.getReturnType();
+    if (getEnumeratorOccurrence() == null) {
+      return null;
     }
-    return TypeFactory.unknownType();
+    return (PropertyNameDeclaration) getEnumeratorOccurrence().getCurrent().getNameDeclaration();
   }
 
   @Override
@@ -104,4 +81,14 @@ public ExpressionNode getEnumerable() {
   public StatementNode getStatement() {
     return (StatementNode) getChild(4);
   }
+
+  @Override
+  @Nullable
+  public EnumeratorOccurrence getEnumeratorOccurrence() {
+    return enumeratorOccurrence;
+  }
+
+  public void setEnumeratorOccurrence(EnumeratorOccurrence enumeratorOccurrence) {
+    this.enumeratorOccurrence = enumeratorOccurrence;
+  }
 }

delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/DependencyAnalysisVisitor.java

@@ -35,6 +35,7 @@
 import org.sonar.plugins.communitydelphi.api.ast.RoutineImplementationNode;
 import org.sonar.plugins.communitydelphi.api.ast.TypeDeclarationNode;
 import org.sonar.plugins.communitydelphi.api.ast.utils.ExpressionNodeUtils;
+import org.sonar.plugins.communitydelphi.api.symbol.EnumeratorOccurrence;
 import org.sonar.plugins.communitydelphi.api.symbol.NameOccurrence;
 import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration;
 import org.sonar.plugins.communitydelphi.api.symbol.declaration.PropertyNameDeclaration;
@@ -184,17 +185,22 @@ public Data visit(ArrayAccessorNode accessorNode, Data data) {
 
   @Override
   public Data visit(ForInStatementNode forInStatementNode, Data data) {
-    RoutineNameDeclaration enumerator = forInStatementNode.getGetEnumeratorDeclaration();
-    addDependenciesForDeclaration(enumerator, data);
-    handleInlineRoutines(enumerator, data);
+    EnumeratorOccurrence enumerator = forInStatementNode.getEnumeratorOccurrence();
 
-    RoutineNameDeclaration moveNext = forInStatementNode.getMoveNextDeclaration();
-    addDependenciesForDeclaration(moveNext, data);
-    handleInlineRoutines(moveNext, data);
+    if (enumerator != null) {
+      var getEnumerator =
+          (RoutineNameDeclaration) enumerator.getGetEnumerator().getNameDeclaration();
+      addDependenciesForDeclaration(getEnumerator, data);
+      handleInlineRoutines(getEnumerator, data);
 
-    PropertyNameDeclaration current = forInStatementNode.getCurrentDeclaration();
-    addDependenciesForDeclaration(current, data);
-    handleInlineRoutines(current, data);
+      var moveNext = (RoutineNameDeclaration) enumerator.getMoveNext().getNameDeclaration();
+      addDependenciesForDeclaration(moveNext, data);
+      handleInlineRoutines(moveNext, data);
+
+      var current = (PropertyNameDeclaration) enumerator.getCurrent().getNameDeclaration();
+      addDependenciesForDeclaration(current, data);
+      handleInlineRoutines(current, data);
+    }
 
     return DelphiParserVisitor.super.visit(forInStatementNode, data);
   }

delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/SymbolAssociationVisitor.java

@@ -27,6 +27,7 @@
 import org.sonar.plugins.communitydelphi.api.ast.ArrayAccessorNode;
 import org.sonar.plugins.communitydelphi.api.ast.DelphiAst;
 import org.sonar.plugins.communitydelphi.api.ast.DelphiNode;
+import org.sonar.plugins.communitydelphi.api.ast.ForInStatementNode;
 import org.sonar.plugins.communitydelphi.api.ast.NameDeclarationNode;
 import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode;
 import org.sonar.plugins.communitydelphi.api.ast.RoutineNameNode;
@@ -98,4 +99,10 @@ public Data visit(RoutineNameNode node, Data data) {
     data.fileScope.attach(node);
     return DelphiParserVisitor.super.visit(node, data);
   }
+
+  @Override
+  public Data visit(ForInStatementNode node, Data data) {
+    data.fileScope.attach(node);
+    return DelphiParserVisitor.super.visit(node, data);
+  }
 }

delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/visitors/SymbolTableVisitor.java

@@ -620,6 +620,8 @@ public Data visit(ForInStatementNode node, Data data) {
     node.getEnumerable().accept(this, data);
     node.getVariable().accept(this, data);
 
+    data.nameResolutionHelper.resolve(node);
+
     return visitScopeFromRoot(node.getStatement(), data);
   }
 

delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/declaration/VariableNameDeclarationImpl.java

@@ -36,6 +36,7 @@
 import org.sonar.plugins.communitydelphi.api.ast.VarDeclarationNode;
 import org.sonar.plugins.communitydelphi.api.ast.VarStatementNode;
 import org.sonar.plugins.communitydelphi.api.ast.Visibility;
+import org.sonar.plugins.communitydelphi.api.symbol.EnumeratorOccurrence;
 import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration;
 import org.sonar.plugins.communitydelphi.api.symbol.declaration.VariableNameDeclaration;
 import org.sonar.plugins.communitydelphi.api.symbol.scope.DelphiScope;
@@ -148,7 +149,12 @@ private static Type loopVarType(NameDeclarationNode node) {
     if (loop instanceof ForToStatementNode) {
       typed = ((ForToStatementNode) loop).getInitializerExpression();
     } else {
-      typed = ((ForInStatementNode) loop).getCurrentDeclaration();
+      EnumeratorOccurrence enumerator = ((ForInStatementNode) loop).getEnumeratorOccurrence();
+      if (enumerator != null) {
+        typed = (Typed) enumerator.getCurrent().getNameDeclaration();
+      } else {
+        typed = null;
+      }
     }
 
     return getDeclaredTypeWithTypeInferenceFallback(

delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/occurrence/EnumeratorOccurrenceImpl.java

@@ -0,0 +1,50 @@
+/*
+ * Sonar Delphi Plugin
+ * Copyright (C) 2025 Integrated Application Development
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package au.com.integradev.delphi.symbol.occurrence;
+
+import org.sonar.plugins.communitydelphi.api.symbol.EnumeratorOccurrence;
+import org.sonar.plugins.communitydelphi.api.symbol.NameOccurrence;
+
+public class EnumeratorOccurrenceImpl implements EnumeratorOccurrence {
+  private final NameOccurrence getEnumerator;
+  private final NameOccurrence moveNext;
+  private final NameOccurrence current;
+
+  public EnumeratorOccurrenceImpl(
+      NameOccurrence getEnumerator, NameOccurrence moveNext, NameOccurrence current) {
+    this.getEnumerator = getEnumerator;
+    this.moveNext = moveNext;
+    this.current = current;
+  }
+
+  @Override
+  public NameOccurrence getGetEnumerator() {
+    return getEnumerator;
+  }
+
+  @Override
+  public NameOccurrence getMoveNext() {
+    return moveNext;
+  }
+
+  @Override
+  public NameOccurrence getCurrent() {
+    return current;
+  }
+}