Skip to content

Conversation

@aschackmull
Copy link
Contributor

This fixes a bug in the interpretation of the subtypes/overrides csv column. Jumping to subtypes and matching members on the subtype doesn't make sense since generics mean that the signature might be different. As an example, MultivaluedMap.put has an (erased) signature that's different from the overridden Map.put due to V being instantiated to List<V>. The first two commits highlights this problem.
The fix is to match the member on the supertype and jump to overrides of that member instead of matching the overriding member on the subtype.

@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Jul 2, 2021
@aschackmull aschackmull requested a review from a team as a code owner July 2, 2021 12:48
@github-actions github-actions bot added the Java label Jul 2, 2021
@owen-mc
Copy link
Contributor

owen-mc commented Jul 2, 2021

Should the change to JakartaRsFlow.java also be made to JaxRsFlow.java?

@aschackmull
Copy link
Contributor Author

Should the change to JakartaRsFlow.java also be made to JaxRsFlow.java?

Unless it's important to keep the two synced up, then I'd say no - it would just test the same thing twice.

@owen-mc
Copy link
Contributor

owen-mc commented Jul 12, 2021

It isn't important, from a technical point of view. But it would probably be less confusing to people looking at them in future if we keep them synced up.

@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged. The following differences were found:

java

Generated file changes for java

  • Changes to framework-coverage-java.rst:
-    Others,"``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.databind``, ``com.unboundid.ldap.sdk``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``",7,8,82,,,,14,18,,
+    Others,"``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.databind``, ``com.unboundid.ldap.sdk``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``",7,12,82,,,,14,18,,
-    Totals,,84,2013,287,13,6,6,98,33,1,66
+    Totals,,84,2017,287,13,6,6,98,33,1,66
  • Changes to framework-coverage-java.csv:
- org.apache.commons.codec,,,2,,,,,,,,,,,,,,,2,
+ org.apache.commons.codec,,,6,,,,,,,,,,,,,,,6,

@smowton smowton merged commit 3ae99b9 into github:main Jul 14, 2021
@aschackmull aschackmull deleted the java/fix-csv-subtype-interpretation branch July 14, 2021 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants