No compiler message when referencing an inaccessible variable

[lmove]

The Maven compiler does not report an error when referencing a variable that cannot be accessed anymore (e.g. class no longer public, class less accessible). The broken use is correctly reported but there is no matching compiler message (true positives reported as false positives).

Related broken use:

Path: path-to/maracas/test-data/comp-changes/client/src/mainclient/classNoLongerPublic/ClassNoLongerPublicTD.java
Line: 10
Breaking change: CLASS_LESS_ACCESSIBLE
API use: TYPE_DEPENDENCY
Used declaration: main.classNoLongerPublic.ClassNoLongerPublic
Source declaration: main.classNoLongerPublic.ClassNoLongerPublic

[lmove]

As discussed in a previous meeting, these cases should be manually inspected and reclassified accordingly. As mentioned before, there is a mismatch between the JDT and the Maven compiler. Nevertheless, it is worth reflecting upon the use of reporting this broken use: any reference to the field whose type has been impacted will be reported as an error. Have a look at this method:

maracas/core/src/main/java/com/github/maracas/visitors/BreakingChangeVisitor.java

Line 91 in cdf2a33

public APIUse getAPIUseByRole(CtElement element) {

.

[tdegueul]

My understanding is that the code you're referring to is only invoked when a type is removed/not visible anymore, and it attempts to find every place in the code where this type was used (as a field type, as a parameter type, a variable declaration, an extension/implementation, or anything).

What do you mean with "any reference to the field whose type has been impacted"? Does it mean that, e.g., if T is removed from the library and there's a field T f; in the client, any reference to f in client code will also be reported as a broken use? If that's the case, I don't think it is right.

[lmove]

That's exactly the situation we have here. Maracas is actually reporting such situations where we reference f. Do we need to avoid reporting such cases?

On a side note, this case is not reported anymore given that I added the requirement to only consider the breaking changes that are to be expected for a specific package (see #34 (comment)).

[tdegueul]

So, if T is removed and the client has:

class C {
  T f;

  void foo() {
    f = null;
  }
}

lines 2 and 5 would be impacted?

That's wrong imho, only line 2 should be a broken use. But it makes sense that it happens: f = null indirectly refers to type T from the perspective of Spoon, so the code you mentioned above will trigger.

[lmove]

Yeap, that's the situation we have right now. The JDT compiler also reports these cases but Maven does not. I don't think it is right or wrong but more related to the information we want to share with the developer. Having these additional broken uses give added value to them? Personally, I think it will depend on the case. Suppose that instead of setting f to null, you initialize it with the constructor of one of C subtypes. Then, you really need to modify that line. In this line of thought, I would be inclined to report these extra cases. Wdyt?