Skip to content

Commit e3743d7

Browse files
cirrasfourls
cirras
authored and
fourls
committed
Add excludedTypes property to ObjectPassedAsInterface
By default, we'll exclude some common types that are known not to use the standard ref-counting implementation.
1 parent 8130fea commit e3743d7

File tree

3 files changed

+82
-8
lines changed

3 files changed

+82
-8
lines changed

‎CHANGELOG.md‎

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

88
## [Unreleased]
99

10+
### Added
11+
12+
- `excludedTypes` rule property to the `ObjectPassedAsInterface` rule.
13+
14+
### Changed
15+
16+
- Exclude common non-ref-counted interface implementations by default in `ObjectPassedAsInterface`.
17+
1018
### Fixed
1119

1220
- Inaccurate type comparisons between class and interface types, where class-to-class upcasts were

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

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@
1818
*/
1919
package au.com.integradev.delphi.checks;
2020

21+
import com.google.common.base.Splitter;
2122
import java.util.Collections;
23+
import java.util.List;
2224
import java.util.Set;
2325
import java.util.stream.Collectors;
2426
import java.util.stream.IntStream;
2527
import org.sonar.check.Rule;
28+
import org.sonar.check.RuleProperty;
2629
import org.sonar.plugins.communitydelphi.api.ast.ArgumentListNode;
2730
import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode;
2831
import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode;
@@ -31,11 +34,31 @@
3134
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
3235
import org.sonar.plugins.communitydelphi.api.symbol.declaration.RoutineNameDeclaration;
3336
import org.sonar.plugins.communitydelphi.api.symbol.declaration.VariableNameDeclaration;
37+
import org.sonar.plugins.communitydelphi.api.type.Type;
3438

