Skip to content

[java] New rule: AssertStatementWithSideEffect #3776

@oowekyala

Description

@oowekyala

Proposed Rule Name: AssertStatementWithSideEffect

Proposed Category: error prone

Description: Flags assert statements that appear to perform side effects. These may change the behaviour of the program depending on whether assertions are enabled at runtime or not.

A possible implementation strategy would be to flag assignments and increments, and any method call that appears to perform side effects. A rough heuristic to find methods that appear to perform side effects would be to say that all methods perform side effects, unless one or more of the following is true

  • the method has a conventional get* or is* property getter name (regardless of arguments)
  • the method is an accessor for a record component
  • the method belongs to a known immutable type:
    • String, primitive wrappers, annotations, BigDecimal, BigInteger, etc
    • utility classes like java.util.Objects
  • the method name is part of an allow-list:
    • equals(Object) and hashCode() should be allowed on any type
    • size() and length() should probably be allowed on any type, maybe other widespread methods like these exist
    • static factory methods should be recognized like List.of and Set.of. Maybe any static method called of or valueOf that is not void should be allowed.

The logic to find methods that appear to perform side effects should be in JavaRuleUtil and usable by other rules. We could improve it later by eg recognizing deeply immutable objects as those that either don't have any fields, or only final fields of a deeply immutable type (this definition is recursive).

Code Sample: This should include code, that should be flagged by the rule. If possible, the "correct" code
according to this new rule should also be demonstrated.

class Foo {
  static <T> void forceAdd(Collection<T> c, T x) {
    assert c.add(x); // not ok, program is incorrect if assertions are disabled
 
    boolean added = c.add(x);
    assert added; // ok
    assert c.size() > 0; // ok
    assert c.getClass().getName().substring(1).equals("foo"); // ok
  }
}

Possible Properties:

  • Maybe a property for the allow-list of types (immutableTypes)
  • Maybe a property for the allow-list of methods (pureMethods) - should probably accept input in the format of PMD 7's invocation matchers, eg _#equals(java.lang.Object)

Additional context:

Discussion on #3488

Metadata

Metadata

Assignees

No one assigned

    Labels

    a:new-ruleProposal to add a new built-in rule

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions