[java] JUnitAssertionsShouldIncludeMessage false positive with AssertJ

[golszewski86]

Affects PMD Version: 6.10.0

Rule: JUnitAssertionsShouldIncludeMessage

Description: PMD reports "JUnit assertions should include a message" for AssertJ assertions with message. AssertJ assertions are described in a different way than JUnit or Hamcrest assertions, e.g.

assertThat(actual).as("Assertion message").isEqualTo(expected);

See: http://joel-costigliola.github.io/assertj/assertj-core-features-highlight.html#describe-assertion

Code Sample demonstrating the issue:

    @Test
    public void shouldMapResult() throws SQLException {
        new JdbcSession(this.database.dataSource()).sql(
            "insert into test values (1, 2), (2, 3), (3, 4)"
        ).update(Outcome.VOID);
        assertThat(
            new JdbcSession(this.database.dataSource()).sql("select * from test")
                .select(new MapOutcome<>(rset -> rset.getInt(1), rset -> rset.getInt(2)))
        ).as("Should return database entries as map")
            .hasSize(3)
            .containsEntry(1, 2)
            .containsEntry(2, 3)
            .containsEntry(3, 4);
    }

Running PMD through: Maven

[oujesky]

Possible workaround is to introduce a suppression when the assertThat method detected has only a single argument

<rule ref="category/java/bestpractices.xml/JUnitAssertionsShouldIncludeMessage">
    <properties>
        <!-- suppress for AssertJ assertThat method -->
        <property name="violationSuppressXPath" value=".[@Image='assertThat'][../following-sibling::PrimarySuffix[1][@ArgumentCount=1]]" />
    </properties>
</rule>

Could be supplemented together with additional rule, that is checking that assertThat statements are followed either by as or describedAs calls (and these are not the last call in the chain)

<rule name="AssertJAssertionsShouldIncludeMessage"
      language="java"
      message="AssertJ assertions should include a message before actual assertions"
      class="net.sourceforge.pmd.lang.rule.XPathRule">
    <description>AssertJ assertions should include an informative message - i.e., call .as() or .describedAs() before the actual assertion to specify the message.

    The call to .as() or .describedAs() should not be the last call in the chain, otherwise it might be ignored when some assertion fails.</description>
    <priority>3</priority>
    <properties>
        <property name="version" value="2.0" />
        <property name="xpath">
            <value>
                <![CDATA[
                    //PrimaryPrefix/Name[@Image='assertThat']
                        [not(../../PrimarySuffix
                            (: not followed by as or describedAs :)
                            [@Image='as' or @Image='describedAs']
                            (: not the last call in the call chain :)
                            [count(following-sibling::PrimarySuffix) > 1]
                        )]
                ]]>
            </value>
        </property>
    </properties>
    <example>
        <![CDATA[
            public class Foo extends TestCase {
                public void testSomething() {
                    assertThat("foo").isEqualTo("bar");
                    // Use the form:
                    // assertThat("foo").as("Foo does not equals bar").isEqualTo("bar");
                    // instead
                }

                public void testSomethingElse() {
                    assertThat("foo").isEqualTo("bar").as("This message will be ignored");
                    // Use the form:
                    // assertThat("foo").as("Foo does not equals bar").isEqualTo("bar");
                    // instead
                }
            }
        ]]>
    </example>
</rule>

[adangel]

Fixed via #2899 for PMD 7.