diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/MissingRaiseCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/MissingRaiseCheck.java index 46c5ba3ac..758d4ea74 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/MissingRaiseCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/MissingRaiseCheck.java @@ -55,30 +55,54 @@ public DelphiCheckContext visit( return context; } - private boolean isExceptionConstructorInvocation(ExpressionNode expression) { - if (!(expression instanceof PrimaryExpressionNode) - || !(expression.getChild(0) instanceof NameReferenceNode)) { - return false; - } + private boolean isExceptionType(Type type) { + return type.is(BASE_EXCEPTION) || type.isDescendantOf(BASE_EXCEPTION); + } - NameDeclaration declaration = - ((NameReferenceNode) expression.getChild(0)).getLastName().getNameDeclaration(); + private RoutineNameDeclaration getConstructor(NameReferenceNode nameReference) { + NameDeclaration declaration = nameReference.getNameDeclaration(); if (!(declaration instanceof RoutineNameDeclaration)) { - return false; + // Not an invocation + return null; } RoutineNameDeclaration routineDeclaration = (RoutineNameDeclaration) declaration; - if (routineDeclaration.getRoutineKind() != RoutineKind.CONSTRUCTOR) { + return routineDeclaration.getRoutineKind() == RoutineKind.CONSTRUCTOR + ? routineDeclaration + : null; + } + + private Type getType(NameReferenceNode nameReference) { + NameDeclaration declaration = nameReference.getNameDeclaration(); + if (!(declaration instanceof TypeNameDeclaration)) { + // Type could not be resolved + return null; + } + + return ((TypeNameDeclaration) declaration).getType(); + } + + private boolean isExceptionConstructorInvocation(ExpressionNode expression) { + if (!(expression instanceof PrimaryExpressionNode) + || !(expression.getChild(0) instanceof NameReferenceNode)) { + // Not a name reference return false; } - TypeNameDeclaration typeDecl = routineDeclaration.getTypeDeclaration(); - if (typeDecl == null) { + NameReferenceNode lastName = ((NameReferenceNode) expression.getChild(0)).getLastName(); + NameReferenceNode prevName = lastName.prevName(); + if (prevName == null) { + // Not a qualified reference return false; } - Type type = typeDecl.getType(); + RoutineNameDeclaration constructorDeclaration = getConstructor(lastName); + if (constructorDeclaration == null) { + // Not a constructor + return false; + } - return type.is(BASE_EXCEPTION) || type.isDescendantOf(BASE_EXCEPTION); + Type constructingType = getType(prevName); + return constructingType != null && isExceptionType(constructingType); } } diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/MissingRaiseCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/MissingRaiseCheckTest.java index 61544be94..c4d05274d 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/MissingRaiseCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/MissingRaiseCheckTest.java @@ -179,4 +179,172 @@ void testAssignedDescendantExceptionShouldNotAddIssue() { .appendImpl(" MyError := ECalculatorError.Create(1, 2);")) .verifyNoIssues(); } + + @Test + void testImplicitSelfCallInConstructorShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new MissingRaiseCheck()) + .withStandardLibraryUnit(sysUtils()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses System.SysUtils;") + .appendDecl("type") + .appendDecl(" ECalculatorError = class(Exception)") + .appendDecl(" public") + .appendDecl(" constructor Create(A: Integer; B: Integer);") + .appendDecl(" constructor CreateFmt(A: Integer; B: Integer);") + .appendDecl(" end;") + .appendImpl("constructor ECalculatorError.CreateFmt(A: Integer; B: Integer);") + .appendImpl("begin") + .appendImpl(" CreateFmt(A, B);") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @Test + void testExplicitSelfCallInConstructorShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new MissingRaiseCheck()) + .withStandardLibraryUnit(sysUtils()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses System.SysUtils;") + .appendDecl("type") + .appendDecl(" ECalculatorError = class(Exception)") + .appendDecl(" public") + .appendDecl(" constructor Create(A: Integer; B: Integer);") + .appendDecl(" constructor CreateFmt(A: Integer; B: Integer);") + .appendDecl(" end;") + .appendImpl("constructor ECalculatorError.CreateFmt(A: Integer; B: Integer);") + .appendImpl("begin") + .appendImpl(" Self.CreateFmt(A, B);") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @Test + void testSelfAncestorCallInMethodShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new MissingRaiseCheck()) + .withStandardLibraryUnit(sysUtils()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses System.SysUtils;") + .appendDecl("type") + .appendDecl(" ECalculatorError = class(Exception)") + .appendDecl(" public") + .appendDecl(" function Add(A: Integer; B: Integer): Integer;") + .appendDecl(" constructor Create(A: Integer; B: Integer);") + .appendDecl(" end;") + .appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;") + .appendImpl("begin") + .appendImpl(" Create('foo');") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @Test + void testNewInstanceOfAncestorTypeInMethodShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new MissingRaiseCheck()) + .withStandardLibraryUnit(sysUtils()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses System.SysUtils;") + .appendDecl("type") + .appendDecl(" ECalculatorError = class(Exception)") + .appendDecl(" public") + .appendDecl(" function Add(A: Integer; B: Integer): Integer;") + .appendDecl(" constructor Create(A: Integer; B: Integer);") + .appendDecl(" end;") + .appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;") + .appendImpl("begin") + .appendImpl(" Exception.Create('foo'); // Noncompliant") + .appendImpl("end;")) + .verifyIssues(); + } + + @Test + void testImplicitSelfCallInMethodShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new MissingRaiseCheck()) + .withStandardLibraryUnit(sysUtils()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses System.SysUtils;") + .appendDecl("type") + .appendDecl(" ECalculatorError = class(Exception)") + .appendDecl(" public") + .appendDecl(" function Add(A: Integer; B: Integer): Integer;") + .appendDecl(" constructor Create(A: Integer; B: Integer);") + .appendDecl(" end;") + .appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;") + .appendImpl("begin") + .appendImpl(" Create(A, B);") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @Test + void testExplicitSelfCallInMethodShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new MissingRaiseCheck()) + .withStandardLibraryUnit(sysUtils()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses System.SysUtils;") + .appendDecl("type") + .appendDecl(" ECalculatorError = class(Exception)") + .appendDecl(" public") + .appendDecl(" function Add(A: Integer; B: Integer): Integer;") + .appendDecl(" constructor Create(A: Integer; B: Integer);") + .appendDecl(" end;") + .appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;") + .appendImpl("begin") + .appendImpl(" Self.Create(A, B);") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @Test + void testNewInstanceOfSelfTypeInMethodShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new MissingRaiseCheck()) + .withStandardLibraryUnit(sysUtils()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses System.SysUtils;") + .appendDecl("type") + .appendDecl(" ECalculatorError = class(Exception)") + .appendDecl(" public") + .appendDecl(" function Add(A: Integer; B: Integer): Integer;") + .appendDecl(" constructor Create(A: Integer; B: Integer);") + .appendDecl(" end;") + .appendImpl("function ECalculatorError.Add(A: Integer; B: Integer): Integer;") + .appendImpl("begin") + .appendImpl(" ECalculatorError.Create(A, B); // Noncompliant") + .appendImpl("end;")) + .verifyIssues(); + } + + @Test + void testNewInstanceOfSelfTypeInConstructorShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new MissingRaiseCheck()) + .withStandardLibraryUnit(sysUtils()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("uses System.SysUtils;") + .appendDecl("type") + .appendDecl(" ECalculatorError = class(Exception)") + .appendDecl(" public") + .appendDecl(" constructor CreateFmt(A: Integer; B: Integer);") + .appendDecl(" constructor Create(A: Integer; B: Integer);") + .appendDecl(" end;") + .appendImpl("constructor ECalculatorError.CreateFmt(A: Integer; B: Integer);") + .appendImpl("begin") + .appendImpl(" ECalculatorError.Create(A, B); // Noncompliant") + .appendImpl("end;")) + .verifyIssues(); + } }