Generated by 🚫 Danger

Seems like my change removes violations on fields that are never assigned (even in their initializer). Should we report them? It may be set via reflection, but also be a legit mistake. It doesn't feel like the correct rule to report that though. In any case the error message should be different in this case...

Thanks @oowekyala. It seems that with the code change, defaultSuppressionAnnotations() is not necessary in ImmutableFieldRule now. I tried to delete line 38-49 and line 7-8, and the build remains successful. Do we want to remove those lines?

Thanks for pointing this out. I think it makes sense, especially since these issues keep coming... (#4020)

Sorry I'm late, but I was curious about the solution not needing to whitelist annotations individually. I love the approach, I think it's the right one, and I don't think we need to deprecate or remove the ignoredAnnotations property (that is standard, and there may still be some edge cases that can be covered that way).

However, upon code review, I think that this is missing initializations on non-static initializers, ie:

class Foo {
  private int myImmutableField;

  {
    myImmutableField = 5;
  }

  public int getMyImmutableField() {
    return myImmutableField;
  }
}

I reckon non-static initializers are not common, but we should still consider them.

However, upon code review, I think that this is missing initializations on non-static initializers

I think you're right Juan, would you open an issue? I think it is rare enough that it has low priority though. The rule also does not handle very complicated control flow yet. In pmd 7 it uses the new data flow logic which is more robust, and which already handles initializers properly