Should ASTHelpers.getSymbol(Tree) be annotated with @Nullable ?

[XN137]

according to the method javadoc it can return null in multiple cases:

error-prone/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java

Lines 244 to 250 in 1c3c09f

	/**
	* Gets the symbol for a tree. Returns null if this tree does not have a symbol because it is of
	* the wrong type, if {@code tree} is null, or if the symbol cannot be found due to a compilation
	* error.
	*/
	// TODO(eaftan): refactor other code that accesses symbols to use this method
	public static Symbol getSymbol(Tree tree) {

one of them being that it calls getDeclaredSymbol which is marked @Nullable already:

error-prone/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java

Lines 219 to 224 in 1c3c09f

	/**
	* Gets the symbol declared by a tree. Returns null if {@code tree} does not declare a symbol or
	* is null.
	*/
	@Nullable
	public static Symbol getDeclaredSymbol(Tree tree) {

this seems kind of inconsistent.

there are of course multiple current usage spots where from the context + outer checks it is clear that it will not return null for example around AssignmentTree.getVariable() here:

error-prone/core/src/main/java/com/google/errorprone/bugpatterns/FieldCanBeLocal.java

Lines 262 to 269 in 1c3c09f

	Symbol rhsSymbol = getSymbol(assignmentTree.getExpression());
	// If the RHS of the assignment is a variable with the same name as the field, just remove
	// the assignment.
	String assigneeName = getSymbol(assignmentTree.getVariable()).getSimpleName().toString();
	if (rhsSymbol != null
	&& assignmentTree.getExpression() instanceof IdentifierTree
	&& rhsSymbol.getSimpleName().contentEquals(assigneeName)) {

but if the goal of the @Nullable annotation is to systematically force proper null handling maybe the annotation should be added.

WDYT ?
if it is an intentional decision, maybe there should be a comment about it to make it look less inconsistent.

[graememorgan]

Yes, I think so. I wonder if I/we accidentally deleted that at some point.

[XN137]

thanks for the quick feedback

closing since fixed by e08edcd