Skip to content

Commit 14e88d5

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 14e88d5

File tree

5 files changed

+96
-4
lines changed

5 files changed

+96
-4
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: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -782,9 +782,17 @@ private void handleParenthesizedExpression(ParenthesizedExpressionNode parenthes
782782
}
783783
}
784784

785-
private boolean handleDefaultArrayProperties(ArrayAccessorNode accessor) {
786-
if (!declarations.isEmpty()
787-
&& declarations.stream().allMatch(NameResolver::isDefaultArrayProperty)) {
785+
private boolean handleExplicitArrayProperties() {
786+
if (declarations.isEmpty()) {
787+
return false;
788+
}
789+
790+
NameDeclaration declaration = Iterables.getLast(declarations);
791+
if (!isArrayProperty(declaration)) {
792+
return false;
793+
}
794+
795+
if (declarations.stream().allMatch(NameResolver::isDefaultArrayProperty)) {
788796
Type type = currentType;
789797
if (type.isClassReference()) {
790798
type = ((ClassReferenceType) type).classType();
@@ -794,13 +802,18 @@ private boolean handleDefaultArrayProperties(ArrayAccessorNode accessor) {
794802

795803
if (type.isStruct()) {
796804
StructType structType = (StructType) TypeUtils.findBaseType(type);
797-
NameDeclaration declaration = Iterables.getLast(declarations);
798805
((StructTypeImpl) structType)
799806
.findDefaultArrayProperties().stream()
800807
.filter(property -> property.getName().equalsIgnoreCase(declaration.getName()))
801808
.forEach(declarations::add);
802809
}
810+
}
811+
812+
return true;
813+
}
803814

815+
private boolean handleDefaultArrayProperties(ArrayAccessorNode accessor) {
816+
if (handleExplicitArrayProperties()) {
804817
// An explicit array property access can be handled by argument disambiguation.
805818
return false;
806819
}

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)