Skip to content

Commit

Permalink
Merge pull request #4097 from tprouvot:feature/addNewClassForAssertion
Browse files Browse the repository at this point in the history
[apex] ApexUnitTestClassShouldHaveAssertsRule: Support new Assert class (Apex v56.0) #4097
  • Loading branch information
adangel committed Aug 29, 2022
2 parents 563d7b6 + 04b38c3 commit 54a2614
Show file tree
Hide file tree
Showing 7 changed files with 270 additions and 17 deletions.
3 changes: 2 additions & 1 deletion .all-contributorsrc
Expand Up @@ -5918,7 +5918,8 @@
"avatar_url": "https://avatars.githubusercontent.com/u/35368290?v=4",
"profile": "https://github.com/tprouvot",
"contributions": [
"bug"
"bug",
"code"
]
},
{
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/pmd/projectdocs/credits.md
Expand Up @@ -935,7 +935,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
<td align="center"><a href="https://github.com/thanosa"><img src="https://avatars.githubusercontent.com/u/24596498?v=4?s=100" width="100px;" alt=""/><br /><sub><b>thanosa</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Athanosa" title="Bug reports">🐛</a></td>
<td align="center"><a href="https://github.com/tiandiyixian"><img src="https://avatars.githubusercontent.com/u/27055337?v=4?s=100" width="100px;" alt=""/><br /><sub><b>tiandiyixian</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Atiandiyixian" title="Bug reports">🐛</a></td>
<td align="center"><a href="https://github.com/tobwoerk"><img src="https://avatars.githubusercontent.com/u/11739442?v=4?s=100" width="100px;" alt=""/><br /><sub><b>tobwoerk</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Atobwoerk" title="Bug reports">🐛</a></td>
<td align="center"><a href="https://github.com/tprouvot"><img src="https://avatars.githubusercontent.com/u/35368290?v=4?s=100" width="100px;" alt=""/><br /><sub><b>tprouvot</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Atprouvot" title="Bug reports">🐛</a></td>
<td align="center"><a href="https://github.com/tprouvot"><img src="https://avatars.githubusercontent.com/u/35368290?v=4?s=100" width="100px;" alt=""/><br /><sub><b>tprouvot</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Atprouvot" title="Bug reports">🐛</a> <a href="https://github.com/pmd/pmd/commits?author=tprouvot" title="Code">💻</a></td>
<td align="center"><a href="https://github.com/trentchilders"><img src="https://avatars.githubusercontent.com/u/6664350?v=4?s=100" width="100px;" alt=""/><br /><sub><b>trentchilders</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Atrentchilders" title="Bug reports">🐛</a></td>
<td align="center"><a href="https://github.com/triandicAnt"><img src="https://avatars.githubusercontent.com/u/2345902?v=4?s=100" width="100px;" alt=""/><br /><sub><b>triandicAnt</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3AtriandicAnt" title="Bug reports">🐛</a></td>
<td align="center"><a href="https://github.com/trishul14"><img src="https://avatars.githubusercontent.com/u/24551131?v=4?s=100" width="100px;" alt=""/><br /><sub><b>trishul14</b></sub></a><br /><a href="https://github.com/pmd/pmd/issues?q=author%3Atrishul14" title="Bug reports">🐛</a></td>
Expand Down
3 changes: 3 additions & 0 deletions docs/pages/release_notes.md
Expand Up @@ -16,6 +16,8 @@ This is a {{ site.pmd.release_type }} release.

### Fixed Issues

* apex
* [#4096](https://github.com/pmd/pmd/issues/4096): \[apex] ApexAssertionsShouldIncludeMessage and ApexUnitTestClassShouldHaveAsserts: support new Assert class (introduced with Apex v56.0)
* java-codestyle
* [#4082](https://github.com/pmd/pmd/issues/4082): \[java] UnnecessaryImport false positive for on-demand imports of nested classes

Expand All @@ -41,6 +43,7 @@ This is a {{ site.pmd.release_type }} release.
* [#4081](https://github.com/pmd/pmd/pull/4081): \[apex] Remove Jorje leaks outside `ast` package - [@eklimo](https://github.com/eklimo)
* [#4083](https://github.com/pmd/pmd/pull/4083): \[java] UnnecessaryImport false positive for on-demand imports of nested classes (fix for #4082) - [@abyss638](https://github.com/abyss638)
* [#4092](https://github.com/pmd/pmd/pull/4092): \[apex] Implement ApexQualifiableNode for ASTUserEnum - [@aaronhurst-google](https://github.com/aaronhurst-google)
* [#4097](https://github.com/pmd/pmd/pull/4097): \[apex] ApexUnitTestClassShouldHaveAssertsRule: Support new Assert class (Apex v56.0) - [@tprouvot](https://github.com/tprouvot)
* [#4104](https://github.com/pmd/pmd/pull/4104): \[doc] Add MegaLinter in the list of integrations - [@nvuillam](https://github.com/nvuillam)

{% endtocmaker %}
Expand Down
Expand Up @@ -12,17 +12,39 @@ public class ApexAssertionsShouldIncludeMessageRule extends AbstractApexUnitTest
private static final String ASSERT = "System.assert";
private static final String ASSERT_EQUALS = "System.assertEquals";
private static final String ASSERT_NOT_EQUALS = "System.assertNotEquals";
private static final String ARE_EQUAL = "Assert.areEqual";
private static final String ARE_NOT_EQUAL = "Assert.areNotEqual";
private static final String IS_FALSE = "Assert.isFalse";
private static final String FAIL = "Assert.fail";
private static final String IS_INSTANCE_OF_TYPE = "Assert.isInstanceOfType";
private static final String IS_NOT_INSTANCE_OF_TYPE = "Assert.isNotInstanceOfType";
private static final String IS_NOT_NULL = "Assert.isNotNull";
private static final String IS_NULL = "Assert.isNull";
private static final String IS_TRUE = "Assert.isTrue";

@Override
public Object visit(ASTMethodCallExpression node, Object data) {
String methodName = node.getFullMethodName();

if (ASSERT.equalsIgnoreCase(methodName) && node.getNumChildren() == 2) {
if (FAIL.equalsIgnoreCase(methodName) && node.getNumChildren() == 1) {
addViolationWithMessage(data, node,
"''{0}'' should have 1 parameters.",
new Object[] { FAIL });
} else if ((ASSERT.equalsIgnoreCase(methodName)
|| IS_FALSE.equalsIgnoreCase(methodName)
|| IS_NOT_NULL.equalsIgnoreCase(methodName)
|| IS_NULL.equalsIgnoreCase(methodName)
|| IS_TRUE.equalsIgnoreCase(methodName))
&& node.getNumChildren() == 2) {
addViolationWithMessage(data, node,
"''{0}'' should have 2 parameters.",
new Object[] { ASSERT });
new Object[] { methodName });
} else if ((ASSERT_EQUALS.equalsIgnoreCase(methodName)
|| ASSERT_NOT_EQUALS.equalsIgnoreCase(methodName))
|| ASSERT_NOT_EQUALS.equalsIgnoreCase(methodName)
|| ARE_EQUAL.equalsIgnoreCase(methodName)
|| ARE_NOT_EQUAL.equalsIgnoreCase(methodName)
|| IS_INSTANCE_OF_TYPE.equalsIgnoreCase(methodName)
|| IS_NOT_INSTANCE_OF_TYPE.equalsIgnoreCase(methodName))
&& node.getNumChildren() == 3) {
addViolationWithMessage(data, node,
"''{0}'' should have 3 parameters.",
Expand Down
Expand Up @@ -37,18 +37,39 @@ public class ApexUnitTestClassShouldHaveAssertsRule extends AbstractApexUnitTest
ASSERT_METHODS.add("system.assert");
ASSERT_METHODS.add("system.assertequals");
ASSERT_METHODS.add("system.assertnotequals");
ASSERT_METHODS.add("assert.areequal");
ASSERT_METHODS.add("assert.arenotequal");
ASSERT_METHODS.add("assert.fail");
ASSERT_METHODS.add("assert.isfalse");
ASSERT_METHODS.add("assert.isinstanceoftype");
ASSERT_METHODS.add("assert.isnotinstanceoftype");
ASSERT_METHODS.add("assert.isnull");
ASSERT_METHODS.add("assert.isnotnull");
ASSERT_METHODS.add("assert.istrue");
// Fully-qualified variants...rare but still valid/possible
ASSERT_METHODS.add("system.system.assert");
ASSERT_METHODS.add("system.system.assertequals");
ASSERT_METHODS.add("system.system.assertnotequals");
ASSERT_METHODS.add("system.assert.areequal");
ASSERT_METHODS.add("system.assert.arenotequal");
ASSERT_METHODS.add("system.assert.fail");
ASSERT_METHODS.add("system.assert.isfalse");
ASSERT_METHODS.add("system.assert.isinstanceoftype");
ASSERT_METHODS.add("system.assert.isnotinstanceoftype");
ASSERT_METHODS.add("system.assert.isnull");
ASSERT_METHODS.add("system.assert.isnotnull");
ASSERT_METHODS.add("system.assert.istrue");
}

// Using a string property instead of a regex property to ensure that the compiled pattern can be case-insensitive
private static final PropertyDescriptor<String> ADDITIONAL_ASSERT_METHOD_PATTERN_DESCRIPTOR =
stringProperty("additionalAssertMethodPattern")
.desc("A regular expression for one or more custom test assertion method patterns.").defaultValue("").build();
// Using a string property instead of a regex property to ensure that the
// compiled pattern can be case-insensitive
private static final PropertyDescriptor<String> ADDITIONAL_ASSERT_METHOD_PATTERN_DESCRIPTOR = stringProperty(
"additionalAssertMethodPattern")
.desc("A regular expression for one or more custom test assertion method patterns.").defaultValue("")
.build();

// A simple compiled pattern cache to ensure that we only ever try to compile the configured pattern once for a given run
// A simple compiled pattern cache to ensure that we only ever try to compile
// the configured pattern once for a given run
private Optional<Pattern> compiledAdditionalAssertMethodPattern = null;

public ApexUnitTestClassShouldHaveAssertsRule() {
Expand Down Expand Up @@ -81,7 +102,8 @@ private Object checkForAssertStatements(ApexNode<?> node, Object data) {
}
}

// If we didn't find assert method invocations the simple way and we have a configured pattern, try it
// If we didn't find assert method invocations the simple way and we have a
// configured pattern, try it
if (!isAssertFound) {
final String additionalAssertMethodPattern = getProperty(ADDITIONAL_ASSERT_METHOD_PATTERN_DESCRIPTOR);
final Pattern compiledPattern = getCompiledAdditionalAssertMethodPattern(additionalAssertMethodPattern);
Expand All @@ -105,12 +127,15 @@ private Object checkForAssertStatements(ApexNode<?> node, Object data) {

private Pattern getCompiledAdditionalAssertMethodPattern(String additionalAssertMethodPattern) {
if (StringUtils.isNotBlank(additionalAssertMethodPattern)) {
// Check for presence first since we will cache a null value for patterns that don't compile
// Check for presence first since we will cache a null value for patterns that
// don't compile
if (compiledAdditionalAssertMethodPattern == null) {
try {
compiledAdditionalAssertMethodPattern = Optional.of(Pattern.compile(additionalAssertMethodPattern, Pattern.CASE_INSENSITIVE));
compiledAdditionalAssertMethodPattern = Optional
.of(Pattern.compile(additionalAssertMethodPattern, Pattern.CASE_INSENSITIVE));
} catch (IllegalArgumentException e) {
// Cache a null compiled pattern so that we won't try to compile this one again during the run
// Cache a null compiled pattern so that we won't try to compile this one again
// during the run
compiledAdditionalAssertMethodPattern = Optional.ofNullable(null);
throw e;
}
Expand Down
Expand Up @@ -46,6 +46,141 @@ public class Foo {
System.assertEquals('Test opportunity', o.Name);
System.assert(o.isClosed);
}
}
]]></code>
</test-code>

<test-code>
<description>[apex] Support new Assert class (Apex v56.0) #4097</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
@isTest
public class Foo {
@isTest
static void testAreEqual() {
String sub = 'abcde'.substring(2);
Assert.areEqual('cde', sub, 'Expected characters after first two');
}
@isTest
static void testAreNotEqual() {
String sub = 'abcde'.substring(2);
Assert.areNotEqual('xyz', sub, 'Characters not expected after first two');
}
@isTest
static void testFail() {
try {
SomeClass.methodUnderTest();
Assert.fail('DmlException Expected');
} catch (DmlException ex) {
// Add assertions here about the expected exception
}
}
@isTest
static void testIsFalse() {
Boolean containsCode = 'Salesforce'.contains('code');
Assert.isFalse(containsCode, 'No code');
}
@isTest
static void testIsInstanceOf() {
Account o = new Account();
Assert.isInstanceOfType(o, Account.class, 'Wrong instance');
}
@isTest
static void testIsNotInstanceOf() {
Contact con = new Contact();
Assert.isNotInstanceOfType(con, Account.class, 'Not expected type');
}
@isTest
static void testIsNotNull() {
String myString = 'value';
Assert.isNotNull(myString, 'myString should not be null');
}
@isTest
static void testIsNull() {
String myString = null;
Assert.isNull(myString, 'String should be null');
}
@isTest
static void testIsTrue() {
Boolean containsForce = 'Salesforce'.contains('force');
Assert.isTrue(containsForce, 'Contains force');
}
}
]]></code>
</test-code>

<test-code>
<description>[apex] Support new Assert class (Apex v56.0) - negative test case #4097</description>
<expected-problems>9</expected-problems>
<expected-linenumbers>6,12,19,28,34,40,46,52,58</expected-linenumbers>
<code><![CDATA[
@isTest
public class Foo {
@isTest
static void testAreEqual() {
String sub = 'abcde'.substring(2);
Assert.areEqual('cde', sub);
}
@isTest
static void testAreNotEqual() {
String sub = 'abcde'.substring(2);
Assert.areNotEqual('xyz', sub);
}
@isTest
static void testFail() {
try {
SomeClass.methodUnderTest();
Assert.fail();
} catch (DmlException ex) {
// Add assertions here about the expected exception
}
}
@isTest
static void testIsFalse() {
Boolean containsCode = 'Salesforce'.contains('code');
Assert.isFalse(containsCode);
}
@isTest
static void testIsInstanceOf() {
Account o = new Account();
Assert.isInstanceOfType(o, Account.class);
}
@isTest
static void testIsNotInstanceOf() {
Contact con = new Contact();
Assert.isNotInstanceOfType(con, Account.class);
}
@isTest
static void testIsNotNull() {
String myString = 'value';
Assert.isNotNull(myString);
}
@isTest
static void testIsNull() {
String myString = null;
Assert.isNull(myString);
}
@isTest
static void testIsTrue() {
Boolean containsForce = 'Salesforce'.contains('force');
Assert.isTrue(containsForce);
}
}
]]></code>
</test-code>
Expand Down
Expand Up @@ -76,13 +76,13 @@ private class C2_Assignment_Report_Job_Test {

<test-code>
<description>#1089 [apex] ApexUnitTestClassShouldHaveAsserts: Verify use of additionalAssertMethodPattern, positive test</description>
<rule-property name="additionalAssertMethodPattern">(Assert\.\w+|verify\w+)</rule-property>
<rule-property name="additionalAssertMethodPattern">(MyAssert\.\w+|verify\w+)</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
@isTest
public class Foo {
public static testMethod void testAssertIsTrue() {
Assert.isTrue(someCondition);
MyAssert.isTrue(someCondition);
}
public static testMethod void testLocalVerify() {
Expand All @@ -103,7 +103,7 @@ public class Foo {
@isTest
public class Foo {
public static testMethod void testAssertIsTrue() {
Assert.isTrue(someCondition);
MyAssert.isTrue(someCondition);
}
public static testMethod void testLocalVerify() {
Expand All @@ -115,4 +115,71 @@ public class Foo {
}
]]></code>
</test-code>

<test-code>
<description>[apex] ApexUnitTestClassShouldHaveAssertsRule: Support new Assert class (Apex v56.0) #4097</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
@isTest
public class Foo {
@isTest
static void testAreEqual() {
String sub = 'abcde'.substring(2);
Assert.areEqual('cde', sub);
}
@isTest
static void testAreNotEqual() {
String sub = 'abcde'.substring(2);
Assert.areNotEqual('xyz', sub);
}
@isTest
static void testFail() {
try {
SomeClass.methodUnderTest();
Assert.fail();
} catch (DmlException ex) {
// Add assertions here about the expected exception
}
}
@isTest
static void testIsFalse() {
Boolean containsCode = 'Salesforce'.contains('code');
Assert.isFalse(containsCode);
}
@isTest
static void testIsInstanceOf() {
Account o = new Account();
Assert.isInstanceOfType(o, Account.class);
}
@isTest
static void testIsNotInstanceOf() {
Contact con = new Contact();
Assert.isNotInstanceOfType(con, Account.class);
}
@isTest
static void testIsNotNull() {
String myString = 'value';
Assert.isNotNull(myString);
}
@isTest
static void testIsNull() {
String myString = null;
Assert.isNull(myString);
}
@isTest
static void testIsTrue() {
Boolean containsForce = 'Salesforce'.contains('force');
Assert.isTrue(containsForce);
}
}
]]></code>
</test-code>
</test-data>

0 comments on commit 54a2614

Please sign in to comment.