3539
@Rule(key = "ObjectPassedAsInterface")
3640
public class ObjectPassedAsInterfaceCheck extends DelphiCheck {
3741
private static final String MESSAGE = "Do not pass this object reference as an interface.";
3842

43+
private static final String DEFAULT_EXCLUDED_TYPES =
44+
"System.TNoRefCountObject,"
45+
+ "System.Generics.Defaults.TSingletonImplementation,"
46+
+ "System.Classes.TComponent";
47+
48+
@RuleProperty(
49+
key = "excludedTypes",
50+
description =
51+
"Comma-delimited list of object types that this rule ignores. (case-insensitive)",
52+
defaultValue = DEFAULT_EXCLUDED_TYPES)
53+
public String excludedTypes = DEFAULT_EXCLUDED_TYPES;
54+
55+
private List<String> excludedTypesList;
56+
57+
@Override
58+
public void start(DelphiCheckContext context) {
59+
excludedTypesList = Splitter.on(',').trimResults().splitToList(excludedTypes);
60+
}
61+
3962
@Override
4063
public DelphiCheckContext visit(ArgumentListNode argumentList, DelphiCheckContext context) {
4164
var interfaceIndices = getInterfaceParameterIndices(argumentList);
@@ -47,15 +70,15 @@ public DelphiCheckContext visit(ArgumentListNode argumentList, DelphiCheckContex
4770

4871
ExpressionNode expression = arguments.get(i).getExpression();
4972

50-
if (isVariableWithClassType(expression)) {
73+
if (isObjectReferenceExpression(expression)) {
5174
reportIssue(context, expression, MESSAGE);
5275
}
5376
}
5477

5578
return super.visit(argumentList, context);
5679
}
5780

58-
private static boolean isVariableWithClassType(ExpressionNode expression) {
81+
private boolean isObjectReferenceExpression(ExpressionNode expression) {
5982
expression = expression.skipParentheses();
6083

6184
if (!(expression instanceof PrimaryExpressionNode)) {
@@ -72,7 +95,10 @@ private static boolean isVariableWithClassType(ExpressionNode expression) {
7295
return false;
7396
}
7497

75-
return ((VariableNameDeclaration) declaration).getType().isClass();
98+
Type type = ((VariableNameDeclaration) declaration).getType();
99+
100+
return type.isClass()
101+
&& excludedTypesList.stream().noneMatch(e -> type.is(e) || type.isDescendantOf(e));
76102
}
77103

78104
private static Set<Integer> getInterfaceParameterIndices(ArgumentListNode argumentList) {

‎delphi-checks/src/test/java/au/com/integradev/delphi/checks/ObjectPassedAsInterfaceCheckTest.java‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ void testObjectPassedAsObjectShouldNotAddIssue() {
3232
.appendDecl("type")
3333
.appendDecl(" IFooIntf = interface")
3434
.appendDecl(" end;")
35-
.appendDecl(" TFooImpl = class(TObject, IFooIntf)")
35+
.appendDecl(" TFooImpl = class(TInterfacedObject, IFooIntf)")
3636
.appendDecl(" end;")
3737
.appendDecl("procedure DoThing(Obj: TFooImpl);")
3838
.appendImpl("procedure Test;")
@@ -54,7 +54,7 @@ void testObjectPassedAsInterfaceShouldAddIssue() {
5454
.appendDecl("type")
5555
.appendDecl(" IFooIntf = interface")
5656
.appendDecl(" end;")
57-
.appendDecl(" TFooImpl = class(TObject, IFooIntf)")
57+
.appendDecl(" TFooImpl = class(TInterfacedObject, IFooIntf)")
5858
.appendDecl(" end;")
5959
.appendDecl("procedure DoThing(Obj: IFooIntf);")
6060
.appendImpl("procedure Test;")
@@ -114,7 +114,7 @@ void testObjectCastToInterfaceShouldNotAddIssue() {
114114
.appendDecl("type")
115115
.appendDecl(" IFooIntf = interface")
116116
.appendDecl(" end;")
117-
.appendDecl(" TFooImpl = class(TObject, IFooIntf)")
117+
.appendDecl(" TFooImpl = class(TInterfacedObject, IFooIntf)")
118118
.appendDecl(" end;")
119119
.appendDecl("procedure DoThing(Obj: IFooIntf);")
120120
.appendImpl("procedure Test;")
@@ -136,7 +136,7 @@ void testNewObjectPassedAsInterfaceShouldNotAddIssue() {
136136
.appendDecl("type")
137137
.appendDecl(" IFooIntf = interface")
138138
.appendDecl(" end;")
139-
.appendDecl(" TFooImpl = class(TObject, IFooIntf)")
139+
.appendDecl(" TFooImpl = class(TInterfacedObject, IFooIntf)")
140140
.appendDecl(" end;")
141141
.appendDecl("procedure DoThing(Obj: IFooIntf);")
142142
.appendImpl("procedure Test;")
@@ -155,7 +155,7 @@ void testObjectPassedAsInterfaceToInheritedShouldAddIssue() {
155155
.appendDecl("type")
156156
.appendDecl(" IFooIntf = interface")
157157
.appendDecl(" end;")
158-
.appendDecl(" TFooParent = class(TObject)")
158+
.appendDecl(" TFooParent = class(TInterfacedObject)")
159159
.appendDecl(" procedure Bar(Foo: IFooIntf); virtual;")
160160
.appendDecl(" end;")
161161
.appendDecl(" TFooImpl = class(TFooParent, IFooIntf)")
@@ -170,4 +170,44 @@ void testObjectPassedAsInterfaceToInheritedShouldAddIssue() {
170170
.appendImpl("end;"))
171171
.verifyIssues();
172172
}
173+
174+
@Test
175+
void testExcludedTypePassedAsInterfaceShouldNotAddIssue() {
176+
CheckVerifier.newVerifier()
177+
.withCheck(new ObjectPassedAsInterfaceCheck())
178+
.onFile(
179+
new DelphiTestUnitBuilder()
180+
.appendDecl("uses")
181+
.appendDecl(" System.Classes;")
182+
.appendDecl("procedure DoThing(Obj: IInterface);")
183+
.appendImpl("procedure Test(Obj: TComponent);")
184+
.appendImpl("begin")
185+
.appendImpl(" DoThing(Obj);")
186+
.appendImpl("end;"))
187+
.verifyNoIssues();
188+
}
189+
190+
@Test
191+
void testExcludedTypeDescendentPassedAsInterfaceShouldNotAddIssue() {
192+
CheckVerifier.newVerifier()
193+
.withCheck(new ObjectPassedAsInterfaceCheck())
194+
.onFile(
195+
new DelphiTestUnitBuilder()
196+
.appendDecl("uses")
197+
.appendDecl(" System.Classes;")
198+
.appendDecl("type")
199+
.appendDecl(" IFooIntf = interface")
200+
.appendDecl(" end;")
201+
.appendDecl(" TFooImpl = class(TComponent, IFooIntf)")
202+
.appendDecl(" end;")
203+
.appendDecl("procedure DoThing(Obj: IFooIntf);")
204+
.appendImpl("procedure Test;")
205+
.appendImpl("var")
206+
.appendImpl(" Obj: TFooImpl;")
207+
.appendImpl("begin")
208+
.appendImpl(" Obj := TFooImpl.Create;")
209+
.appendImpl(" DoThing(Obj);")
210+
.appendImpl("end;"))
211+
.verifyNoIssues();
212+
}
173213
}

0 commit comments

Comments
 (0)