externalClassExposedInAPI in dependencies ? about revapi HOT 15 OPEN

Hmm... that's an interesting idea. I still do think that by exposing a class in your API, you're becoming responsible for it in the sense that you make your users dependent on it, but I do understand your point that you don't control the class so it's kinda hard for you to influence the decisions of its authors.

In my mind, the best solution is to hide that external class and don't expose it in your API, which would shield your users from any upgrades you make to your dependency. But I do understand that that is frequently not possible (for backwards compatibility reasons or just because of the convenience the class provides).

So what do you think about this proposal? What if we had a new configuration property that would limit the "depth" at which the external class is reported? E.g. depth 1 would mean that only external classes that are directly exposed in your API would be reported. Depth 2 would mean that only external classes exposed in API + the classes that they expose would be reported, etc.

In the current version of Revapi, the depth as outlined above is not limited and so some obscure usage paths can lead to surprising results sometimes.

To add the notion of depth to the use-chain analysis is going to be a little bit involved though so I can't promise this can be done quickly. Even more so given my recent lack of time that I can dedicate to Revapi.

from revapi.

In my mind, the best solution is to hide that external class and don't expose it in your API, which would shield your users from any upgrades you make to your dependency. But I do understand that that is frequently not possible (for backwards compatibility reasons or just because of the convenience the class provides).

Does it means that if I face this issue this is because I have a public or protected attribute (or any other way to expose the type) which is typed with a class of slf4j API ?

If this is the reason, I'm not sure this is intended.
I will double check my code.

So what do you think about this proposal? What if we had a new configuration property that would limit the "depth" at which the external class is reported? E.g. depth 1 would mean that only external classes that are directly exposed in your API would be reported. Depth 2 would mean that only external classes exposed in API + the classes that they expose would be reported, etc.

I think it could make sense. 🤔

from revapi.

I will double check my code.

I didn't find anything in my code which could looks like that.
But I guess this could come from a class from a dependencies which could expose an slf4j class ?

But how to find it ?

• manually that sounds very hard.
• current revapi output does not give enough information to find that easily.

from revapi.

@metlos any idea about having more information to find easily the code which cause the error ?

from revapi.

Revapi should report an "exampleUseChainInOldApi" and "exampleUseChainInNewApi" as attachments of the problem, which I am not mistaken, should be a part of the output in the maven build. These give you one of the paths from the class in question back to the API method/type/field that introduced it.

from revapi.

Please see "reportUsesFor" in https://revapi.org/revapi-java/0.27.0/configuration.html which configures when the use-chain is attached to the problems.

from revapi.

Reading reportUsesFor, I understand that externalClassExposedInAPI is in the default list.

I guess the report you talked about is :

[INFO] Comparing [org.eclipse.leshan:leshan-core-cf:jar:1.4.2] against [org.eclipse.leshan:leshan-core-cf:jar:2.0.0-SNAPSHOT] (including their transitive dependencies).
[INFO] API problems found.
[INFO] If you're using the semver-ignore extension, update your module's version to one compatible with the current changes (e.g. mvn package revapi:update-versions). If you want to explicitly ignore these changes or provide justifications for them, add the json snippets to your Revapi configuration for the "revapi.differences" extension.

{
  "ignore": true,
  "code": "java.class.externalClassExposedInAPI",
  "new": "enum org.slf4j.event.Level",
  "justification": "ADD YOUR EXPLANATION FOR THE NECESSITY OF THIS CHANGE"
  /*  "classSimpleName": "Level",
  "exampleUseChainInNewApi": "",
  "package": "org.slf4j.event",
  "classQualifiedName": "org.slf4j.event.Level",
  "elementKind": "enum",
  "newArchive": "org.slf4j:slf4j-api:jar:2.0.3",
  "newArchiveRole": "supplementary",
  "breaksVersioningRules": "unknown",
  */

},
{
  "ignore": true,
  "code": "java.class.externalClassExposedInAPI",
  "new": "interface org.slf4j.spi.LoggingEventBuilder",
  "justification": "ADD YOUR EXPLANATION FOR THE NECESSITY OF THIS CHANGE"
  /*  "classSimpleName": "LoggingEventBuilder",
  "exampleUseChainInNewApi": "",
  "package": "org.slf4j.spi",
  "classQualifiedName": "org.slf4j.spi.LoggingEventBuilder",
  "elementKind": "interface",
  "newArchive": "org.slf4j:slf4j-api:jar:2.0.3",
  "newArchiveRole": "supplementary",
  "breaksVersioningRules": "unknown",
  */

},

I tried to find anything in LoggingEventBuilder or Level in leshan-core-cf but nothing...

And there is no exampleUseChainInOldApi value and exampleUseChainInNewApi is empty 🤔

from revapi.

Keep in mind that the only change we do is just upgrading slf4j from 1.7.36 to 2.0.3.

from revapi.

Trying to investigate a bit more, I see that the first module which fails with this error is the first module which depends from Californium.

So, I was asking myself if the issue is in californium ? 🤔
But we have this kind of configuration in Leshan :

<revapi.filter>
  <elements>
    <exclude>
      <item>
        <!--  Californium is exclude from 
              API check as it does not have clear definition of its API and do not really 
              respect Semantic versioning: https://github.com/eclipse/californium/issues/1159 
              https://github.com/eclipse/californium/issues/1166 -->
        <matcher>java-package</matcher>
        <match>/org\.eclipse\.californium(\..*)?/</match>
      </item>
    </exclude>
  </elements>
</revapi.filter>

So, I was wondering that maybe this filter affect the report and this is maybe why we didn't see anything about exampleUseChainInOldApi or exampleUseChainInNewApi

I tried to remove this revapi.filter about californium ☝️ and now revapi doesn't complain anymore 🤯 ...
I must confess that I'm totally lost 😅
Any explanation about that ? (Do I find a kind of bug or ?)

from revapi.

yes, this definitely looks like a bug

from revapi.

Thanks for the investigation, this'll definitely help diagnose wth is going on

from revapi.

I think the latest commit to main - 08b32c5 - should fix the issue. I will look into how to make these kind of errors less sneaky. I tried with mvn verify -DskipTests -pl leshan-core-cf in your project after bumping slf4j to 2.0.3 (and bumping revapi-java to locally built 0.27.1-SNAPSHOT of course) and the issue was no longer present.

from revapi.

Good news 👍

Is there anything we could do on our side to help ?

Do you have a vague idea about when a release will be available with this fix ?

from revapi.

The thing is that I don't like the fix too much and would like to understand how the bug in the package matcher was able to influence the results the way it did. There's some non-trivial class tree filtering and pruning going on and I was not able to fully understand how we got to the results we did...

from revapi.

Ok I understand so you will need more time 👍

from revapi.