Skip to content

Commit 9b96b0c

Browse files
cirrasfourls
cirras
authored and
fourls
committed
Fix grammar ambiguity between attributes and interface GUIDs
1 parent 5b09e54 commit 9b96b0c

File tree

19 files changed

+227
-51
lines changed

19 files changed

+227
-51
lines changed

‎CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- **API:** `TypeParameterNode::getTypeParameters` method.
13+
- **API:** `InterfaceTypeNode::getGuidExpression` method.
14+
- **API:** `AttributeNode::getExpression` method.
15+
16+
### Deprecated
17+
18+
- **API:** `InterfaceGuidNode` node type.
19+
- **API:** `InterfaceTypeNode::getGuid` method, use `getGuidExpression` instead.
20+
- **API:** `DelphiTokenType.GUID`, as the associated `InterfaceGuidNode` is no longer parsed.
1321

1422
### Fixed
1523

1624
- Name resolution failures on generic routine invocations where later type parameters are constrained by earlier type parameters.
1725
- Type resolution failures on `as` casts where the type is returned from a routine invocation.
1826
- Inaccurate type resolution when calling a constructor on a class reference type.
27+
- Grammar ambiguity causing attributes to be misinterpreted as interface GUIDs.
1928

2029
## [1.16.0] - 2025-05-09
2130

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import com.google.common.base.Splitter;
2222
import com.google.common.collect.ImmutableSortedSet;
23+
import java.util.Objects;
2324
import java.util.Set;
2425
import org.sonar.check.Rule;
2526
import org.sonar.check.RuleProperty;
@@ -76,7 +77,8 @@ public DelphiCheckContext visit(AttributeNode attribute, DelphiCheckContext cont
7677
NameDeclaration declaration = occurrence.getNameDeclaration();
7778
if (declaration instanceof RoutineNameDeclaration
7879
&& routinesSet.contains(((RoutineNameDeclaration) declaration).fullyQualifiedName())) {
79-
reportIssue(context, attribute.getNameReference().getIdentifier(), message);
80+
NameReferenceNode reference = Objects.requireNonNull(attribute.getNameReference());
81+
reportIssue(context, reference.getIdentifier(), message);
8082
}
8183
}
8284
return super.visit(attribute, context);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public class InterfaceGuidCheck extends DelphiCheck {
3434
public DelphiCheckContext visit(TypeDeclarationNode typeDeclaration, DelphiCheckContext context) {
3535
if (typeDeclaration.isInterface()) {
3636
InterfaceTypeNode interfaceType = (InterfaceTypeNode) typeDeclaration.getTypeNode();
37-
if (!interfaceType.isForwardDeclaration() && interfaceType.getGuid() == null) {
37+
if (!interfaceType.isForwardDeclaration() && interfaceType.getGuidExpression() == null) {
3838
reportIssue(context, typeDeclaration.getTypeNameNode(), MESSAGE);
3939
}
4040
}

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,12 @@ public DelphiCheckContext visit(UnitImportNode importNode, DelphiCheckContext co
118118

119119
@Override
120120
public DelphiCheckContext visit(AttributeNode attributeNode, DelphiCheckContext context) {
121-
List<NameReferenceNode> nameReferences = attributeNode.getNameReference().flatten();
121+
NameReferenceNode reference = attributeNode.getNameReference();
122+
if (reference == null) {
123+
return super.visit(attributeNode, context);
124+
}
125+
126+
List<NameReferenceNode> nameReferences = reference.flatten();
122127

123128
for (int i = 0; i + 1 < nameReferences.size(); i++) {
124129
context = super.visit(nameReferences.get(i), context);

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ tokens {
1111
// Deprecated tokens
1212
//----------------------------------------------------------------------------
1313
AMPERSAND__deprecated;
14+
TkGuid__deprecated;
1415

1516
//----------------------------------------------------------------------------
1617
// Imaginary tokens
@@ -25,7 +26,6 @@ tokens {
2526
TkRecordVariantItem;
2627
TkRecordVariantTag;
2728
TkRecordExpressionItem;
28-
TkGuid;
2929
TkClassParents;
3030
TkLocalDeclarations;
3131
TkCaseItem;
@@ -708,9 +708,7 @@ fieldDecl : attributeList? nameDeclarationList ':' varType po
708708
;
709709
classHelperType : CLASS<ClassHelperTypeNodeImpl>^ HELPER classParent? FOR typeReference visibilitySection* END
710710
;
711-
interfaceType : (INTERFACE<InterfaceTypeNodeImpl>^ | DISPINTERFACE<InterfaceTypeNodeImpl>^) classParent? (interfaceGuid? interfaceItems? END)?
712-
;
713-
interfaceGuid : lbrack expression rbrack -> ^(TkGuid<InterfaceGuidNodeImpl> expression)
711+
interfaceType : (INTERFACE<InterfaceTypeNodeImpl>^ | DISPINTERFACE<InterfaceTypeNodeImpl>^) classParent? ((interfaceItems | attributeList?) END)?
714712
;
715713
interfaceItems : interfaceItem+ -> ^(TkVisibilitySection<VisibilitySectionNodeImpl> interfaceItem+)
716714
;
@@ -913,8 +911,8 @@ attributeList : attributeGroup+
913911
attributeGroup : lbrack (attribute ','?)+ rbrack
914912
-> ^(TkAttributeGroup<AttributeGroupNodeImpl> attribute+)
915913
;
916-
attribute : (ASSEMBLY ':')? nameReference argumentList? (':' nameReference argumentList?)*
917-
-> ^(TkAttribute<AttributeNodeImpl> ASSEMBLY? nameReference argumentList? (':' nameReference argumentList?)*)
914+
attribute : (ASSEMBLY ':')? expression (':' expression)*
915+
-> ^(TkAttribute<AttributeNodeImpl> ASSEMBLY? expression (':' expression)*)
918916
;
919917

920918
//----------------------------------------------------------------------------

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public List<AttributeNode> getAttributes() {
4747
@Override
4848
public String getImage() {
4949
return getAttributes().stream()
50-
.map(attribute -> attribute.getNameReference().fullyQualifiedName())
50+
.map(AttributeNode::getImage)
5151
.collect(Collectors.joining(", ", "[", "]"));
5252
}
5353
}

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

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,8 @@
2525
import org.sonar.plugins.communitydelphi.api.ast.AttributeGroupNode;
2626
import org.sonar.plugins.communitydelphi.api.ast.AttributeListNode;
2727
import org.sonar.plugins.communitydelphi.api.ast.AttributeNode;
28-
import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration;
29-
import org.sonar.plugins.communitydelphi.api.symbol.declaration.TypeNameDeclaration;
28+
import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
3029
import org.sonar.plugins.communitydelphi.api.type.Type;
31-
import org.sonar.plugins.communitydelphi.api.type.TypeFactory;
3230

3331
public final class AttributeListNodeImpl extends DelphiNodeImpl implements AttributeListNode {
3432
public AttributeListNodeImpl(Token token) {
@@ -57,20 +55,9 @@ public List<AttributeNode> getAttributes() {
5755
@Override
5856
public List<Type> getAttributeTypes() {
5957
return getAttributes().stream()
60-
.map(AttributeNode::getTypeNameOccurrence)
61-
.map(
62-
occurrence -> {
63-
if (occurrence == null) {
64-
return TypeFactory.unknownType();
65-
}
66-
67-
NameDeclaration declaration = occurrence.getNameDeclaration();
68-
if (!(declaration instanceof TypeNameDeclaration)) {
69-
return TypeFactory.unknownType();
70-
}
71-
72-
return ((TypeNameDeclaration) declaration).getType();
73-
})
58+
.map(AttributeNode::getExpression)
59+
.map(ExpressionNode::getType)
60+
.filter(type -> type.isUnknown() || type.isDescendantOf("System.TCustomAttribute"))
7461
.collect(Collectors.toList());
7562
}
7663
}

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

Lines changed: 63 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,25 @@
2020

2121
import au.com.integradev.delphi.antlr.ast.visitors.DelphiParserVisitor;
2222
import au.com.integradev.delphi.symbol.occurrence.AttributeNameOccurrenceImpl;
23+
import com.google.common.base.Suppliers;
24+
import java.util.function.Supplier;
25+
import javax.annotation.Nullable;
2326
import org.antlr.runtime.Token;
2427
import org.sonar.plugins.communitydelphi.api.ast.ArgumentListNode;
2528
import org.sonar.plugins.communitydelphi.api.ast.AttributeNode;
2629
import org.sonar.plugins.communitydelphi.api.ast.DelphiNode;
30+
import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
2731
import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode;
32+
import org.sonar.plugins.communitydelphi.api.ast.PrimaryExpressionNode;
2833
import org.sonar.plugins.communitydelphi.api.symbol.NameOccurrence;
2934
import org.sonar.plugins.communitydelphi.api.token.DelphiTokenType;
3035

3136
public final class AttributeNodeImpl extends DelphiNodeImpl implements AttributeNode {
37+
private final Supplier<NameReferenceNode> nameReferenceSupplier =
38+
Suppliers.memoize(this::findNameReference);
39+
private final Supplier<ArgumentListNode> argumentListSupplier =
40+
Suppliers.memoize(this::findArgumentList);
41+
3242
public AttributeNodeImpl(Token token) {
3343
super(token);
3444
}
@@ -42,14 +52,32 @@ public boolean isAssembly() {
4252
return getChild(0).getTokenType() == DelphiTokenType.ASSEMBLY;
4353
}
4454

55+
@Override
56+
public ExpressionNode getExpression() {
57+
return (ExpressionNode) getChild(isAssembly() ? 1 : 0);
58+
}
59+
60+
@Nullable
4561
@Override
4662
public NameReferenceNode getNameReference() {
47-
return (NameReferenceNode) getChild(isAssembly() ? 1 : 0);
63+
return nameReferenceSupplier.get();
64+
}
65+
66+
@Override
67+
public ArgumentListNode getArgumentList() {
68+
return argumentListSupplier.get();
4869
}
4970

5071
@Override
5172
public NameOccurrence getTypeNameOccurrence() {
52-
return getNameReference().getLastName().getNameOccurrence();
73+
NameReferenceNode nameReference = getNameReference();
74+
if (nameReference != null) {
75+
NameOccurrence occurrence = nameReference.getLastName().getNameOccurrence();
76+
if (occurrence instanceof AttributeNameOccurrenceImpl) {
77+
return occurrence;
78+
}
79+
}
80+
return null;
5381
}
5482

5583
@Override
@@ -62,21 +90,45 @@ public NameOccurrence getConstructorNameOccurrence() {
6290
}
6391

6492
@Override
65-
public ArgumentListNode getArgumentList() {
66-
DelphiNode node = getChild(isAssembly() ? 2 : 1);
67-
if (node instanceof ArgumentListNode) {
68-
return (ArgumentListNode) node;
93+
public String getImage() {
94+
DelphiNode imageNode = getNameReference();
95+
if (imageNode == null) {
96+
imageNode = getExpression();
6997
}
70-
return null;
71-
}
7298

73-
@Override
74-
public String getImage() {
75-
return "[" + getNameReference().fullyQualifiedName() + "]";
99+
String image = imageNode.getImage();
100+
if (isAssembly()) {
101+
image = "assembly " + image;
102+
}
103+
104+
return image;
76105
}
77106

78107
@Override
79108
public <T> T accept(DelphiParserVisitor<T> visitor, T data) {
80109
return visitor.visit(this, data);
81110
}
111+
112+
private NameReferenceNode findNameReference() {
113+
ExpressionNode expression = getExpression();
114+
if (expression instanceof PrimaryExpressionNode) {
115+
DelphiNode child = expression.getChild(0);
116+
if (child instanceof NameReferenceNode) {
117+
return (NameReferenceNode) child;
118+
}
119+
}
120+
return null;
121+
}
122+
123+
private ArgumentListNode findArgumentList() {
124+
NameReferenceNode nameReference = getNameReference();
125+
if (nameReference != null) {
126+
int index = nameReference.getChildIndex() + 1;
127+
DelphiNode next = nameReference.getParent().getChild(index);
128+
if (next instanceof ArgumentListNode) {
129+
return (ArgumentListNode) next;
130+
}
131+
}
132+
return null;
133+
}
82134
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.antlr.runtime.Token;
2323
import org.sonar.plugins.communitydelphi.api.ast.InterfaceGuidNode;
2424

25+
@SuppressWarnings("removal")
2526
public final class InterfaceGuidNodeImpl extends DelphiNodeImpl implements InterfaceGuidNode {
2627
public InterfaceGuidNodeImpl(Token token) {
2728
super(token);

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

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,25 @@
1919
package au.com.integradev.delphi.antlr.ast.node;
2020

2121
import au.com.integradev.delphi.antlr.ast.visitors.DelphiParserVisitor;
22+
import com.google.common.base.Suppliers;
23+
import com.google.common.collect.Iterables;
24+
import java.util.function.Supplier;
25+
import javax.annotation.Nullable;
2226
import org.antlr.runtime.Token;
27+
import org.sonar.plugins.communitydelphi.api.ast.AttributeGroupNode;
28+
import org.sonar.plugins.communitydelphi.api.ast.AttributeListNode;
29+
import org.sonar.plugins.communitydelphi.api.ast.AttributeNode;
30+
import org.sonar.plugins.communitydelphi.api.ast.DelphiNode;
31+
import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
2332
import org.sonar.plugins.communitydelphi.api.ast.InterfaceGuidNode;
2433
import org.sonar.plugins.communitydelphi.api.ast.InterfaceTypeNode;
34+
import org.sonar.plugins.communitydelphi.api.ast.PropertyNode;
35+
import org.sonar.plugins.communitydelphi.api.ast.RoutineDeclarationNode;
36+
import org.sonar.plugins.communitydelphi.api.ast.VisibilitySectionNode;
2537

2638
public final class InterfaceTypeNodeImpl extends StructTypeNodeImpl implements InterfaceTypeNode {
39+
private final Supplier<ExpressionNode> guid = Suppliers.memoize(this::findGuidExpression);
40+
2741
public InterfaceTypeNodeImpl(Token token) {
2842
super(token);
2943
}
@@ -38,8 +52,54 @@ public boolean isForwardDeclaration() {
3852
return getChildren().isEmpty();
3953
}
4054

55+
@SuppressWarnings("removal")
4156
@Override
4257
public InterfaceGuidNode getGuid() {
43-
return getFirstChildOfType(InterfaceGuidNode.class);
58+
return null;
59+
}
60+
61+
@Nullable
62+
@Override
63+
public ExpressionNode getGuidExpression() {
64+
return guid.get();
65+
}
66+
67+
private ExpressionNode findGuidExpression() {
68+
AttributeListNode attributeList = findFirstAttributeList();
69+
if (attributeList != null) {
70+
AttributeGroupNode attributeGroup = attributeList.getAttributeGroups().get(0);
71+
AttributeNode attribute = Iterables.getLast(attributeGroup.getAttributes());
72+
ExpressionNode expression = attribute.getExpression();
73+
if (expression.getType().isString()) {
74+
return expression;
75+
}
76+
}
77+
return null;
78+
}
79+
80+
private AttributeListNode findFirstAttributeList() {
81+
for (DelphiNode child : getChildren()) {
82+
if (child instanceof AttributeListNode) {
83+
return (AttributeListNode) child;
84+
}
85+
86+
if (child instanceof VisibilitySectionNode) {
87+
return findFirstAttributeList((VisibilitySectionNode) child);
88+
}
89+
}
90+
return null;
91+
}
92+
93+
private AttributeListNode findFirstAttributeList(VisibilitySectionNode visibilitySection) {
94+
for (DelphiNode child : visibilitySection.getChildren()) {
95+
if (child instanceof RoutineDeclarationNode) {
96+
return ((RoutineDeclarationNode) child).getRoutineHeading().getAttributeList();
97+
}
98+
99+
if (child instanceof PropertyNode) {
100+
return ((PropertyNode) child).getAttributeList();
101+
}
102+
}
103+
return null;
44104
}
45105
}

0 commit comments

Comments
 (0)