[java] LawOfDemeter: False positive with fields assigned to local vars

[linusjf]

#2160 (comment)
Affects PMD Version:
6.20.0.

Rule:
LawOfDemeter.

Description:
Assigning an instance or local variables (created locally) should not be flagged as an object created elsewhere by this rule.

Code Sample demonstrating the issue:

package pmdtests;

public class AddressUsingCopyOnWrite {
  private volatile AddressValue addressValue;

 public AddressUsingCopyOnWrite(String       street, String city, String phone) {
   this.addressValue = new                    AddressValue(street, city, phone);
   }
  

 // thread-safe method
 @Override
  public String toString() {
    AddressValue local = addressValue;
    return "street=" + local.getStreet() + ",city=" + local.getCity()
        + ",phoneNumber=" + local.getPhoneNumber();
  }

 
}

Running PMD through: [CLI | Ant | Maven | Gradle | Designer | Other]

[adangel]

Not sure, if I understand this correctly.
Here's an example how a locally created variable would look: AdressValue local = new AddressValue("foo", "bar", ...)

In your code snippet, we assign a field to a local variable - the field is our own, that's why we should not flag access to local. Is this correct?

So, it seems, that we don't consider fields when checking local variable initializers (I think, we check for method parameters, though).

[linusjf]

(It would help if you came back to me within a shorter period of the issue written up. It takes some time to recall why it was written up in the first place. )

I'd expect the field assignment was to a locally created instance, not created elsewhere as specified by the rule. Perhaps, the code sample ought to have examples of fields initialised in both ways to check whether that's the case.

#2188 (comment)

What are you driving at? Are you saying that all aliases i.e., using assignment operator (a new reference, if you may) should not be flagged as created elsewhere? Yes, of course. Add another check for that in the source if you like. If an object is created locally, all references to the object must be considered local as well. Why on earth would it be otherwise?

As far as I can tell, there are two ways to tackle this. Track the original variable or reference and all aliases or references will have the same property i.e., created locally or externally. Or ignore all such assignments since the original reference will be flagged by the rule, anyway. The former involves more work and effort. Do you see any side effects of the latter? Evidently, further violations of Law of Demeter will not be flagged if an externally created object's alias is treated as a local variable.
Am I complicating this too much or should we simply write test code and see how the existing implementation stacks up?

[jsotuyod]

This is fixed in PMD 7.0.0-rc1. It won't be backported to PMD 6.