Skip to content

[java] New rule: UnnecessaryBoxing #2973

@oowekyala

Description

@oowekyala

Proposed Rule Name: UnnecessaryBoxing (pmd7)

The rule "PrimitiveWrapperInstantiation" has been implemented for PMD 6.37.0.

Proposed Category: Best Practices

Description:
The new rules that merges together:

(all in category performance)

The first rule flags valueOf calls, and only cares about primitive wrappers that are created and unboxed explicitly in the same expression, like Integer.valueOf(0).intValue(). This is contrived and no one does it: the rule has zero violations in checkstyle, spring, or the jdk.

The other rules flag uses of the constructors of primitive wrappers (but for some reason not Double, Character or Float). The rationale is that using valueOf may avoid creating a new instance. This is true, but also,

  • the constructors are deprecated since Java 9, because of project Valhalla
  • usage of both the constructors and valueOf can be replaced nearly everywhere by autoboxing

So the idea is to merge all those into a new UnnecessaryBoxing rule that cares about:

  • Explicit boxing in contexts where the primitive would be autoboxed: Object i = Integer.valueOf(2)
  • Explicit unboxing in a context where it would be auto-unboxed: int i = integer.intValue()
  • Explicit unboxing in contexts where the value is immediately reboxed: Object i = integer.intValue() (would rebox the int)
  • Explicit boxing in contexts where the value is immediately unboxed: int i = Integer.valueOf(0) (would unbox the integer)
  • Boxing of an already boxed value: Integer.valueOf(integer) (unboxes integer, then boxes it)
  • Uses of Integer.valueOf(someString) where an int is expected: Integer.parseInt is more appropriate. -> [java] UnnecessaryBoxing - check for Integer.valueOf(String) calls #3500
  • Casts that box/unbox their operand (if ((boolean) Boolean.TRUE) ...) are unnecessary -> this is left to rule UnnecessaryCast, see [java] Generalize UnnecessaryCast to flag all unnecessary casts #3218

The new rule PrimitiveWrapperConstructor cares about:

  • Usage of primitive wrapper constructors, e.g. new Integer(1). Instead, Integer.valueOf(1) should be used.

This applies to all primitives.

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 Scratch {

    public static void main(String[] args) {

        Integer anInteger = 2; // ok
        
        Object a = Integer.valueOf(2);  // explicit boxing where the value would be autoboxed
        int b = anInteger.intValue();     // explicit unboxing where the value would be auto-unboxed
        Object c = anInteger.intValue();  // unboxing where the value is immediately reboxed
        int i = Integer.valueOf(0);     // boxing where the value is immediately unboxed
        
        Integer.valueOf(anInteger);       // boxing of already boxed value
        
        if ((boolean) Boolean.TRUE);    // Casts that unboxes the operand        
        
        // Actually a cast to a wrapper can be treated just like a valueOf call
        // (when the cast operand is primitive) and be handled exactly like a valueOf call,
        // so be handled in all the situations above.
        
        Integer o = (Integer) a; // a is an Object so the cast is not redundant
    }
}

Possible Properties:

None that I can think of.

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

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions