Skip to content

Commit df79caf

Browse files
committed
Prevent double-resolution of property specifiers
Property specifiers `read`/`write`/`implements` were being resolved twice, which was unnecessary because they should be resolved as part of the top-level resolution of the `PropertyNode`. In a funny twist, it turns out we were actually relying on that second visit to resolve fields in `read`/`write` specifiers. This is now handled properly in the top-level resolve.
1 parent 2f35841 commit df79caf

File tree

2 files changed

+31
-2
lines changed

2 files changed

+31
-2
lines changed

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@
102102
import org.sonar.plugins.communitydelphi.api.ast.PointerTypeNode;
103103
import org.sonar.plugins.communitydelphi.api.ast.PrimaryExpressionNode;
104104
import org.sonar.plugins.communitydelphi.api.ast.ProcedureTypeHeadingNode;
105+
import org.sonar.plugins.communitydelphi.api.ast.PropertyImplementsSpecifierNode;
105106
import org.sonar.plugins.communitydelphi.api.ast.PropertyNode;
107+
import org.sonar.plugins.communitydelphi.api.ast.PropertyReadSpecifierNode;
108+
import org.sonar.plugins.communitydelphi.api.ast.PropertyWriteSpecifierNode;
106109
import org.sonar.plugins.communitydelphi.api.ast.RecordTypeNode;
107110
import org.sonar.plugins.communitydelphi.api.ast.RecordVariantTagNode;
108111
import org.sonar.plugins.communitydelphi.api.ast.RepeatStatementNode;
@@ -517,6 +520,24 @@ public Data visit(PropertyNode node, Data data) {
517520
return createDeclarationScope(node, data);
518521
}
519522

523+
@Override
524+
public Data visit(PropertyReadSpecifierNode node, Data data) {
525+
// Already resolved by the PropertyNode visit.
526+
return data;
527+
}
528+
529+
@Override
530+
public Data visit(PropertyWriteSpecifierNode node, Data data) {
531+
// Already resolved by the PropertyNode visit.
532+
return data;
533+
}
534+
535+
@Override
536+
public Data visit(PropertyImplementsSpecifierNode node, Data data) {
537+
// Already resolved by the PropertyNode visit.
538+
return data;
539+
}
540+
520541
@Nullable
521542
private static PropertyNameDeclaration findConcretePropertyDeclaration(PropertyNode property) {
522543
if (!property.getType().isUnknown()) {

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,15 +253,19 @@ public void resolve(PropertyNode property) {
253253
if (read != null) {
254254
NameResolver readResolver = createNameResolver();
255255
readResolver.readPrimaryExpression(read.getExpression());
256-
readResolver.disambiguateParameters(getGetterParameterTypes(property));
256+
if (isResolvingRoutine(readResolver)) {
257+
readResolver.disambiguateParameters(getGetterParameterTypes(property));
258+
}
257259
readResolver.addToSymbolTable();
258260
}
259261

260262
PropertyWriteSpecifierNode write = property.getWriteSpecifier();
261263
if (write != null) {
262264
NameResolver writeResolver = createNameResolver();
263265
writeResolver.readPrimaryExpression(write.getExpression());
264-
writeResolver.disambiguateParameters(getSetterParameterTypes(property));
266+
if (isResolvingRoutine(writeResolver)) {
267+
writeResolver.disambiguateParameters(getSetterParameterTypes(property));
268+
}
265269
writeResolver.addToSymbolTable();
266270
}
267271

@@ -271,6 +275,10 @@ public void resolve(PropertyNode property) {
271275
}
272276
}
273277

278+
private static boolean isResolvingRoutine(NameResolver resolver) {
279+
return resolver.getDeclarations().stream().anyMatch(RoutineNameDeclaration.class::isInstance);
280+
}
281+
274282
private List<Type> getGetterParameterTypes(PropertyNode property) {
275283
if (property.getIndexSpecifier() == null) {
276284
return property.getParameterTypes();

0 commit comments

Comments
 (0)