Implement best-effort resolution of arguments to unresolved invocables

Author: cirras

CHANGELOG.md

@@ -36,7 +36,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Detect tab-indented multiline strings in `TabulationCharacter`.
 - Improve support for evaluating name references in compiler directive expressions.
 - Improve overload resolution in cases involving generic type parameter constraints.
-- Improve handling for MSBuild properties, items, and conditional evaluation. 
+- Improve handling for MSBuild properties, items, and conditional evaluation.
+- Perform best-effort name resolution of arguments to unresolved routines or array properties, which
+  improves analysis quality in cases where symbol information is incomplete.
 
 ### Deprecated
 

delphi-checks/src/main/java/au/com/integradev/delphi/checks/VariableInitializationCheck.java

@@ -194,7 +194,7 @@ private void handleNestedRoutineInvocations(StatementNode statement, DelphiCheck
 
   private void searchForInitializationsByOutArgument(StatementNode statement) {
     findNameReferences(statement).stream()
-        .filter(VariableInitializationCheck::isOutArgument)
+        .filter(VariableInitializationCheck::isPotentiallyInitializingArgument)
         .map(this::getReferredInitializationState)
         .filter(Objects::nonNull)
         .forEach(InitializationState::assignedTo);
@@ -275,7 +275,7 @@ private void handleAssignment(StatementNode statement) {
     }
 
     for (NameReferenceNode name : findNameReferences(assignmentStatement.getValue())) {
-      if (name.getNameDeclaration() == declaration && !isOutArgument(name)) {
+      if (name.getNameDeclaration() == declaration && !isPotentiallyInitializingArgument(name)) {
         return;
       }
     }
@@ -294,7 +294,7 @@ private void handleAssignment(StatementNode statement) {
     }
   }
 
-  private static boolean isOutArgument(NameReferenceNode name) {
+  private static boolean isPotentiallyInitializingArgument(NameReferenceNode name) {
     ArgumentNode argument = getArgument(name);
     if (argument == null) {
       return false;
@@ -303,7 +303,10 @@ private static boolean isOutArgument(NameReferenceNode name) {
     ArgumentListNode argumentList = (ArgumentListNode) argument.getParent();
     ProceduralType procedural = getInvokedProcedural(argumentList);
     if (procedural == null) {
-      return false;
+      // If we can't find the procedural, it might mean:
+      // - an unresolved procedural reference (potentially initializing)
+      // - an explicit array constructor (not initializing)
+      return !isExplicitArrayConstructor(argumentList);
     }
 
     int argumentIndex = argumentList.getArgumentNodes().indexOf(argument);
@@ -400,6 +403,22 @@ private static ProceduralType getInvokedProcedural(ArgumentListNode argumentList
     return (ProceduralType) type;
   }
 
+  private static boolean isExplicitArrayConstructor(ArgumentListNode argumentList) {
+    Node previous = argumentList.getParent().getChild(argumentList.getChildIndex() - 1);
+    if (previous instanceof NameReferenceNode) {
+      NameReferenceNode name = ((NameReferenceNode) previous).getLastName();
+      NameReferenceNode prevName = name.prevName();
+      if (prevName != null) {
+        NameDeclaration arrayDeclaration = prevName.getNameDeclaration();
+        if (arrayDeclaration instanceof TypeNameDeclaration) {
+          return ((TypeNameDeclaration) arrayDeclaration).getType().isDynamicArray()
+              && name.simpleName().equalsIgnoreCase("Create");
+        }
+      }
+    }
+    return false;
+  }
+
   private static RoutineNameDeclaration getRoutineDeclaration(ArgumentListNode argumentList) {
     Node previous = argumentList.getParent().getChild(argumentList.getChildIndex() - 1);
     if (!(previous instanceof NameReferenceNode)) {

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

@@ -720,22 +720,22 @@ void testPassedToProcVarOutParameterShouldNotAddIssue() {
   }
 
   @Test
-  void testFailOnUpgradeVarPassedToArrayProcVarOutParameterShouldAddIssue() {
+  void testFailOnUpgradePassedToArrayElementProceduralShouldNotAddIssue() {
     CheckVerifier.newVerifier()
         .withCheck(new VariableInitializationCheck())
         .onFile(
             new DelphiTestUnitBuilder()
                 .appendDecl("type")
-                .appendDecl("  TTestProc = reference to procedure(out Int: Integer);")
+                .appendDecl("  TTestProc = reference to procedure(Int: Integer);")
                 .appendDecl("procedure Foo(Int: Integer);")
                 .appendImpl("procedure Test(ProcArray: array of TTestProc);")
                 .appendImpl("var")
                 .appendImpl("  I: Integer;")
                 .appendImpl("begin")
-                .appendImpl("  ProcArray[0](I); // Noncompliant")
+                .appendImpl("  ProcArray[0](I);")
                 .appendImpl("  Foo(I);")
                 .appendImpl("end;"))
-        .verifyIssues();
+        .verifyNoIssues();
   }
 
   @Test
@@ -947,6 +947,24 @@ void testUnknownRoutineArgumentShouldNotAddIssue() {
         .verifyNoIssues();
   }
 
+  @Test
+  void testUninitializedArgumentToExplicitArrayConstructorShouldAddIssue() {
+    CheckVerifier.newVerifier()
+        .withCheck(new VariableInitializationCheck())
+        .onFile(
+            new DelphiTestUnitBuilder()
+                .appendImpl("procedure Test;")
+                .appendImpl("type")
+                .appendImpl("  TBar = array of Integer;")
+                .appendImpl("var")
+                .appendImpl("  Foo: Integer;")
+                .appendImpl("  Bar: TBar;")
+                .appendImpl("begin")
+                .appendImpl("  Bar := TBar.Create(Foo); // Noncompliant")
+                .appendImpl("end;"))
+        .verifyIssues();
+  }
+
   @Test
   void testUninitializedObjectWithFreeAndNilShouldAddIssue() {
     CheckVerifier.newVerifier()

delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/InvocationArgument.java

@@ -29,6 +29,7 @@
 import org.sonar.plugins.communitydelphi.api.ast.utils.ExpressionNodeUtils;
 import org.sonar.plugins.communitydelphi.api.type.Type;
 import org.sonar.plugins.communitydelphi.api.type.Type.ProceduralType;
+import org.sonar.plugins.communitydelphi.api.type.TypeFactory;
 import org.sonar.plugins.communitydelphi.api.type.Typed;
 
 public class InvocationArgument implements Typed {
@@ -55,7 +56,7 @@ public class InvocationArgument implements Typed {
   void resolve(Type parameterType) {
     if (resolver != null) {
       if (isRoutineReference(parameterType)) {
-        disambiguateRoutineReference(resolver, parameterType);
+        resolver.disambiguateRoutineReference((ProceduralType) parameterType);
       } else if (!resolver.isExplicitInvocation()) {
         resolver.disambiguateImplicitEmptyArgumentList();
       }
@@ -92,13 +93,12 @@ Type findRoutineReferenceType(Type parameterType) {
     Preconditions.checkNotNull(resolver);
 
     NameResolver clone = new NameResolver(resolver);
-    disambiguateRoutineReference(clone, parameterType);
-    return clone.getApproximateType();
-  }
+    clone.disambiguateRoutineReference((ProceduralType) parameterType);
+    if (!clone.isAmbiguous()) {
+      return clone.getApproximateType();
+    }
 
-  private static void disambiguateRoutineReference(NameResolver resolver, Type parameterType) {
-    resolver.disambiguateRoutineReference((ProceduralType) parameterType);
-    resolver.checkAmbiguity();
+    return TypeFactory.unknownType();
   }
 
   @Override

delphi-frontend/src/main/java/au/com/integradev/delphi/symbol/resolve/NameResolver.java

@@ -25,7 +25,6 @@
 import static org.sonar.plugins.communitydelphi.api.type.TypeFactory.voidType;
 
 import au.com.integradev.delphi.antlr.ast.node.ArrayAccessorNodeImpl;
-import au.com.integradev.delphi.antlr.ast.node.DelphiNodeImpl;
 import au.com.integradev.delphi.antlr.ast.node.NameReferenceNodeImpl;
 import au.com.integradev.delphi.symbol.Search;
 import au.com.integradev.delphi.symbol.SearchMode;
@@ -200,8 +199,12 @@ NameDeclaration addResolvedDeclaration() {
     return resolved;
   }
 
-  public void checkAmbiguity() {
-    if (declarations.size() > 1) {
+  public boolean isAmbiguous() {
+    return declarations.size() > 1;
+  }
+
+  private void checkAmbiguity() {
+    if (isAmbiguous()) {
       if (LOG.isWarnEnabled()) {
         LOG.warn(
             "Ambiguous declarations could not be resolved\n[Occurrence] {}\n{}",
@@ -288,19 +291,11 @@ void readPrimaryExpression(PrimaryExpressionNode node) {
     }
 
     for (DelphiNode child : node.getChildren()) {
-      if (!readPrimaryExpressionPart(child)) {
-        break;
-      }
+      readPrimaryExpressionPart(child);
     }
   }
 
-  /**
-   * Reads part of a primary expression
-   *
-   * @param node part of a primary expression
-   * @return false if a name resolution failure occurs
-   */
-  private boolean readPrimaryExpressionPart(Node node) {
+  private void readPrimaryExpressionPart(Node node) {
     if (node instanceof NameReferenceNode) {
       readNameReference((NameReferenceNode) node);
     } else if (node instanceof ArgumentListNode) {
@@ -314,11 +309,13 @@ private boolean readPrimaryExpressionPart(Node node) {
     } else {
       handlePrimaryExpressionToken(node);
     }
-
-    return !nameResolutionFailed();
   }
 
   private void handlePrimaryExpressionToken(Node node) {
+    if (nameResolutionFailed()) {
+      return;
+    }
+
     switch (node.getTokenType()) {
       case DEREFERENCE:
         Type dereferenced = TypeUtils.dereference(getApproximateType());
@@ -389,9 +386,7 @@ private boolean handleInheritedExpression(PrimaryExpressionNode node) {
     int nextChild = ExpressionNodeUtils.isBareInherited(node) ? 1 : 2;
 
     for (int i = nextChild; i < node.getChildren().size(); ++i) {
-      if (!readPrimaryExpressionPart(node.getChild(i))) {
-        break;
-      }
+      readPrimaryExpressionPart(node.getChild(i));
     }
 
     return true;
@@ -520,8 +515,7 @@ private void handleTypeParameterReferences(List<TypeReferenceNode> typeReference
       occurrence.setNameDeclaration(declaration);
       ((NameReferenceNodeImpl) typeReference).setNameOccurrence(occurrence);
 
-      NameResolver resolver =
-          new NameResolver(((DelphiNodeImpl) typeReference).getTypeFactory(), searchMode);
+      NameResolver resolver = new NameResolver(typeFactory, searchMode);
       resolver.resolvedDeclarations.add(declaration);
       resolver.names.add(occurrence);
       resolver.addToSymbolTable();
@@ -540,8 +534,7 @@ private void handleTypeParameterForwardReferences(
       occurrence.setNameDeclaration(declaration);
       ((NameReferenceNodeImpl) typeReference).setNameOccurrence(occurrence);
 
-      NameResolver resolver =
-          new NameResolver(((DelphiNodeImpl) typeNode).getTypeFactory(), searchMode);
+      NameResolver resolver = new NameResolver(typeFactory, searchMode);
       resolver.resolvedDeclarations.add(declaration);
       resolver.names.add(occurrence);
       resolver.addToSymbolTable();
@@ -576,6 +569,10 @@ void readAttribute(AttributeNode node) {
   }
 
   private void readNameReference(NameReferenceNode node, boolean inAttribute) {
+    if (nameResolutionFailed()) {
+      return;
+    }
+
     boolean couldBeUnitNameReference =
         currentScope == null
             || (!(currentScope instanceof UnknownScope) && currentScope.equals(node.getScope()));
@@ -709,7 +706,7 @@ private boolean isTypeParameterConstructorInvocation(ArgumentListNode argumentLi
   }
 
   private void readPossibleUnitNameReference(NameReferenceNode node, boolean inAttribute) {
-    NameResolver unitNameResolver = new NameResolver(((DelphiNodeImpl) node).getTypeFactory());
+    NameResolver unitNameResolver = new NameResolver(typeFactory);
     if (unitNameResolver.readUnitNameReference(node, inAttribute)) {
       this.currentType = unknownType();
       this.names.clear();
@@ -781,6 +778,11 @@ private boolean matchReferenceToUnitNameDeclaration(
   }
 
   private void handleArrayAccessor(ArrayAccessorNode accessor) {
+    if (nameResolutionFailed()) {
+      resolveArgumentsBestEffort(accessor.getExpressions());
+      return;
+    }
+
     Type type = TypeUtils.findBaseType(getApproximateType());
     if (type.isPointer()) {
       Type dereferenced = TypeUtils.dereference(type);
@@ -921,6 +923,14 @@ private void handleArgumentList(ArgumentListNode node) {
       }
     }
 
+    if (nameResolutionFailed()) {
+      resolveArgumentsBestEffort(
+          node.getArgumentNodes().stream()
+              .map(ArgumentNode::getExpression)
+              .collect(Collectors.toUnmodifiableList()));
+      return;
+    }
+
     if (handleExplicitArrayConstructorInvocation(node)
         || isTypeParameterConstructorInvocation(node)) {
       return;
@@ -1008,6 +1018,20 @@ private boolean handleProcVarInvocation(List<ExpressionNode> argumentExpressions
     return true;
   }
 
+  private void resolveArgumentsBestEffort(List<ExpressionNode> argumentExpressions) {
+    for (ExpressionNode expression : argumentExpressions) {
+      getNameResolutionHelper().resolveSubExpressions(expression);
+      if (expression instanceof PrimaryExpressionNode) {
+        NameResolver resolver = new NameResolver(typeFactory, searchMode);
+        resolver.readPrimaryExpression((PrimaryExpressionNode) expression);
+        if (resolver.isAmbiguous()) {
+          resolver.declarations.clear();
+        }
+        resolver.addToSymbolTable();
+      }
+    }
+  }
+
   private void disambiguateArguments(List<ExpressionNode> argumentExpressions, boolean explicit) {
     if (handleHardTypeCast(argumentExpressions) || handleProcVarInvocation(argumentExpressions)) {
       return;

delphi-frontend/src/test/java/au/com/integradev/delphi/executor/DelphiSymbolTableExecutorTest.java

@@ -426,6 +426,12 @@ void testLabels() {
     verifyUsages(29, 6, reference(31, 7), reference(32, 2));
   }
 
+  @Test
+  void testBestEffortArguments() {
+    execute("BestEffortArguments.pas");
+    verifyUsages(9, 2, reference(11, 6), reference(11, 11));
+  }
+
   @Test
   void testClassReferenceMethodResolution() {
     execute("classReferences/MethodResolution.pas");

delphi-frontend/src/test/resources/au/com/integradev/delphi/symbol/BestEffortArguments.pas

@@ -0,0 +1,14 @@
+unit BestEffortArguments;
+
+interface
+
+implementation
+
+procedure Foo;
+var
+  Bar: Integer;
+begin
+  Baz(Bar)[Bar];
+end;
+
+end.