diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e3579ecc..fab6eddc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - False positives from case statements in `LoopExecutingAtMostOnce`. - False positives from nested finally-except blocks in `RedundantJump`. - False positives around wrapped type declarations in `VisibilityKeywordIndentation`. +- False negatives around inline `var` and `const` in `PlatformDependentTruncation`. - Trailing whitespace within comments not recognized in `TrailingWhitespace`. - Several compiler directives were not being recognized: - `E` diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/PlatformDependentTruncationCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/PlatformDependentTruncationCheck.java index 40917e83f..2cd41fb58 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/PlatformDependentTruncationCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/PlatformDependentTruncationCheck.java @@ -19,14 +19,18 @@ package au.com.integradev.delphi.checks; import java.util.List; +import javax.annotation.Nullable; import org.sonar.check.Rule; import org.sonar.plugins.communitydelphi.api.ast.ArgumentListNode; import org.sonar.plugins.communitydelphi.api.ast.ArgumentNode; import org.sonar.plugins.communitydelphi.api.ast.AssignmentStatementNode; import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; +import org.sonar.plugins.communitydelphi.api.ast.ConstStatementNode; import org.sonar.plugins.communitydelphi.api.ast.ExpressionNode; import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode; import org.sonar.plugins.communitydelphi.api.ast.Node; +import org.sonar.plugins.communitydelphi.api.ast.TypeNode; +import org.sonar.plugins.communitydelphi.api.ast.VarStatementNode; import org.sonar.plugins.communitydelphi.api.check.DelphiCheck; import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext; import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration; @@ -51,6 +55,22 @@ public DelphiCheckContext visit(AssignmentStatementNode assignment, DelphiCheckC return super.visit(assignment, context); } + @Override + public DelphiCheckContext visit(VarStatementNode varStatement, DelphiCheckContext context) { + if (isViolation(varStatement.getExpression(), varStatement.getTypeNode())) { + reportIssue(context, varStatement, MESSAGE); + } + return super.visit(varStatement, context); + } + + @Override + public DelphiCheckContext visit(ConstStatementNode constStatement, DelphiCheckContext context) { + if (isViolation(constStatement.getExpression(), constStatement.getTypeNode())) { + reportIssue(context, constStatement, MESSAGE); + } + return super.visit(constStatement, context); + } + @Override public DelphiCheckContext visit(ArgumentListNode argumentList, DelphiCheckContext context) { Node previous = argumentList.getParent().getChild(argumentList.getChildIndex() - 1); @@ -85,6 +105,13 @@ private static ProceduralType getProceduralType(NameReferenceNode reference) { return null; } + private static boolean isViolation(@Nullable ExpressionNode from, @Nullable TypeNode to) { + if (from == null || to == null) { + return false; + } + return isViolation(from, to.getType()); + } + private static boolean isViolation(ExpressionNode from, Type to) { if (!from.getType().isInteger() || !to.isInteger()) { return false; diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/PlatformDependentTruncationCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/PlatformDependentTruncationCheckTest.java index 84bb884ab..11d3e8e21 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/PlatformDependentTruncationCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/PlatformDependentTruncationCheckTest.java @@ -282,4 +282,223 @@ void testPlatformDependentCastInBinaryExpressionShouldNotAddIssue(String version .appendImpl("end;")) .verifyNoIssues(); } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testIntegerToNativeIntInlineVarAssignmentShouldNotAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Int: Integer;") + .appendImpl("begin") + .appendImpl(" var Nat: NativeInt := Int;") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testInt64ToNativeIntInlineVarAssignmentShouldAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" I64: Int64;") + .appendImpl("begin") + .appendImpl(" var Nat: NativeInt := I64; // Noncompliant") + .appendImpl("end;")) + .verifyIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testNativeIntToIntegerInlineVarAssignmentShouldAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Nat: NativeInt;") + .appendImpl("begin") + .appendImpl(" var Int: Integer := Nat; // Noncompliant") + .appendImpl("end;")) + .verifyIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testNativeIntToI64InlineVarAssignmentShouldNotAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Nat: NativeInt;") + .appendImpl("begin") + .appendImpl(" var I64: Int64 := Nat;") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testNativeIntToNativeIntInlineVarAssignmentShouldNotAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Nat2: NativeInt;") + .appendImpl("begin") + .appendImpl(" var Nat1: NativeInt := Nat2;") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testNativeIntToTypeInferredInlineVarAssignmentShouldNotAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Nat2: NativeInt;") + .appendImpl("begin") + .appendImpl(" var Nat1 := Nat2;") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testNoAssignmentToNativeIntInlineVarShouldNotAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("begin") + .appendImpl(" var Nat: NativeInt;") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testIntegerToNativeIntInlineConstAssignmentShouldNotAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Int: Integer;") + .appendImpl("begin") + .appendImpl(" const Nat: NativeInt = Int;") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testInt64ToNativeIntInlineConstAssignmentShouldAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" I64: Int64;") + .appendImpl("begin") + .appendImpl(" const Nat: NativeInt = I64; // Noncompliant") + .appendImpl("end;")) + .verifyIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testNativeIntToIntegerInlineConstAssignmentShouldAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Nat: NativeInt;") + .appendImpl("begin") + .appendImpl(" const Int: Integer = Nat; // Noncompliant") + .appendImpl("end;")) + .verifyIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testNativeIntToI64InlineConstAssignmentShouldNotAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Nat: NativeInt;") + .appendImpl("begin") + .appendImpl(" const I64: Int64 = Nat;") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testNativeIntToNativeIntInlineConstAssignmentShouldNotAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Nat2: NativeInt;") + .appendImpl("begin") + .appendImpl(" const Nat1: NativeInt = Nat2;") + .appendImpl("end;")) + .verifyNoIssues(); + } + + @ParameterizedTest + @ValueSource(strings = {VERSION_ALEXANDRIA, VERSION_ATHENS}) + void testNativeIntToTypeInferredInlineConstAssignmentShouldNotAddIssue(String versionSymbol) { + CheckVerifier.newVerifier() + .withCheck(new PlatformDependentTruncationCheck()) + .withCompilerVersion(CompilerVersion.fromVersionSymbol(versionSymbol)) + .onFile( + new DelphiTestUnitBuilder() + .appendImpl("procedure Foo;") + .appendImpl("var") + .appendImpl(" Nat2: NativeInt;") + .appendImpl("begin") + .appendImpl(" const Nat1 = Nat2;") + .appendImpl("end;")) + .verifyNoIssues(); + } }