diff --git a/CHANGELOG.md b/CHANGELOG.md index 31a514bf2..74bda66d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Library path (`DelphiLibraryPath`/`DelphiTranslatedLibraryPath`) - Browsing path (`DelphiBrowsingPath`) - Standard library +- Empty anonymous methods are now ignored in `EmptyBlock`. +- Empty anonymous methods are now flagged in `EmptyRoutine`. +- **API:** `AnonymousMethodNode::getStatementBlock` method. +- **API:** `AnonymousMethodNode::isEmpty` method. ## [1.12.2] - 2025-01-06 diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/EmptyBlockCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/EmptyBlockCheck.java index 54e3276e0..2cb368d47 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/EmptyBlockCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/EmptyBlockCheck.java @@ -19,6 +19,7 @@ package au.com.integradev.delphi.checks; import org.sonar.check.Rule; +import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode; import org.sonar.plugins.communitydelphi.api.ast.CaseItemStatementNode; import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode; import org.sonar.plugins.communitydelphi.api.ast.CompoundStatementNode; @@ -49,7 +50,7 @@ public DelphiCheckContext visit(CompoundStatementNode block, DelphiCheckContext private static boolean shouldAddViolation(CompoundStatementNode block) { DelphiNode parent = block.getParent(); - if (parent instanceof RoutineBodyNode) { + if (parent instanceof RoutineBodyNode || parent instanceof AnonymousMethodNode) { // Handled by EmptyRoutineRule return false; } diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/EmptyRoutineCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/EmptyRoutineCheck.java index 1b2a0bc46..61669468f 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/EmptyRoutineCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/EmptyRoutineCheck.java @@ -20,10 +20,12 @@ import au.com.integradev.delphi.utils.InterfaceUtils; import org.sonar.check.Rule; +import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; import org.sonar.plugins.communitydelphi.api.ast.RoutineImplementationNode; import org.sonar.plugins.communitydelphi.api.check.DelphiCheck; import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext; +import org.sonar.plugins.communitydelphi.api.check.FilePosition; import org.sonar.plugins.communitydelphi.api.symbol.declaration.RoutineDirective; import org.sonar.plugins.communitydelphi.api.symbol.declaration.RoutineNameDeclaration; import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; @@ -36,15 +38,30 @@ public class EmptyRoutineCheck extends DelphiCheck { @Override public DelphiCheckContext visit(RoutineImplementationNode routine, DelphiCheckContext context) { - if (routine.isEmpty() && shouldAddViolation(routine)) { + if (shouldAddViolation(routine)) { reportIssue(context, routine.getRoutineNameNode(), MESSAGE); } return super.visit(routine, context); } + @Override + public DelphiCheckContext visit(AnonymousMethodNode anonymousMethod, DelphiCheckContext context) { + if (shouldAddViolation(anonymousMethod)) { + context + .newIssue() + .onFilePosition(FilePosition.from(anonymousMethod.getFirstToken())) + .withMessage(MESSAGE) + .report(); + } + return super.visit(anonymousMethod, context); + } + private static boolean shouldAddViolation(RoutineImplementationNode routine) { - DelphiNode block = routine.getBlock(); + if (!routine.isEmpty()) { + return false; + } + DelphiNode block = routine.getBlock(); if (block != null && block.getComments().isEmpty()) { // All exclusions aside, an explanatory comment is mandatory return true; @@ -59,4 +76,8 @@ private static boolean shouldAddViolation(RoutineImplementationNode routine) { && !declaration.hasDirective(RoutineDirective.VIRTUAL) && !InterfaceUtils.implementsMethodOnInterface(declaration); } + + private static boolean shouldAddViolation(AnonymousMethodNode anonymousMethod) { + return anonymousMethod.isEmpty() && anonymousMethod.getStatementBlock().getComments().isEmpty(); + } } diff --git a/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/EmptyRoutine.html b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/EmptyRoutine.html index ea34fc2dd..4a2a28e04 100644 --- a/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/EmptyRoutine.html +++ b/delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/EmptyRoutine.html @@ -9,6 +9,7 @@

