From 13be25a8ca2c521b09346b6b8b7ab141f782e818 Mon Sep 17 00:00:00 2001 From: Artur Lipinski Date: Fri, 30 Jul 2021 14:04:48 +0200 Subject: [PATCH] Extended CatchingGeneralExceptionRule to detect more use cases - except HandleException(); is now detected (#4) - on Exception do is now also detected --- .../rules/CatchingGeneralExceptionRule.java | 65 +++++++++++++++++++ .../org/sonar/plugins/delphi/pmd/rules.xml | 5 +- .../delphi/pmd/DelphiPmdSensorTest.java | 12 +++- .../org/sonar/plugins/delphi/PMDTest/pmd.pas | 60 +++++++++++++++++ 4 files changed, 136 insertions(+), 6 deletions(-) create mode 100644 src/main/java/org/sonar/plugins/delphi/pmd/rules/CatchingGeneralExceptionRule.java diff --git a/src/main/java/org/sonar/plugins/delphi/pmd/rules/CatchingGeneralExceptionRule.java b/src/main/java/org/sonar/plugins/delphi/pmd/rules/CatchingGeneralExceptionRule.java new file mode 100644 index 0000000..6f1ec79 --- /dev/null +++ b/src/main/java/org/sonar/plugins/delphi/pmd/rules/CatchingGeneralExceptionRule.java @@ -0,0 +1,65 @@ +package org.sonar.plugins.delphi.pmd.rules; + +import net.sourceforge.pmd.RuleContext; +import org.sonar.plugins.delphi.antlr.DelphiLexer; +import org.sonar.plugins.delphi.antlr.ast.DelphiNode; +import org.sonar.plugins.delphi.antlr.ast.DelphiPMDNode; + +public class CatchingGeneralExceptionRule extends DelphiRule { + + public CatchingGeneralExceptionRule() + { + } + + @Override + public void visit(DelphiPMDNode node, RuleContext ctx) { + + + int a = node.getLine(); + // Skip if not an except block + if (node.getType() != DelphiLexer.EXCEPT) + return; + + int childIndexDelta = 1; + DelphiPMDNode parent = (DelphiPMDNode) node.getParent(); + DelphiPMDNode nextNode = (DelphiPMDNode) parent.getChild(node.getChildIndex() + childIndexDelta); + + // ... except HandleException ... + if (nextNode.getType() != DelphiLexer.ON) { + addViolation(ctx, node); + return; + } + + DelphiPMDNode currentNode; + do { + currentNode = (DelphiPMDNode) parent.getChild(node.getChildIndex() + childIndexDelta); + if (currentNode.getType() == DelphiLexer.ON) { + // ... on Exception ... + nextNode = (DelphiPMDNode) parent.getChild(currentNode.getChildIndex() + 1); + if (nextNode.getType() == DelphiLexer.TkIdentifier && nextNode.getText().equals("Exception")) { + addViolation(ctx, nextNode); + return; + } + + // ... on E : Exception ... + nextNode = (DelphiPMDNode) parent.getChild(currentNode.getChildIndex() + 3); + if (nextNode.getType() == DelphiLexer.TkIdentifier && nextNode.getText().equals("Exception")) { + addViolation(ctx, nextNode); + return; + } + } + + childIndexDelta++; + } while (currentNode.getType() != DelphiLexer.END); + + // try + // ... + // except + // on EZeroDivide do HandleZeroDivide; + // on EOverflow do HandleOverflow; + // else + // HandleAllOthers; + // end; + + } +} \ No newline at end of file diff --git a/src/main/resources/org/sonar/plugins/delphi/pmd/rules.xml b/src/main/resources/org/sonar/plugins/delphi/pmd/rules.xml index c9b37a5..a4414c9 100644 --- a/src/main/resources/org/sonar/plugins/delphi/pmd/rules.xml +++ b/src/main/resources/org/sonar/plugins/delphi/pmd/rules.xml @@ -279,7 +279,7 @@ - You should not catch general exceptions but descendants of Exception class. @@ -287,9 +287,6 @@ 15min - - :,exception,do - except diff --git a/src/test/java/org/sonar/plugins/delphi/pmd/DelphiPmdSensorTest.java b/src/test/java/org/sonar/plugins/delphi/pmd/DelphiPmdSensorTest.java index 43fc655..cbff9fe 100644 --- a/src/test/java/org/sonar/plugins/delphi/pmd/DelphiPmdSensorTest.java +++ b/src/test/java/org/sonar/plugins/delphi/pmd/DelphiPmdSensorTest.java @@ -118,7 +118,8 @@ public void analyseTest() { sensor.execute(sensorContext); RuleData ruleData[] = // all expected rule violations and their lines - {new RuleData("ClassNameRule", 7), + { + new RuleData("ClassNameRule", 7), new RuleData("NoSemiAfterOverloadRule", 9), new RuleData("TypeAliasRule", 13), new RuleData("TypeAliasRule", 14), @@ -129,6 +130,7 @@ public void analyseTest() { new RuleData("RecordNameRule", 34), new RuleData("InheritedMethodWithNoCodeRule", 45), new RuleData("ThenTryRule", 51), + new RuleData("CatchingGeneralExceptionRule", 54), new RuleData("EmptyExceptBlockRule", 54), new RuleData("TooManyArgumentsRule", 58), new RuleData("TooManyVariablesRule", 59), @@ -159,7 +161,13 @@ public void analyseTest() { new RuleData("NoSemicolonRule", 291), new RuleData("NoSemicolonRule", 293), new RuleData("CastAndFreeRule", 302), - new RuleData("CastAndFreeRule", 303)}; + new RuleData("CastAndFreeRule", 303), + new RuleData("CatchingGeneralExceptionRule", 309), + new RuleData("CatchingGeneralExceptionRule", 318), + new RuleData("CatchingGeneralExceptionRule", 329), + new RuleData("CatchingGeneralExceptionRule", 344), + new RuleData("CatchingGeneralExceptionRule", 359), + }; // Sort the violations by line number, so we don't have to add // violations order diff --git a/src/test/resources/org/sonar/plugins/delphi/PMDTest/pmd.pas b/src/test/resources/org/sonar/plugins/delphi/PMDTest/pmd.pas index f170f3b..7ba2635 100644 --- a/src/test/resources/org/sonar/plugins/delphi/PMDTest/pmd.pas +++ b/src/test/resources/org/sonar/plugins/delphi/PMDTest/pmd.pas @@ -303,4 +303,64 @@ procedure FreeAfterCast(Sender: TObject); //dont cast then free (myClass as TMyClass).free(); //violation end; +procedure ImplicitGeneralException(); +begin + try + except + HandleException; + end; +end; + +procedure ExplicitGeneralExceptionWithoutVariable(); +begin + try + except + on Exception do + begin + HandleException; + end; + end; +end; + +procedure ExplicitGeneralExceptionWithVariable(); +begin + try + except + on e: Exception do + begin + HandleException; + end; + end; +end; + +procedure SequenceOfExceptionHandlersWithExplicitGeneralExceptionWithoutVariable(); +begin + try + except + on EZeroDivide do + begin + HandleZeroDivide; + end; + on Exception do + begin + HandleException; + end; + end; +end; + +procedure SequenceOfExceptionHandlersWithExplicitGeneralExceptionWithoutVariable(); +begin + try + except + on z: EZeroDivide do + begin + HandleZeroDivide; + end; + on e: Exception do + begin + HandleException; + end; + end; +end; + end. \ No newline at end of file