Skip to content

Commit 8ad1f9f

Browse files
fourlsfourls
committed
Fix MissingRaise false positives on Self constructor calls
1 parent 847e4b2 commit 8ad1f9f

File tree

2 files changed

+205
-13
lines changed

2 files changed

+205
-13
lines changed

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

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,30 +55,54 @@ public DelphiCheckContext visit(
5555
return context;
5656
}
5757

58-
private boolean isExceptionConstructorInvocation(ExpressionNode expression) {
59-
if (!(expression instanceof PrimaryExpressionNode)
60-
|| !(expression.getChild(0) instanceof NameReferenceNode)) {
61-
return false;
62-
}
58+
private boolean isExceptionType(Type type) {
59+
return type.is(BASE_EXCEPTION) || type.isDescendantOf(BASE_EXCEPTION);
60+
}
6361

64-
NameDeclaration declaration =
65-
((NameReferenceNode) expression.getChild(0)).getLastName().getNameDeclaration();
62+
private RoutineNameDeclaration getConstructor(NameReferenceNode nameReference) {
63+
NameDeclaration declaration = nameReference.getNameDeclaration();
6664
if (!(declaration instanceof RoutineNameDeclaration)) {
67-
return false;
65+
// Not an invocation
66+
return null;
6867
}
6968

7069
RoutineNameDeclaration routineDeclaration = (RoutineNameDeclaration) declaration;
71-
if (routineDeclaration.getRoutineKind() != RoutineKind.CONSTRUCTOR) {
70+
return routineDeclaration.getRoutineKind() == RoutineKind.CONSTRUCTOR
71+
? routineDeclaration
72+
: null;
73+
}
74+
75+
private Type getType(NameReferenceNode nameReference) {
76+
NameDeclaration declaration = nameReference.getNameDeclaration();
77+
if (!(declaration instanceof TypeNameDeclaration)) {
78+
// Type could not be resolved
79+
return null;
80+
}
81+
82+
return ((TypeNameDeclaration) declaration).getType();
83+
}
84+
85+
private boolean isExceptionConstructorInvocation(ExpressionNode expression) {
86+
if (!(expression instanceof PrimaryExpressionNode)
87+
|| !(expression.getChild(0) instanceof NameReferenceNode)) {
88+
// Not a name reference
7289
return false;
7390
}
7491

75-
TypeNameDeclaration typeDecl = routineDeclaration.getTypeDeclaration();
76-
if (typeDecl == null) {
92+
NameReferenceNode lastName = ((NameReferenceNode) expression.getChild(0)).getLastName();
93+
NameReferenceNode prevName = lastName.prevName();
94+
if (prevName == null) {
95+
// Not a qualified reference
7796
return false;
7897
}
7998

80-
Type type = typeDecl.getType();
99+
RoutineNameDeclaration constructorDeclaration = getConstructor(lastName);
100+
if (constructorDeclaration == null) {
101+
// Not a constructor
102+
return false;
103+
}
81104

82-
return type.is(BASE_EXCEPTION) || type.isDescendantOf(BASE_EXCEPTION);
105+
Type constructingType = getType(prevName);
106+
return constructingType != null && isExceptionType(constructingType);
83107
}
84108
}

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

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,4 +179,172 @@ void testAssignedDescendantExceptionShouldNotAddIssue() {
179179
.appendImpl(" MyError := ECalculatorError.Create(1, 2);"))
180180
.verifyNoIssues();
181181
}
182+
183+
@Test
184+
void testImplicitSelfCallInConstructorShouldNotAddIssue() {
185+
CheckVerifier.newVerifier()
186+
.withCheck(new MissingRaiseCheck())
187+
.withStandardLibraryUnit(sysUtils())
188+
.onFile(
189+
new DelphiTestUnitBuilder()
190+
.appendDecl("uses System.SysUtils;")
191+
.appendDecl("type")
192+
.appendDecl(" ECalculatorError = class(Exception)")
193+
.appendDecl(" public")
194+
.appendDecl(" constructor Create(A: Integer; B: Integer);")
195+
.appendDecl(" constructor CreateFmt(A: Integer; B: Integer);")
196+
.appendDecl(" end;")
197+
.appendImpl("constructor ECalculatorError.CreateFmt(A: Integer; B: Integer);")
198+
.appendImpl("begin")
199+
.appendImpl(" CreateFmt(A, B);")
200+
.appendImpl("end;"))
201+
.verifyNoIssues();
202+
}
203+
204+
@Test
205+
void testExplicitSelfCallInConstructorShouldNotAddIssue() {
206+
CheckVerifier.newVerifier()
207+
.withCheck(new MissingRaiseCheck())
208+
.withStandardLibraryUnit(sysUtils())
209+
.onFile(
210+
new DelphiTestUnitBuilder()
211+
.appendDecl("uses System.SysUtils;")
212+
.appendDecl("type")
213+
.appendDecl(" ECalculatorError = class(Exception)")
214+
.appendDecl(" public")
215+
.appendDecl(" constructor Create(A: Integer; B: Integer);")
216+
.appendDecl(" constructor CreateFmt(A: Integer; B: Integer);")
217+
.appendDecl(" end;")
218+
.appendImpl("constructor ECalculatorError.CreateFmt(A: Integer; B: Integer);")
219+
.appendImpl("begin")
220+
.appendImpl(" Self.CreateFmt(A, B);")
221+
.appendImpl("end;"))
222+
.verifyNoIssues();
223+
}
224+
225+
@Test
226+
void testSelfAncestorCallInMethodShouldNotAddIssue() {
227+
CheckVerifier.newVerifier()
228+
.withCheck(new MissingRaiseCheck())
229+
.withStandardLibraryUnit(sysUtils())
230+
.onFile(
231+
new DelphiTestUnitBuilder()
232+
.appendDecl("uses System.SysUtils;")
233+
.appendDecl("type")
234+
.appendDecl(" ECalculatorError = class(Exception)")
235+
.appendDecl(" public")
236+
.appendDecl(" function Add(A: Integer; B: Integer): Integer;")
237+
.appendDecl(" constructor Create(A: Integer; B: Integer);")
238+
.appendDecl(" end;")
239+
.appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;")
240+
.appendImpl("begin")
241+
.appendImpl(" Create('foo');")
242+
.appendImpl("end;"))
243+
.verifyNoIssues();
244+
}
245+
246+
@Test
247+
void testNewInstanceOfAncestorTypeInMethodShouldAddIssue() {
248+
CheckVerifier.newVerifier()
249+
.withCheck(new MissingRaiseCheck())
250+
.withStandardLibraryUnit(sysUtils())
251+
.onFile(
252+
new DelphiTestUnitBuilder()
253+
.appendDecl("uses System.SysUtils;")
254+
.appendDecl("type")
255+
.appendDecl(" ECalculatorError = class(Exception)")
256+
.appendDecl(" public")
257+
.appendDecl(" function Add(A: Integer; B: Integer): Integer;")
258+
.appendDecl(" constructor Create(A: Integer; B: Integer);")
259+
.appendDecl(" end;")
260+
.appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;")
261+
.appendImpl("begin")
262+
.appendImpl(" Exception.Create('foo'); // Noncompliant")
263+
.appendImpl("end;"))
264+
.verifyIssues();
265+
}
266+
267+
@Test
268+
void testImplicitSelfCallInMethodShouldNotAddIssue() {
269+
CheckVerifier.newVerifier()
270+
.withCheck(new MissingRaiseCheck())
271+
.withStandardLibraryUnit(sysUtils())
272+
.onFile(
273+
new DelphiTestUnitBuilder()
274+
.appendDecl("uses System.SysUtils;")
275+
.appendDecl("type")
276+
.appendDecl(" ECalculatorError = class(Exception)")
277+
.appendDecl(" public")
278+
.appendDecl(" function Add(A: Integer; B: Integer): Integer;")
279+
.appendDecl(" constructor Create(A: Integer; B: Integer);")
280+
.appendDecl(" end;")
281+
.appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;")
282+
.appendImpl("begin")
283+
.appendImpl(" Create(A, B);")
284+
.appendImpl("end;"))
285+
.verifyNoIssues();
286+
}
287+
288+
@Test
289+
void testExplicitSelfCallInMethodShouldNotAddIssue() {
290+
CheckVerifier.newVerifier()
291+
.withCheck(new MissingRaiseCheck())
292+
.withStandardLibraryUnit(sysUtils())
293+
.onFile(
294+
new DelphiTestUnitBuilder()
295+
.appendDecl("uses System.SysUtils;")
296+
.appendDecl("type")
297+
.appendDecl(" ECalculatorError = class(Exception)")
298+
.appendDecl(" public")
299+
.appendDecl(" function Add(A: Integer; B: Integer): Integer;")
300+
.appendDecl(" constructor Create(A: Integer; B: Integer);")
301+
.appendDecl(" end;")
302+
.appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;")
303+
.appendImpl("begin")
304+
.appendImpl(" Self.Create(A, B);")
305+
.appendImpl("end;"))
306+
.verifyNoIssues();
307+
}
308+
309+
@Test
310+
void testNewInstanceOfSelfTypeInMethodShouldAddIssue() {
311+
CheckVerifier.newVerifier()
312+
.withCheck(new MissingRaiseCheck())
313+
.withStandardLibraryUnit(sysUtils())
314+
.onFile(
315+
new DelphiTestUnitBuilder()
316+
.appendDecl("uses System.SysUtils;")
317+
.appendDecl("type")
318+
.appendDecl(" ECalculatorError = class(Exception)")
319+
.appendDecl(" public")
320+
.appendDecl(" function Add(A: Integer; B: Integer): Integer;")
321+
.appendDecl(" constructor Create(A: Integer; B: Integer);")
322+
.appendDecl(" end;")
323+
.appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;")
324+
.appendImpl("begin")
325+
.appendImpl(" ECalculatorError.Create(A, B); // Noncompliant")
326+
.appendImpl("end;"))
327+
.verifyIssues();
328+
}
329+
330+
@Test
331+
void testNewInstanceOfSelfTypeInConstructorShouldAddIssue() {
332+
CheckVerifier.newVerifier()
333+
.withCheck(new MissingRaiseCheck())
334+
.withStandardLibraryUnit(sysUtils())
335+
.onFile(
336+
new DelphiTestUnitBuilder()
337+
.appendDecl("uses System.SysUtils;")
338+
.appendDecl("type")
339+
.appendDecl(" ECalculatorError = class(Exception)")
340+
.appendDecl(" public")
341+
.appendDecl(" constructor CreateFmt(A: Integer; B: Integer);")
342+
.appendDecl(" constructor Create(A: Integer; B: Integer);")
343+
.appendDecl(" end;")
344+
.appendImpl("constructor ECalculatorError.CreateFmt(A: Integer; B: Integer);")
345+
.appendImpl("begin")
346+
.appendImpl(" ECalculatorError.Create(A, B); // Noncompliant")
347+
.appendImpl("end;"))
348+
.verifyIssues();
349+
}
182350
}

0 commit comments

Comments
 (0)