Checkstyle JavadocMethod vs DRE_DECLARED_RUNTIME_EXCEPTION

[victorwss]

I'm using both Checkstyle and SpotBugs with fb-contrib on a project of mine.

In Checkstyle, I have a rule for checking JavadocMethod like this:

        <module name="JavadocMethod">
            <property name="scope" value="public"/>
            <property name="allowMissingParamTags" value="false"/>
            <property name="allowMissingThrowsTags" value="false"/>
            <property name="allowMissingReturnTag" value="false"/>
            <property name="allowMissingJavadoc" value="true"/>
            <property name="minLineCount" value="1"/>
            <property name="allowedAnnotations" value="Override, Test"/>
            <property name="allowThrowsTagsForSubclasses" value="true"/>
            <property name="allowUndeclaredRTE" value="false"/>
            <property name="validateThrows" value="true"/>
        </module>

Then, I have some method like this:

    /**
     * Do something.
     * @param x Some parameter to do something
     * @throws NullPointerException If the {@code x} parameter is {@code null}.
     */
    public void doSomething(String x) throws NullPointerException {
        if (x == null) throw new NullPointerException("We don't accept nulls, bro!");
        // Do something with the x here.
    }

The problem is that fb-contrib complains with DRE_DECLARED_RUNTIME_EXCEPTION due to the throws NullPointerException.

If I remove the throws NullPointerException, then checkstyle will complain instead.

The solution is to switch Checkstyle's <property name="allowUndeclaredRTE" value="false"/> to true. Its default value is false.

The current description of DRE_DECLARED_RUNTIME_EXCEPTION states this:

This method declares a RuntimeException derived class in its throws clause. This may indicate a misunderstanding as to how unchecked exceptions are handled. If it is felt that a RuntimeException is so prevalent that it should be declared, it is probably a better idea to prevent the occurrence in code.

I think that this description should be changed because some people occasionally add throws clauses with RuntimeExceptions for documentations purposes. I propose that the description should state explicitly that even for documentation purposes, it isn't a good idea to add RuntimeExceptions into the throws clause because nothing can guarantee their correctness, they just pollute the method signature without offering any benefit for that and that their place are in the javadoc's @throws clause instead. It could even cite explicitly Checkstyle's <property name="allowUndeclaredRTE" value="true"/>.

[ThrawnCA]

Well, given that there's a conflict between Checkstyle's expectations and fb-contrib's, it might make just as much sense to change Checkstyle. I expect that you could find plenty of advocates of each approach.

For this specific case, I'll note that you could use Objects.requireNonNull to avoid the conflict.

[mebigfatguy]

update bug description to emphasis documentation alternative with javadoc