Skip to content

Commit

Permalink
Extended CatchingGeneralExceptionRule to detect more use cases
Browse files Browse the repository at this point in the history
- except HandleException(); is now detected (#4)
- on Exception do is now also detected
  • Loading branch information
Artur Lipinski committed Jul 30, 2021
1 parent 81e5e91 commit 13be25a
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -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;

}
}
5 changes: 1 addition & 4 deletions src/main/resources/org/sonar/plugins/delphi/pmd/rules.xml
Original file line number Diff line number Diff line change
Expand Up @@ -279,17 +279,14 @@
</properties>
</rule>

<rule since="" class="org.sonar.plugins.delphi.pmd.rules.NodeSequenceRule" message="Catching general exception"
<rule since="" class="org.sonar.plugins.delphi.pmd.rules.CatchingGeneralExceptionRule" message="Catching general exception"
name="CatchingGeneralExceptionRule">
<description>You should not catch general exceptions but descendants of Exception class.
</description>
<properties>
<property name="baseEffort">
<value>15min</value>
</property>
<property name="sequence">
<value>:,exception,do</value>
</property>
</properties>
<example><![CDATA[
<b>except</b>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions src/test/resources/org/sonar/plugins/delphi/PMDTest/pmd.pas
Original file line number Diff line number Diff line change
Expand Up @@ -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.

0 comments on commit 13be25a

Please sign in to comment.