Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[java] FN in ImmutableField for field with a default + constructor assignment #4869

Open
jsotuyod opened this issue Mar 21, 2024 · 1 comment
Labels
a:false-negative PMD doesn't flag a problematic piece of code

Comments

@jsotuyod
Copy link
Member

Under PMD 7.0.0 there is no report for the code:

public class NotSoImmutableField {
    private int foo = 2;

    NotSoImmutableField() {
    }

    NotSoImmutableField(final int foo) {
        this.foo = foo;
    }

    public int getFoo() {
        return foo;
    }
}

The field is never written more than once within a given constructor, nor depends on the previous value. Therefore, the code can effectively be rewritten to make foo immutable, as it's never written after initialization, for instance, the code could be rewritten as:

public class NotSoImmutableField {
    private final int foo;

    NotSoImmutableField() {
        this(2);
    }

    NotSoImmutableField(final int foo) {
        this.foo = foo;
    }

    public int getFoo() {
        return foo;
    }
}

Originally posted by @jsotuyod in #4046 (comment)

@jsotuyod jsotuyod added the a:false-negative PMD doesn't flag a problematic piece of code label Mar 21, 2024
@oowekyala
Copy link
Member

I think this makes sense in theory, but it probably should be a new rule. I like that ImmutableField always recommends making things final if it actually can be done by just adding a final keyword, and there is no chance the compiler will reject the new code. The fix could in theory be done automatically. Recommending that the user adapts their initialization logic like you suggest is a different task for the programmer and requires more effort, also there are different ways to refactor that. The rule implementation would also likely be very different... So yeah I think it should be a new rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-negative PMD doesn't flag a problematic piece of code
Projects
None yet
Development

No branches or pull requests

2 participants