Why is this an issue?

  • Virtual methods (e.g. to provide a no-op default behaviour for child classes)
  • Override methods (e.g. to not run behaviour implemented in ancestor classes)
  • Methods that implement an interface
    • +
  • Anonymous methods

    • How to fix it

    If the empty routine is an omission, remove it.

    diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/EmptyBlockCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/EmptyBlockCheckTest.java index 03e98f380..14e39141c 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/EmptyBlockCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/EmptyBlockCheckTest.java @@ -157,4 +157,24 @@ void testEmptyRoutineShouldNotAddIssue() { .appendImpl("end;")) .verifyNoIssues(); } + + @Test + void testEmptyAnonymousMethodShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new EmptyBlockCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("type") + .appendDecl(" TProc = reference to procedure;") + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Bar: TProc;") + .appendImpl("begin") + .appendImpl(" Bar :=") + .appendImpl(" procedure") + .appendImpl(" begin") + .appendImpl(" end;") + .appendImpl("end;")) + .verifyNoIssues(); + } } diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/EmptyRoutineCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/EmptyRoutineCheckTest.java index 18475b6e0..63079e5cf 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/EmptyRoutineCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/EmptyRoutineCheckTest.java @@ -200,6 +200,47 @@ void testOverloadedMethodShouldNotAddIssue() { .verifyNoIssues(); } + @Test + void testEmptyAnonymousMethodWithoutCommentShouldAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new EmptyRoutineCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("type") + .appendDecl(" TProc = reference to procedure;") + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Bar: TProc;") + .appendImpl("begin") + .appendImpl(" Bar :=") + .appendImpl(" procedure // Noncompliant") + .appendImpl(" begin") + .appendImpl(" end;") + .appendImpl("end;")) + .verifyIssues(); + } + + @Test + void testEmptyAnonymousMethodWithCommentShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new EmptyRoutineCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("type") + .appendDecl(" TProc = reference to procedure;") + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Bar: TProc;") + .appendImpl("begin") + .appendImpl(" Bar :=") + .appendImpl(" procedure") + .appendImpl(" begin") + .appendImpl(" // do nothing") + .appendImpl(" end;") + .appendImpl("end;")) + .verifyNoIssues(); + } + @Test void testForwardDeclarationShouldNotAddIssue() { CheckVerifier.newVerifier() diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/AnonymousMethodNodeImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/AnonymousMethodNodeImpl.java index 23657e9cc..cebae9566 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/AnonymousMethodNodeImpl.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/antlr/ast/node/AnonymousMethodNodeImpl.java @@ -28,6 +28,7 @@ import org.antlr.runtime.Token; import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodHeadingNode; import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode; +import org.sonar.plugins.communitydelphi.api.ast.CompoundStatementNode; import org.sonar.plugins.communitydelphi.api.ast.RoutineParametersNode; import org.sonar.plugins.communitydelphi.api.ast.RoutineReturnTypeNode; import org.sonar.plugins.communitydelphi.api.symbol.declaration.RoutineDirective; @@ -98,6 +99,16 @@ public boolean isProcedure() { return getRoutineKind() == RoutineKind.PROCEDURE; } + @Override + public CompoundStatementNode getStatementBlock() { + return (CompoundStatementNode) getChild(1); + } + + @Override + public boolean isEmpty() { + return getStatementBlock().isEmpty(); + } + @Override public String getImage() { if (image == null) { diff --git a/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/AnonymousMethodNode.java b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/AnonymousMethodNode.java index a005eddf9..8b25dd36f 100644 --- a/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/AnonymousMethodNode.java +++ b/delphi-frontend/src/main/java/org/sonar/plugins/communitydelphi/api/ast/AnonymousMethodNode.java @@ -41,4 +41,8 @@ public interface AnonymousMethodNode extends ExpressionNode { boolean isFunction(); boolean isProcedure(); + + CompoundStatementNode getStatementBlock(); + + boolean isEmpty(); }