Fix grammar ambiguity between attributes and interface GUIDs

CHANGELOG.md

@@ -10,12 +10,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - **API:** `TypeParameterNode::getTypeParameters` method.
+- **API:** `InterfaceTypeNode::getGuidExpression` method.
+- **API:** `AttributeNode::getExpression` method.
+
+### Changed
+
+- Issue locations no longer span the entire routine declaration in `RoutineName`.
+
+### Deprecated
+
+- **API:** `InterfaceGuidNode` node type.
+- **API:** `InterfaceTypeNode::getGuid` method, use `getGuidExpression` instead.
+- **API:** `DelphiTokenType.GUID`, as the associated `InterfaceGuidNode` is no longer parsed.
 
 ### Fixed
 
 - Name resolution failures on generic routine invocations where later type parameters are constrained by earlier type parameters.
+- Type resolution failures on property attribute lists.
+- Type resolution failures on implementation-local routine attribute lists.
 - Type resolution failures on `as` casts where the type is returned from a routine invocation.
 - Inaccurate type resolution when calling a constructor on a class reference type.
+- Grammar ambiguity causing attributes to be misinterpreted as interface GUIDs.
 
 ## [1.16.0] - 2025-05-09
 

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

@@ -20,6 +20,7 @@
 
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableSortedSet;
+import java.util.Objects;
 import java.util.Set;
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;
@@ -76,7 +77,8 @@ public DelphiCheckContext visit(AttributeNode attribute, DelphiCheckContext cont
       NameDeclaration declaration = occurrence.getNameDeclaration();
       if (declaration instanceof RoutineNameDeclaration
           && routinesSet.contains(((RoutineNameDeclaration) declaration).fullyQualifiedName())) {
-        reportIssue(context, attribute.getNameReference().getIdentifier(), message);
+        NameReferenceNode reference = Objects.requireNonNull(attribute.getNameReference());
+        reportIssue(context, reference.getIdentifier(), message);
       }
     }
     return super.visit(attribute, context);

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

