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

Fix annotation testing could be interrupted #11460

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LemonCaramel
Copy link
Contributor

comment

Also, the issue of failing tests in nested classes has been resolved.

@LemonCaramel LemonCaramel requested a review from a team as a code owner October 1, 2024 15:57
@kennytv
Copy link
Member

kennytv commented Oct 1, 2024

Not sure if nullmarked actually applies to inner classes, those might be valid fails never mind

}
}

+ // Paper start - skip class if it's @NullMarked
+ private static boolean isClassNullMarked(@NotNull ClassNode clazz) {
+ private static boolean isClassNullMarked(@NotNull ClassNode clazz, @NotNull Map<String, ClassNode> allClasses) {
+ if (clazz.nestHostClass != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

While technically useless nitpick, this would fail if the annotation was present on the inner class but not on the outer one?

E.g. this may instead have to be something like

private static boolean isClassNullMarked(@NotNull ClassNode clazz, @NotNull Map<String, ClassNode> allClasses) {
    if (isClassNullMarked0(clazz)) return true;

    String parentClass = clazz.nestHostClass;
    while (parentClass != null) {
        final ClassNode parentClassNode = allClasses.get(parentClass);
        if (parentClassNode == null) return false;

        if (isClassNullMarked0(parentClassNode)) return true;

        parentClass = parentClassNode.nestHostClass;
    }
    return false;
}

if we truely care about supporting inner types correctly in this. Not that that is really important lol

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

Successfully merging this pull request may close these issues.

3 participants