Skip to content

Commit 7b7f062

Browse files
cirrasfourls
authored andcommitted
Prevent double-resolution of property specifiers
Property specifiers `read`/`write`/`implements`/`stored` 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 non-invocable expressions in `read`/`write`/`stored` specifiers. This is now handled properly in the top-level resolve.
1 parent 737d197 commit 7b7f062

File tree

2 files changed

+41
-3
lines changed

2 files changed

+41
-3
lines changed

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,11 @@
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.PropertyStoredSpecifierNode;
109+
import org.sonar.plugins.communitydelphi.api.ast.PropertyWriteSpecifierNode;
106110
import org.sonar.plugins.communitydelphi.api.ast.RecordTypeNode;
107111
import org.sonar.plugins.communitydelphi.api.ast.RecordVariantTagNode;
108112
import org.sonar.plugins.communitydelphi.api.ast.RepeatStatementNode;
@@ -517,6 +521,30 @@ public Data visit(PropertyNode node, Data data) {
517521
return createDeclarationScope(node, data);
518522
}
519523

524+
@Override
525+
public Data visit(PropertyReadSpecifierNode node, Data data) {
526+
// Already resolved by the PropertyNode visit.
527+
return data;
528+
}
529+
530+
@Override
531+
public Data visit(PropertyWriteSpecifierNode node, Data data) {
532+
// Already resolved by the PropertyNode visit.
533+
return data;
534+
}
535+
536+
@Override
537+
public Data visit(PropertyImplementsSpecifierNode node, Data data) {
538+
// Already resolved by the PropertyNode visit.
539+
return data;
540+
}
541+
542+
@Override
543+
public Data visit(PropertyStoredSpecifierNode node, Data data) {
544+
// Already resolved by the PropertyNode visit.
545+
return data;
546+
}
547+
520548
@Nullable
521549
private static PropertyNameDeclaration findConcretePropertyDeclaration(PropertyNode property) {
522550
if (!property.getType().isUnknown()) {

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,15 +255,19 @@ public void resolve(PropertyNode property) {
255255
if (read != null) {
256256
NameResolver readResolver = createNameResolver();
257257
readResolver.readPrimaryExpression(read.getExpression());
258-
readResolver.disambiguateParameters(getGetterParameterTypes(property));
258+
if (isResolvingRoutine(readResolver)) {
259+
readResolver.disambiguateParameters(getGetterParameterTypes(property));
260+
}
259261
readResolver.addToSymbolTable();
260262
}
261263

262264
PropertyWriteSpecifierNode write = property.getWriteSpecifier();
263265
if (write != null) {
264266
NameResolver writeResolver = createNameResolver();
265267
writeResolver.readPrimaryExpression(write.getExpression());
266-
writeResolver.disambiguateParameters(getSetterParameterTypes(property));
268+
if (isResolvingRoutine(writeResolver)) {
269+
writeResolver.disambiguateParameters(getSetterParameterTypes(property));
270+
}
267271
writeResolver.addToSymbolTable();
268272
}
269273

@@ -276,11 +280,17 @@ public void resolve(PropertyNode property) {
276280
if (stored != null && stored.getExpression() instanceof PrimaryExpressionNode) {
277281
NameResolver storedResolver = createNameResolver();
278282
storedResolver.readPrimaryExpression((PrimaryExpressionNode) stored.getExpression());
279-
storedResolver.disambiguateParameters(getStorageParameterTypes(property));
283+
if (isResolvingRoutine(storedResolver)) {
284+
storedResolver.disambiguateParameters(getStorageParameterTypes(property));
285+
}
280286
storedResolver.addToSymbolTable();
281287
}
282288
}
283289

290+
private static boolean isResolvingRoutine(NameResolver resolver) {
291+
return resolver.getDeclarations().stream().anyMatch(RoutineNameDeclaration.class::isInstance);
292+
}
293+
284294
private List<Type> getGetterParameterTypes(PropertyNode property) {
285295
if (property.getIndexSpecifier() == null) {
286296
return property.getParameterTypes();

0 commit comments

Comments
 (0)