diff --git a/CHANGELOG.md b/CHANGELOG.md index b073fce66..5b2ecd603 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Exceptions from empty structures (e.g., `if`) in `LoopExecutingAtMostOnce` and `RedundantJump`. - False positives from case statements in `LoopExecutingAtMostOnce`. - False positives from nested finally-except blocks in `RedundantJump`. +- False positives around wrapped type declarations in `VisibilityKeywordIndentation`. ## [1.14.1] - 2025-03-05 diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/VisibilityKeywordIndentationCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/VisibilityKeywordIndentationCheck.java index 11422c0d4..c60e13077 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/VisibilityKeywordIndentationCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/VisibilityKeywordIndentationCheck.java @@ -20,7 +20,8 @@ import au.com.integradev.delphi.utils.IndentationUtils; import org.sonar.check.Rule; -import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; +import org.sonar.plugins.communitydelphi.api.ast.NameDeclarationNode; +import org.sonar.plugins.communitydelphi.api.ast.TypeDeclarationNode; import org.sonar.plugins.communitydelphi.api.ast.VisibilityNode; import org.sonar.plugins.communitydelphi.api.check.DelphiCheck; import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext; @@ -32,19 +33,21 @@ public class VisibilityKeywordIndentationCheck extends DelphiCheck { private static final String MESSAGE = "Indent this visibility specifier to the indentation level of the containing type."; - private static String getExpectedIndentation(DelphiNode node) { - var visibilityNode = (VisibilityNode) node; - // Class/Record/etc. -> VisibilitySection -> Visibility - var parent = visibilityNode.getParent().getParent(); - return IndentationUtils.getLineIndentation(parent); - } - @Override public DelphiCheckContext visit(VisibilityNode visibilityNode, DelphiCheckContext context) { - if (!IndentationUtils.getLineIndentation(visibilityNode) - .equals(getExpectedIndentation(visibilityNode))) { - reportIssue(context, visibilityNode, MESSAGE); + var declaration = visibilityNode.getNthParent(3); + + if (declaration instanceof TypeDeclarationNode) { + NameDeclarationNode typeName = ((TypeDeclarationNode) declaration).getTypeNameNode(); + + String actual = IndentationUtils.getLineIndentation(visibilityNode); + String expected = IndentationUtils.getLineIndentation(typeName); + + if (!actual.equals(expected)) { + reportIssue(context, visibilityNode, MESSAGE); + } } + return super.visit(visibilityNode, context); } } diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/VisibilityKeywordIndentationCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/VisibilityKeywordIndentationCheckTest.java index ba1e39b68..b5521960f 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/VisibilityKeywordIndentationCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/VisibilityKeywordIndentationCheckTest.java @@ -34,7 +34,7 @@ class VisibilityKeywordIndentationCheckTest { "class helper for TObject", "record helper for string" }) - void testTooIndentedVisibilitySpecifierShouldAddIssue(String structType) { + void testVisibilityWithTooMuchIndentationShouldAddIssue(String structType) { CheckVerifier.newVerifier() .withCheck(new VisibilityKeywordIndentationCheck()) .onFile( @@ -56,7 +56,7 @@ void testTooIndentedVisibilitySpecifierShouldAddIssue(String structType) { "class helper for TObject", "record helper for string" }) - void testCorrectlyIndentedVisibilitySpecifierShouldNotAddIssue(String structType) { + void testVisibilityAnchoredToTypeDeclarationShouldNotAddIssue(String structType) { CheckVerifier.newVerifier() .withCheck(new VisibilityKeywordIndentationCheck()) .onFile( @@ -70,7 +70,7 @@ void testCorrectlyIndentedVisibilitySpecifierShouldNotAddIssue(String structType } @Test - void testImplicitPublishedVisibilitySectionShouldNotAddIssue() { + void testImplicitPublishedVisibilityShouldNotAddIssue() { CheckVerifier.newVerifier() .withCheck(new VisibilityKeywordIndentationCheck()) .onFile( @@ -84,7 +84,7 @@ void testImplicitPublishedVisibilitySectionShouldNotAddIssue() { } @Test - void testUnindentedVisibilitySpecifierShouldAddIssue() { + void testVisibilityAtMarginShouldAddIssue() { CheckVerifier.newVerifier() .withCheck(new VisibilityKeywordIndentationCheck()) .onFile( @@ -96,4 +96,78 @@ void testUnindentedVisibilitySpecifierShouldAddIssue() { .appendDecl(" end;")) .verifyIssues(); } + + @Test + void testWrappedClassDeclarationShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new VisibilityKeywordIndentationCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("type") + .appendDecl(" TFoo =") + .appendDecl(" class(TObject)") + .appendDecl(" private") + .appendDecl(" procedure Proc;") + .appendDecl(" end;")) + .verifyNoIssues(); + } + + @Test + void testMisalignedCustomAttributeShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new VisibilityKeywordIndentationCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("type") + .appendDecl(" [Foo]") + .appendDecl(" TBar = class(TObject)") + .appendDecl(" private") + .appendDecl(" procedure Baz;") + .appendDecl(" end;")) + .verifyNoIssues(); + } + + @Test + void testSameLineCustomAttributeShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new VisibilityKeywordIndentationCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("type") + .appendDecl(" [Foo] TBar = class(TObject)") + .appendDecl(" private") + .appendDecl(" procedure Baz;") + .appendDecl(" end;")) + .verifyNoIssues(); + } + + @Test + void testVarAnonymousRecordShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new VisibilityKeywordIndentationCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("var") + .appendDecl(" Foo: record") + .appendDecl(" public") + .appendDecl(" public") + .appendDecl(" Bar: Integer;") + .appendDecl(" end;")) + .verifyNoIssues(); + } + + @Test + void testConstAnonymousRecordShouldNotAddIssue() { + CheckVerifier.newVerifier() + .withCheck(new VisibilityKeywordIndentationCheck()) + .onFile( + new DelphiTestUnitBuilder() + .appendDecl("const") + .appendDecl(" Foo: record") + .appendDecl(" public") + .appendDecl(" public") + .appendDecl(" Bar: Integer;") + .appendDecl(" end = ();")) + .verifyNoIssues(); + } }