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

Add a utility class for checking if Guice has enhanced a class (and returning the unenhanced version). #1689

Closed
wants to merge 0 commits into from

Conversation

copybara-service[bot]
Copy link

@copybara-service copybara-service bot commented Apr 18, 2023

Add a utility class for checking if Guice has enhanced a class (and returning the unenhanced version).

(Also changes the classname suffix to be a hex string instead of an int, which had previously included a leading negative sign, which was weird.)

Fixes #1340 & fixes #187.

@copybara-service copybara-service bot changed the title Add a utility class for checking if Guice has enhanced a class (and returning the unenahnced version). Add a utility class for checking if Guice has enhanced a class (and returning the unenhanced version). Apr 18, 2023
@sameb
Copy link
Member

sameb commented Apr 18, 2023

@mcculls -- Would you be able to review this PR before I submit it?

There's a couple dozen places in google's codebase where we could use it, and presumably a bunch more in the wild.

Also LMK if you think this it's better to avoid adding this API at all.

Thanks!

(For context, I'm spending this & next week scrubbing Guice's open PRs & issues as a tribute to @crazybob.)


/** Returns true if this is a class that Guice enhanced with AOP functionality. */
public static boolean isEnhanced(Class<?> clazz) {
return ENHANCED.matcher(clazz.getSimpleName()).matches();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wanted to simplify this then just checking the simple name contained the BytecodeGen.ENHANCER_BY_GUICE_MARKER string would IMHO be enough.

if (!isEnhanced(clazz)) {
return Optional.empty();
}
return Optional.of(clazz.getSuperclass());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the ternary style easier to read, but that's just a matter of taste :)

return isEnhanced(clazz) ? Optional.of(clazz.getSuperclass()) : Optional.empty();

Copy link
Contributor

@mcculls mcculls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sameb
Copy link
Member

sameb commented Apr 22, 2023

Thanks for reviewing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All enhanced classes should implement a marker interface
2 participants