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 support for external init annotations in constructors #725

Merged
merged 1 commit into from
Jan 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ public interface Config {
* Checks if annotation marks an "external-init class," i.e., a class where some external
* framework initializes object fields after invoking the zero-argument constructor.
*
* <p>Note that this annotation can be on the class itself, or on the zero-arguments constructor,
* but will be ignored anywhere else.
*
* @param annotationName fully-qualified annotation name
* @return true if classes with the annotation are external-init
*/
Expand Down
13 changes: 8 additions & 5 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
Original file line number Diff line number Diff line change
Expand Up @@ -1761,7 +1761,8 @@ private void checkFieldInitialization(ClassTree tree, VisitorState state) {
} else if (constructorInitInfo == null) {
// report it on the field, except in the case where the class is externalInit and
// we have no initializer methods
if (!(isExternalInit(classSymbol) && entities.instanceInitializerMethods().isEmpty())) {
if (!(symbolHasExternalInitAnnotation(classSymbol)
&& entities.instanceInitializerMethods().isEmpty())) {
errorBuilder.reportInitErrorOnField(
uninitField, state, buildDescription(getTreesInstance(state).getTree(uninitField)));
}
Expand Down Expand Up @@ -1868,12 +1869,14 @@ private SetMultimap<MethodTree, Symbol> checkConstructorInitialization(
SetMultimap<MethodTree, Symbol> result = LinkedHashMultimap.create();
Set<Symbol> nonnullInstanceFields = entities.nonnullInstanceFields();
Trees trees = getTreesInstance(state);
boolean isExternalInit = isExternalInit(entities.classSymbol());
boolean isExternalInitClass = symbolHasExternalInitAnnotation(entities.classSymbol());
for (MethodTree constructor : entities.constructors()) {
if (constructorInvokesAnother(constructor, state)) {
continue;
}
if (constructor.getParameters().size() == 0 && isExternalInit) {
if (constructor.getParameters().size() == 0
&& (isExternalInitClass
|| symbolHasExternalInitAnnotation(ASTHelpers.getSymbol(constructor)))) {
// external framework initializes fields in this case
continue;
}
Expand All @@ -1888,8 +1891,8 @@ private SetMultimap<MethodTree, Symbol> checkConstructorInitialization(
return result;
}

private boolean isExternalInit(Symbol.ClassSymbol classSymbol) {
return StreamSupport.stream(NullabilityUtil.getAllAnnotations(classSymbol).spliterator(), false)
private boolean symbolHasExternalInitAnnotation(Symbol symbol) {
return StreamSupport.stream(NullabilityUtil.getAllAnnotations(symbol).spliterator(), false)
.map((anno) -> anno.getAnnotationType().toString())
.anyMatch(config::isExternalInitClassAnnotation);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.uber.nullaway;

import java.util.Arrays;
import org.junit.Test;

public class NullAwayInitializationTests extends NullAwayTestsBase {
Expand Down Expand Up @@ -68,6 +69,59 @@ public void externalInitSupport() {
.doTest();
}

@Test
public void externalInitSupportConstructors() {
makeTestHelperWithArgs(
Arrays.asList(
"-d",
temporaryFolder.getRoot().getAbsolutePath(),
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
"-XepOpt:NullAway:ExternalInitAnnotations=com.uber.ExternalInitConstructor"))
.addSourceLines(
"ExternalInitConstructor.java",
"package com.uber;",
"import java.lang.annotation.ElementType;",
"import java.lang.annotation.Retention;",
"import java.lang.annotation.RetentionPolicy;",
"import java.lang.annotation.Target;",
"@Retention(RetentionPolicy.CLASS)",
"@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})",
"public @interface ExternalInitConstructor {}")
.addSourceLines(
"Test.java",
"package com.uber;",
"class Test {",
" Object f;",
// no error here due to external init
" @ExternalInitConstructor",
" public Test() {}",
" // BUG: Diagnostic contains: initializer method does not guarantee @NonNull field",
" public Test(int x) {}",
" public Test(Object o) { this.f = o; }",
"}")
.addSourceLines(
"Test2.java",
"package com.uber;",
"class Test2 {",
" // BUG: Diagnostic contains: @NonNull field f not initialized",
" Object f;",
// must be on a constructor!
" @ExternalInitConstructor",
" public void init() {}",
"}")
.addSourceLines(
"Test3.java",
"package com.uber;",
"class Test3 {",
" Object f;",
// Must be zero-args constructor!
" @ExternalInitConstructor",
" // BUG: Diagnostic contains: initializer method does not guarantee @NonNull field",
" public Test3(int x) {}",
"}")
.doTest();
}

@Test
public void externalInitSupportFields() {
defaultCompilationHelper
Expand Down