@@ -34,7 +34,7 @@ public class InterfaceGuidCheck extends DelphiCheck {
   public DelphiCheckContext visit(TypeDeclarationNode typeDeclaration, DelphiCheckContext context) {
     if (typeDeclaration.isInterface()) {
       InterfaceTypeNode interfaceType = (InterfaceTypeNode) typeDeclaration.getTypeNode();
-      if (!interfaceType.isForwardDeclaration() && interfaceType.getGuid() == null) {
+      if (!interfaceType.isForwardDeclaration() && interfaceType.getGuidExpression() == null) {
         reportIssue(context, typeDeclaration.getTypeNameNode(), MESSAGE);
       }
     }

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

@@ -118,7 +118,12 @@ public DelphiCheckContext visit(UnitImportNode importNode, DelphiCheckContext co
 
   @Override
   public DelphiCheckContext visit(AttributeNode attributeNode, DelphiCheckContext context) {
-    List<NameReferenceNode> nameReferences = attributeNode.getNameReference().flatten();
+    NameReferenceNode reference = attributeNode.getNameReference();
+    if (reference == null) {
+      return super.visit(attributeNode, context);
+    }
+
+    List<NameReferenceNode> nameReferences = reference.flatten();
 
     for (int i = 0; i + 1 < nameReferences.size(); i++) {
       context = super.visit(nameReferences.get(i), context);

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

@@ -38,7 +38,7 @@ public class RoutineNameCheck extends DelphiCheck {
   @Override
   public DelphiCheckContext visit(RoutineDeclarationNode routine, DelphiCheckContext context) {
     if (isViolation(routine) && !isExcluded(routine)) {
-      reportIssue(context, routine, MESSAGE);
+      reportIssue(context, routine.getRoutineNameNode(), MESSAGE);
     }
     return super.visit(routine, context);
   }

delphi-frontend/src/main/antlr3/au/com/integradev/delphi/antlr/Delphi.g

@@ -11,6 +11,7 @@ tokens {
   // Deprecated tokens
   //----------------------------------------------------------------------------
   AMPERSAND__deprecated;
+  TkGuid__deprecated;
 
   //----------------------------------------------------------------------------
   // Imaginary tokens
@@ -25,7 +26,6 @@ tokens {
   TkRecordVariantItem;
   TkRecordVariantTag;
   TkRecordExpressionItem;
-  TkGuid;
   TkClassParents;
   TkLocalDeclarations;
   TkCaseItem;
@@ -708,9 +708,7 @@ fieldDecl                    : attributeList? nameDeclarationList ':' varType po
                              ;
 classHelperType              : CLASS<ClassHelperTypeNodeImpl>^ HELPER classParent? FOR typeReference visibilitySection* END
                              ;
-interfaceType                : (INTERFACE<InterfaceTypeNodeImpl>^ | DISPINTERFACE<InterfaceTypeNodeImpl>^) classParent? (interfaceGuid? interfaceItems? END)?
-                             ;
-interfaceGuid                : lbrack expression rbrack -> ^(TkGuid<InterfaceGuidNodeImpl> expression)
+interfaceType                : (INTERFACE<InterfaceTypeNodeImpl>^ | DISPINTERFACE<InterfaceTypeNodeImpl>^) classParent? ((interfaceItems | attributeList?)  END)?
                              ;
 interfaceItems               : interfaceItem+ -> ^(TkVisibilitySection<VisibilitySectionNodeImpl> interfaceItem+)
                              ;
@@ -913,8 +911,8 @@ attributeList                : attributeGroup+
 attributeGroup               : lbrack (attribute ','?)+ rbrack
                              -> ^(TkAttributeGroup<AttributeGroupNodeImpl> attribute+)
                              ;
-attribute                    : (ASSEMBLY ':')? nameReference argumentList? (':' nameReference argumentList?)*
-                             -> ^(TkAttribute<AttributeNodeImpl> ASSEMBLY? nameReference argumentList? (':' nameReference argumentList?)*)
+attribute                    : (ASSEMBLY ':')? expression (':' expression)*
+                             -> ^(TkAttribute<AttributeNodeImpl> ASSEMBLY? expression (':' expression)*)
                              ;
 
 //----------------------------------------------------------------------------

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

@@ -47,7 +47,7 @@ public List<AttributeNode> getAttributes() {
   @Override
   public String getImage() {
     return getAttributes().stream()
-        .map(attribute -> attribute.getNameReference().fullyQualifiedName())
+        .map(AttributeNode::getImage)
         .collect(Collectors.joining(", ", "[", "]"));
   }
 }

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

@@ -25,10 +25,8 @@
 import org.sonar.plugins.communitydelphi.api.ast.AttributeGroupNode;
 import org.sonar.plugins.communitydelphi.api.ast.AttributeListNode;
 import org.sonar.plugins.communitydelphi.api.ast.AttributeNode;
-import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration;
-import org.sonar.plugins.communitydelphi.api.symbol.declaration.TypeNameDeclaration;
+import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
 import org.sonar.plugins.communitydelphi.api.type.Type;
-import org.sonar.plugins.communitydelphi.api.type.TypeFactory;
 
 public final class AttributeListNodeImpl extends DelphiNodeImpl implements AttributeListNode {
   public AttributeListNodeImpl(Token token) {
@@ -57,20 +55,9 @@ public List<AttributeNode> getAttributes() {
   @Override
   public List<Type> getAttributeTypes() {
     return getAttributes().stream()
-        .map(AttributeNode::getTypeNameOccurrence)
-        .map(
-            occurrence -> {
-              if (occurrence == null) {
-                return TypeFactory.unknownType();
-              }
-
-              NameDeclaration declaration = occurrence.getNameDeclaration();
-              if (!(declaration instanceof TypeNameDeclaration)) {
-                return TypeFactory.unknownType();
-              }
-
-              return ((TypeNameDeclaration) declaration).getType();
-            })
+        .map(AttributeNode::getExpression)
+        .map(ExpressionNode::getType)
+        .filter(type -> type.isUnknown() || type.isDescendantOf("System.TCustomAttribute"))
         .collect(Collectors.toList());
   }
 }

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

@@ -20,15 +20,25 @@
 
 import au.com.integradev.delphi.antlr.ast.visitors.DelphiParserVisitor;
 import au.com.integradev.delphi.symbol.occurrence.AttributeNameOccurrenceImpl;
+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.ArgumentListNode;
 import org.sonar.plugins.communitydelphi.api.ast.AttributeNode;
 import org.sonar.plugins.communitydelphi.api.ast.DelphiNode;
+import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
 import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode;
+import org.sonar.plugins.communitydelphi.api.ast.PrimaryExpressionNode;
 import org.sonar.plugins.communitydelphi.api.symbol.NameOccurrence;
 import org.sonar.plugins.communitydelphi.api.token.DelphiTokenType;
 
 public final class AttributeNodeImpl extends DelphiNodeImpl implements AttributeNode {
+  private final Supplier<NameReferenceNode> nameReferenceSupplier =
+      Suppliers.memoize(this::findNameReference);
+  private final Supplier<ArgumentListNode> argumentListSupplier =
+      Suppliers.memoize(this::findArgumentList);
+
   public AttributeNodeImpl(Token token) {
     super(token);
   }
@@ -42,14 +52,32 @@ public boolean isAssembly() {
     return getChild(0).getTokenType() == DelphiTokenType.ASSEMBLY;
   }
 
+  @Override
+  public ExpressionNode getExpression() {
+    return (ExpressionNode) getChild(isAssembly() ? 1 : 0);
+  }
+
+  @Nullable
   @Override
   public NameReferenceNode getNameReference() {
-    return (NameReferenceNode) getChild(isAssembly() ? 1 : 0);
+    return nameReferenceSupplier.get();
+  }
+
+  @Override
+  public ArgumentListNode getArgumentList() {
+    return argumentListSupplier.get();
   }
 
   @Override
   public NameOccurrence getTypeNameOccurrence() {
-    return getNameReference().getLastName().getNameOccurrence();
+    NameReferenceNode nameReference = getNameReference();
+    if (nameReference != null) {
+      NameOccurrence occurrence = nameReference.getLastName().getNameOccurrence();
+      if (occurrence instanceof AttributeNameOccurrenceImpl) {
+        return occurrence;
+      }
+    }
+    return null;
   }
 
   @Override
@@ -62,21 +90,45 @@ public NameOccurrence getConstructorNameOccurrence() {
   }
 
   @Override
-  public ArgumentListNode getArgumentList() {
-    DelphiNode node = getChild(isAssembly() ? 2 : 1);
-    if (node instanceof ArgumentListNode) {
-      return (ArgumentListNode) node;
+  public String getImage() {
+    DelphiNode imageNode = getNameReference();
+    if (imageNode == null) {
+      imageNode = getExpression();
     }
-    return null;
-  }
 
-  @Override
-  public String getImage() {
-    return "[" + getNameReference().fullyQualifiedName() + "]";
+    String image = imageNode.getImage();
+    if (isAssembly()) {
+      image = "assembly " + image;
+    }
+
+    return image;
   }
 
   @Override
   public <T> T accept(DelphiParserVisitor<T> visitor, T data) {
     return visitor.visit(this, data);
   }
+
+  private NameReferenceNode findNameReference() {
+    ExpressionNode expression = getExpression();
+    if (expression instanceof PrimaryExpressionNode) {
+      DelphiNode child = expression.getChild(0);
+      if (child instanceof NameReferenceNode) {
+        return (NameReferenceNode) child;
+      }
+    }
+    return null;
+  }
+
+  private ArgumentListNode findArgumentList() {
+    NameReferenceNode nameReference = getNameReference();
+    if (nameReference != null) {
+      int index = nameReference.getChildIndex() + 1;
+      DelphiNode next = nameReference.getParent().getChild(index);
+      if (next instanceof ArgumentListNode) {
+        return (ArgumentListNode) next;
+      }
+    }
+    return null;
+  }
 }

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

@@ -22,6 +22,7 @@
 import org.antlr.runtime.Token;
 import org.sonar.plugins.communitydelphi.api.ast.InterfaceGuidNode;
 
+@SuppressWarnings("removal")
 public final class InterfaceGuidNodeImpl extends DelphiNodeImpl implements InterfaceGuidNode {
   public InterfaceGuidNodeImpl(Token token) {
     super(token);

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

@@ -19,11 +19,25 @@
 package au.com.integradev.delphi.antlr.ast.node;
 
 import au.com.integradev.delphi.antlr.ast.visitors.DelphiParserVisitor;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.Iterables;
+import java.util.function.Supplier;
+import javax.annotation.Nullable;
 import org.antlr.runtime.Token;
+import org.sonar.plugins.communitydelphi.api.ast.AttributeGroupNode;
+import org.sonar.plugins.communitydelphi.api.ast.AttributeListNode;
+import org.sonar.plugins.communitydelphi.api.ast.AttributeNode;
+import org.sonar.plugins.communitydelphi.api.ast.DelphiNode;
+import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
 import org.sonar.plugins.communitydelphi.api.ast.InterfaceGuidNode;
 import org.sonar.plugins.communitydelphi.api.ast.InterfaceTypeNode;
+import org.sonar.plugins.communitydelphi.api.ast.PropertyNode;
+import org.sonar.plugins.communitydelphi.api.ast.RoutineDeclarationNode;
+import org.sonar.plugins.communitydelphi.api.ast.VisibilitySectionNode;
 
 public final class InterfaceTypeNodeImpl extends StructTypeNodeImpl implements InterfaceTypeNode {
+  private final Supplier<ExpressionNode> guid = Suppliers.memoize(this::findGuidExpression);
+
   public InterfaceTypeNodeImpl(Token token) {
     super(token);
   }
@@ -38,8 +52,54 @@ public boolean isForwardDeclaration() {
     return getChildren().isEmpty();
   }
 
+  @SuppressWarnings("removal")
   @Override
   public InterfaceGuidNode getGuid() {
-    return getFirstChildOfType(InterfaceGuidNode.class);
+    return null;
+  }
+
+  @Nullable
+  @Override
+  public ExpressionNode getGuidExpression() {
+    return guid.get();
+  }
+
+  private ExpressionNode findGuidExpression() {
+    AttributeListNode attributeList = findFirstAttributeList();
+    if (attributeList != null) {
+      AttributeGroupNode attributeGroup = attributeList.getAttributeGroups().get(0);
+      AttributeNode attribute = Iterables.getLast(attributeGroup.getAttributes());
+      ExpressionNode expression = attribute.getExpression();
+      if (expression.getType().isString()) {
+        return expression;
+      }
+    }
+    return null;
+  }
+
+  private AttributeListNode findFirstAttributeList() {
+    for (DelphiNode child : getChildren()) {
+      if (child instanceof AttributeListNode) {
+        return (AttributeListNode) child;
+      }
+
+      if (child instanceof VisibilitySectionNode) {
+        return findFirstAttributeList((VisibilitySectionNode) child);
+      }
+    }
+    return null;
+  }
+
+  private AttributeListNode findFirstAttributeList(VisibilitySectionNode visibilitySection) {
+    for (DelphiNode child : visibilitySection.getChildren()) {
+      if (child instanceof RoutineDeclarationNode) {
+        return ((RoutineDeclarationNode) child).getRoutineHeading().getAttributeList();
+      }
+
+      if (child instanceof PropertyNode) {
+        return ((PropertyNode) child).getAttributeList();
+      }
+    }
+    return null;
   }
 }

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

@@ -319,6 +319,7 @@ default T visit(InitializationSectionNode node, T data) {
     return visit((DelphiNode) node, data);
   }
 
+  @SuppressWarnings("removal")
   default T visit(InterfaceGuidNode node, T data) {
     return visit((DelphiNode) node, data);
   }