Skip to content

Commit 48b50a6

Browse files
committed
Fix incorrect type resolution around array property accesses
#313 introduced a regression in type resolution for plain array properties. We started erroneously calling `addResolvedDeclaration()` before resolving array properties, where before we would exit immediately and return to argument disambiguation on the array accessor. The effect was a subtle breakage of the type resolution around array properties. Take, for example, the expression `Foo.Bar[0]`, where `Bar` is an array property returning `string`. The old behavior would yield type `string`, while the new behavior would yield type `Char` due to the premature `addResolvedDeclaration()` call updating the `currentType` to `string` before the array accessor was processed.
1 parent 6d8ffb2 commit 48b50a6

File tree

5 files changed

+102
-18
lines changed

5 files changed

+102
-18
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Various intrinsic routines had incorrect signatures around dynamic and open arrays.
1313
- False positives around platform-dependent binary expressions in `PlatformDependentTruncation`.
14+
- Incorrect type resolution around array property accesses.
1415

1516
## [1.12.0] - 2024-12-02
1617

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

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -782,26 +782,31 @@ private void handleParenthesizedExpression(ParenthesizedExpressionNode parenthes
782782
}
783783
}
784784

785-
private boolean handleDefaultArrayProperties(ArrayAccessorNode accessor) {
786-
if (!declarations.isEmpty()
787-
&& declarations.stream().allMatch(NameResolver::isDefaultArrayProperty)) {
788-
Type type = currentType;
789-
if (type.isClassReference()) {
790-
type = ((ClassReferenceType) type).classType();
791-
} else if (type.isProcedural()) {
792-
type = ((ProceduralType) type).returnType();
793-
}
785+
private void addExplicitDefaultArrayPropertyDeclarations() {
786+
if (!declarations.stream().allMatch(NameResolver::isDefaultArrayProperty)) {
787+
return;
788+
}
794789

795-
if (type.isStruct()) {
796-
StructType structType = (StructType) TypeUtils.findBaseType(type);
797-
NameDeclaration declaration = Iterables.getLast(declarations);
798-
((StructTypeImpl) structType)
799-
.findDefaultArrayProperties().stream()
800-
.filter(property -> property.getName().equalsIgnoreCase(declaration.getName()))
801-
.forEach(declarations::add);
802-
}
790+
Type type = currentType;
791+
if (type.isClassReference()) {
792+
type = ((ClassReferenceType) type).classType();
793+
} else if (type.isProcedural()) {
794+
type = ((ProceduralType) type).returnType();
795+
}
803796

804-
// An explicit array property access can be handled by argument disambiguation.
797+
if (type.isStruct()) {
798+
StructType structType = (StructType) TypeUtils.findBaseType(type);
799+
NameDeclaration declaration = Iterables.getLast(declarations);
800+
((StructTypeImpl) structType)
801+
.findDefaultArrayProperties().stream()
802+
.filter(property -> property.getName().equalsIgnoreCase(declaration.getName()))
803+
.forEach(declarations::add);
804+
}
805+
}
806+
807+
private boolean handleDefaultArrayProperties(ArrayAccessorNode accessor) {
808+
if (!declarations.isEmpty() && isArrayProperty(Iterables.getLast(declarations))) {
809+
addExplicitDefaultArrayPropertyDeclarations();
805810
return false;
806811
}
807812

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,20 @@ void testHiddenDefaultProperties() {
870870
verifyUsages(11, 14, reference(27, 25));
871871
}
872872

873+
@Test
874+
void testArrayProperties() {
875+
execute("properties/ArrayProperties.pas");
876+
verifyUsages(9, 13, reference(29, 23));
877+
verifyUsages(16, 10, reference(28, 2));
878+
}
879+
880+
@Test
881+
void testClassArrayProperties() {
882+
execute("properties/ClassArrayProperties.pas");
883+
verifyUsages(9, 13, reference(29, 24));
884+
verifyUsages(16, 10, reference(28, 2));
885+
}
886+
873887
@Test
874888
void testDefaultArrayProperties() {
875889
execute("properties/DefaultArrayProperties.pas");
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
unit ArrayProperties;
2+
3+
interface
4+
5+
implementation
6+
7+
type
8+
TStringHelper = record helper for string
9+
function IsEmpty: Boolean;
10+
end;
11+
12+
TFoo = class
13+
property Baz[I: Integer]: string;
14+
end;
15+
16+
procedure Consume(S: string);
17+
begin
18+
// do nothing
19+
end;
20+
21+
procedure Consume(C: Char);
22+
begin
23+
// do nothing
24+
end;
25+
26+
function Test(Foo: TFoo): Boolean;
27+
begin
28+
Consume(Foo.Baz[0]);
29+
Result := Foo.Baz[0].IsEmpty;
30+
end;
31+
32+
end.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
unit ClassArrayProperties;
2+
3+
interface
4+
5+
implementation
6+
7+
type
8+
TStringHelper = record helper for string
9+
function IsEmpty: Boolean;
10+
end;
11+
12+
TFoo = class
13+
class property Baz[I: Integer]: string;
14+
end;
15+
16+
procedure Consume(S: string);
17+
begin
18+
// do nothing
19+
end;
20+
21+
procedure Consume(C: Char);
22+
begin
23+
// do nothing
24+
end;
25+
26+
function Test(Foo: TFoo): Boolean;
27+
begin
28+
Consume(TFoo.Baz[0]);
29+
Result := TFoo.Baz[0].IsEmpty;
30+
end;
31+
32+
end.

0 commit comments

Comments
 (0)