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

Docs: @Nullable/@NotNull on a type declaration? #51

Open
CasualSuperman opened this issue Jun 3, 2021 · 1 comment
Open

Docs: @Nullable/@NotNull on a type declaration? #51

CasualSuperman opened this issue Jun 3, 2021 · 1 comment

Comments

@CasualSuperman
Copy link

The documentation for @Nullable and @NotNull explicitly state the actions that these annotations apply to when used with methods, parameters, and variables/fields, but don't mention what it means when these annotations are used on a class.

An element annotated with Nullable claims null value is perfectly valid to return (for methods), pass to (parameters) or hold in (local variables and fields).

I tried annotating a class with @NotNull and @Nullable to see if IntelliJ's static analysis could provide hints about what's expected, but no warnings were generated for anything I did with it (beyond some "always-true" warnings for various combinations, but these were also generated without the class-level annotation). I've included the class below and documented my assumptions as a sanity-check.

I can think of two possible (likely) meanings:

  1. Variables, fields, parameters. and returns declared as the annotated type should default to the annotated nullability
  2. The fields, parameters, and/or return types within the annotated type should default to the annotated nullability

For the first option, I'm unsure what it would mean to apply @NotNull to a class given the strict constraints on using it with, for example, Map::get. There's no way a class can ever guarantee that it can or cannot be null everywhere it's used[*], since anyone can write MyClass myClass = null; (although that could generate a warning). However, if the strictness for using these annotations is more relaxed than with fields/methods/etc. and is more of a suggestion, then perhaps java.util.Optional should be annotated with @NotNull?

The second option also seems possible given my experience with other projects, but unlikely. Some class-level annotations are "distributed" into the class, such as Lombok's @Getter). However, I don't know of any annotation libraries that treat @Nullable/@NotNull this way, so I wouldn't assume that's how this annotation behaves. Plus, given the stance in #18, it seems this kind of functionality isn't desired (at least by JetBrains).

The official Javadoc for ElementType specifically mentions a hypothetical @NonNull annotation applied to a class via TYPE_USE, but I'm not clear on what it's trying to say.

The TYPE_USE constant includes class and interface declarations and type parameter declarations as a convenience for designers of type checkers which give semantics to annotation interfaces. For example, if the annotation interface NonNull is meta-annotated with @Target(ElementType.TYPE_USE), then @NonNull class C {...} could be treated by a type checker as indicating that all variables of class C are non-null, while still allowing variables of other classes to be non-null or not non-null based on whether @NonNull appears at the variable's declaration.

Does "variables of class C" refer to variables declared within class C, or variables with a type of C? I believe it's the latter, but IntelliJ's code analysis doesn't appear to use this information in this way (or at all).

In summary, what I'm requesting is:

  1. Clarification of which of the scenarios in the example should be influenced by a type-level nullity annotation (plus any obvious ones I missed)
  2. Updates to the documentation for @NotNull/@Nullable with the expected semantics (or lack thereof?) of annotating a type declaration with these annotations
  3. If any of the scenarios should be influenced, I'd like IntelliJ's static analysis updated to report them.

[*] Ignoring Project Valalla, but from my understanding we would already be able to detect that a class is inline without an annotation

Tested using Java 8 & 11 with IntelliJ Ultimate 2021.1.2:

import java.util.concurrent.ThreadLocalRandom;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@NotNull
public class Example {
	private Integer field1;
	private int     field2;

	// Class-level @NotNull doesn't seem to influence fields, since this assignment doesn't produce a warning
	public void setField1(@Nullable Integer value) {
		this.field1 = value;
	}

	// Class-level @NotNull/@Nullable don't seem to influence method parameters, since this (questionably) doesn't
	// produce a warning with either class annotation, nor with an absent class-level annotation.
	public void setField2(Integer value) {
		this.field2 = value;
	}

	protected Boolean validate() {
		Integer value1 = this.field1;
		Integer value2 = this.field2;
		// This is included solely to prevent an unavoidable constant conditions warning on the return, since
		// field2 is a primitive. I'm avoiding a suppression comment so that other warnings can still appear.
		if (ThreadLocalRandom.current().nextBoolean()) {
			value2 = this.field1;
		}
		// Class-level @NotNull doesn't seem to influence method return types, since possibly returning null
		// here doesn't produce a warning when the class is annotated with @NotNull.
		return (value2 != null && value1 == null) ? null : true;
	}

	// Annotating the following method's parameter with @NotNull/@Nullable does not produce any warnings beyond
	// the null-check when the parameter is @NotNull, so a parameter of the annotated class's type doesn't appear
	// to default to the type's annotation, nor is a conflicting declaration considered invalid.

	// If nullity was expected to match the type's annotation, this would produce a warning if the parameter was
	// annotated with the opposite annotation.
	public static boolean validate(Example example) {
		// If parameter nullity was inherited from the parameter's type in the absence of an explicit
		// annotation this would produce a warning when the type was annotated @NotNull, even when
		// the parameter doesn't have an annotation
		if (example == null) {
			return false;
		}
		// Likewise, if the previous null check is removed and the class is annotated @Nullable, this would
		// produce a warning if nullity is inherited, but it does not, so @Nullable is also not inherited.
		Boolean value = example.validate();
		if (value == null) {
			return false;
		}
		return value;
	}

	// Annotating the following method's return type with @NotNull/@Nullable does not produce warnings beyond
	// returning null when the method is @NotNull, so a return type's nullity doesn't appear to default to the
	// type's annotation, nor is a conflicting declaration considered invalid.

	// If nullity was expected to match the class's annotation, this would produce a warning when the return
	// type was annotated the opposite annotation.
	public static Example create() {
		Example ex = new Example();
		if (ex.validate() == Boolean.TRUE) {
			return ex;
		}
		// If nullity for return types defaulted to the type's nullity, this would produce a warning when the
		// type was annotated @NotNull.
		return null;
	}
}
@amaembo
Copy link
Collaborator

amaembo commented Jun 4, 2021

Hello! Annotations on class should be considered illegal and should be reported.

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

No branches or pull requests

2 participants