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

MultipleNullnessAnnotations inconsistent on nested class #4320

Closed
xenoterracide opened this issue Mar 12, 2024 · 5 comments · Fixed by #4333
Closed

MultipleNullnessAnnotations inconsistent on nested class #4320

xenoterracide opened this issue Mar 12, 2024 · 5 comments · Fixed by #4333

Comments

@xenoterracide
Copy link

xenoterracide commented Mar 12, 2024

supressing the warning on the nested static class fixes the error, no suppression is needed on the outer class. This is inconsistent and makes no sense to me.

/home/xeno/IdeaProjects/spring-app-commons/module/jpa/src/main/java/com/xenoterracide/jpa/AbstractUuidEntityBase.java:112: warning: [MultipleNullnessAnnotations] This type use has conflicting nullness annotations
package com.xenoterracide.jpa;

import jakarta.validation.constraints.NotNull;
import java.util.UUID;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;

public abstract class AbstractUuidEntityBase<ID extends AbstractUuidEntityBase.AbstractIdentity> {

  @NotNull
  private @Nullable ID id;

  protected AbstractUuidEntityBase(@NonNull ID id) {
    this.id = id;
  }

  public abstract static class AbstractIdentity {

    @NotNull
    private @Nullable UUID id;

    protected AbstractIdentity(@NonNull UUID id) {
      this.id = id;
    }
  }
}
@cushon
Copy link
Collaborator

cushon commented Mar 12, 2024

Suppressions should be handled on any syntactically enclosing scope, i.e. I'd expect any of the following to suppress the diagnostic. Can you elaborate on how the behaviour you're seeing is inconsistent / surprising?

class AbstractUuidEntityBase {
  class AbstractIdentity {
    @SuppressWarnings("MultipleNullnessAnnotations")
    @NotNull @Nullable UUID id; // This type use has conflicting nullness annotations
  }
}
class AbstractUuidEntityBase {
  @SuppressWarnings("MultipleNullnessAnnotations")
  class AbstractIdentity {
    @NotNull
    private @Nullable UUID id; // This type use has conflicting nullness annotations
  }
}
@SuppressWarnings("MultipleNullnessAnnotations")
class AbstractUuidEntityBase {
  class AbstractIdentity {
    @NotNull @Nullable UUID id; // This type use has conflicting nullness annotations
  }
}

@xenoterracide
Copy link
Author

xenoterracide commented Mar 12, 2024

The problem is that in my code I only needed an annotation on the nested field, I didn't need it on the outer fields. I only got the error once. Oh, and to emphasize, I put it on the field in the nested class, not on the class itself, so I wouldn't expect it to have any impact on the outer class or other fields.

@cushon
Copy link
Collaborator

cushon commented Mar 12, 2024

I am not sure I follow. Can you share a minimal example that reproduces the issue you're experiencing?

@xenoterracide
Copy link
Author

xenoterracide commented Mar 13, 2024

I updated the original example. It only generates 1 error (with this rule), it should generate 2. This will fix it.

This code doesn't make much sense, but it's the minimal example. Obviously real world I need that no arg constructor, it's not required for the example though.

package com.xenoterracide.jpa;

import jakarta.validation.constraints.NotNull;
import java.util.UUID;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;

public abstract class AbstractUuidEntityBase<ID extends AbstractUuidEntityBase.AbstractIdentity> {

  @NotNull
  private @Nullable ID id;

  protected AbstractUuidEntityBase(@NonNull ID id) {
    this.id = id;
  }

  public abstract static class AbstractIdentity {

    @NotNull
    @SuppressWarnings("MultipleNullnessAnnotations")
    private @Nullable UUID id;

    protected AbstractIdentity(@NonNull UUID id) {
      this.id = id;
    }
  }
}

@cushon
Copy link
Collaborator

cushon commented Mar 13, 2024

Thanks. I think there is an issue with the annotation on the type parameter use of ID not being visible to Error Prone.

Repro:

import jakarta.validation.constraints.NotNull;
import org.jspecify.annotations.Nullable;

class T<X> {
  @NotNull @Nullable X one;
  @NotNull @Nullable int two;
}
javac -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED   -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED   -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED   -J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED   -J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED   -J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED   -J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED   -J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED   -J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED   -J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED   -XDcompilePolicy=simple   -processorpath error_prone_core-${EP_VERSION?}-with-dependencies.jar:dataflow-errorprone-${DATAFLOW_VERSION?}.jar -cp jakarta.validation-api-3.1.0-M1.jar:jspecify-0.3.0.jar   '-Xplugin:ErrorProne -XepDisableAllChecks -Xep:MultipleNullnessAnnotations:ERROR'   T.java

Expected: findings on both one and two.

Actual: the finding shows up only on two:

T.java:8: error: [MultipleNullnessAnnotations] This type use has conflicting nullness annotations
  @NotNull @Nullable int two;
                         ^
    (see https://errorprone.info/bugpattern/MultipleNullnessAnnotations)
1 error

copybara-service bot pushed a commit that referenced this issue Mar 19, 2024
…om type variable uses

For whatever reason, the type annotations cannot be accessed using the `Tree`s for the corresponding AST node for those types (`getType(methodTree.getReturnType())` and `getType(variableTree.getType())`.

Fixes #4320

PiperOrigin-RevId: 615641851
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants