Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have Refaster resolve classes against the appropriate modules #1239

Conversation

Stephan202
Copy link
Contributor

Looking up classes only in java.base seems to work fine on JDK 8, but fails on
JDK 9+. This causes Refaster templates that reference non-JDK types to silently
not match in places where they should. By resolving classes against the
appropriate modules the problem goes away.

Prior to this change the newly added test would pass when executed on JDK 8,
but fail on JDK 9+.

Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Mar 2, 2019
Looking up classes only in java.base seems to work fine on JDK 8, but fails on
JDK 9+. This causes Refaster templates that reference non-JDK types to silently
not match in places where they should. By resolving classes against the
appropriate modules the problem goes away.

Prior to this change the newly added test would pass when executed on JDK 8,
but fail on JDK 9+.

See google#1239
@Stephan202
Copy link
Contributor Author

@lowasser would it be possible to have this change merged in time for the next release? Without it a significant portion of Refaster templates simply don't match on JDK 9+.

Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Jul 5, 2019
Looking up classes only in java.base seems to work fine on JDK 8, but fails on
JDK 9+. This causes Refaster templates that reference non-JDK types to silently
not match in places where they should. By resolving classes against the
appropriate modules the problem goes away.

Prior to this change the newly added test would pass when executed on JDK 8,
but fail on JDK 9+.

See google#1239
@Stephan202 Stephan202 force-pushed the bugfix/refaster-on-jdk-9-plus branch from 6a00996 to 2cc5fe4 Compare August 1, 2019 09:37
Looking up classes only in java.base seems to work fine on JDK 8, but fails on
JDK 9+. This causes Refaster templates that reference non-JDK types to silently
not match in places where they should. By resolving classes against the
appropriate modules the problem goes away.

Prior to this change the newly added test would pass when executed on JDK 8,
but fail on JDK 9+.
@Stephan202 Stephan202 force-pushed the bugfix/refaster-on-jdk-9-plus branch from 2cc5fe4 to b3ee169 Compare September 28, 2019 19:49
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Dec 7, 2019
Looking up classes only in java.base seems to work fine on JDK 8, but fails on
JDK 9+. This causes Refaster templates that reference non-JDK types to silently
not match in places where they should. By resolving classes against the
appropriate modules the problem goes away.

Prior to this change the newly added test would pass when executed on JDK 8,
but fail on JDK 9+.

See google#1239
@knutwannheden
Copy link

Does this effectively mean that I cannot use Refaster in a Java 11 code base, where I also want to write Refaster rules pertaining to the types in said code base?

@Stephan202
Copy link
Contributor Author

@knutwannheden indeed, many interesting Refaster rules will not match without these changes. I wonder whether inside Google no code has been migrated to Java 9+ yet, because otherwise they should have hit this same issue 🤔 .

Within our company we work around this issue (and a small number of others) by maintaining a fork. It is available on Jitpack. I tagged version v2.3.4-picnic-2 this weekend; it has a delta of 6 commits commits compared to the official version 2.3.4.

@knutwannheden
Copy link

@Stephan202 Thanks for the heads up. I am new to Refaster and it took me a while to find this PR. I thought I had hit some corner case with generics.

I will give your fork a try. Thanks for the tip!

@knutwannheden
Copy link

@Stephan202 Using your forked error-prone build I was indeed able to write and apply Refaster rules against Java 11 code. Very nice! Now it would be nice if this PR could also be integrated...

@Xcelled
Copy link

Xcelled commented Mar 25, 2020

This bit us, too. It seemed like refaster just wasn't working due to lack of any obvious error. I had to spend a while spelunking through refaster to find what was wrong, and had created a band-aid fix before seeing this PR. It'd be great to get this merged, as more of the world moves to JDK 9+.

nick-someone pushed a commit that referenced this pull request Mar 27, 2020
Looking up classes only in java.base seems to work fine on JDK 8, but fails on
JDK 9+. This causes Refaster templates that reference non-JDK types to silently
not match in places where they should. By resolving classes against the
appropriate modules the problem goes away.

Prior to this change the newly added test would pass when executed on JDK 8,

Fixes #1239

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=303219121
This was referenced Mar 27, 2020
nick-someone pushed a commit that referenced this pull request Mar 30, 2020
Looking up classes only in java.base seems to work fine on JDK 8, but fails on
JDK 9+. This causes Refaster templates that reference non-JDK types to silently
not match in places where they should. By resolving classes against the
appropriate modules the problem goes away.

Prior to this change the newly added test would pass when executed on JDK 8,

Fixes #1239

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=303219121
nick-someone pushed a commit that referenced this pull request Mar 30, 2020
Looking up classes only in java.base seems to work fine on JDK 8, but fails on
JDK 9+. This causes Refaster templates that reference non-JDK types to silently
not match in places where they should. By resolving classes against the
appropriate modules the problem goes away.

Prior to this change the newly added test would pass when executed on JDK 8,

Fixes #1239

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=303219121
@Stephan202 Stephan202 deleted the bugfix/refaster-on-jdk-9-plus branch December 20, 2020 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants