From dfcdc9ccb02987cae76f9027ed9735b8f9d1c3d2 Mon Sep 17 00:00:00 2001 From: Brad Corso Date: Tue, 15 Oct 2024 11:18:27 -0700 Subject: [PATCH] Remove the `ignorePrivateAndStaticInjectionForComponent` compiler option. This option was only used by the TCK tests, so I've replaced the compiler option with a hard-coded check for the TCK package. As was stated in the javadoc for the flag, it was not meant to be used for other purposes, so removing it should be fine. This change also fixes a some memory bugs in how `InjectValidator` was previously implemented. 1. Now, only a single validator instance is used in the common case where `privateMemberDiagnostic`/`staticMemberDiagnostic` have the default value (`ERROR`). Previously, this resulted in at least 3 instances being used, which duplicated caching across all three instances 2. Now, both `validator` and `validatorWhenGeneratingCode` have their caches cleared between rounds. Previously, only `validator` had its cache cleared because `validatorWhenGeneratingCode` was being created and managed outside Dagger. This also created a memory leak in one case because it was used within a scoped binding and it's cache was never cleared. RELNOTES=Remove the `ignorePrivateAndStaticInjectionForComponent` compiler option. PiperOrigin-RevId: 686174065 --- .../InjectBindingValidator.java | 5 +- .../compileroption/CompilerOptions.java | 9 - .../ProcessingEnvironmentCompilerOptions.java | 8 - .../javac/JavacPluginCompilerOptions.java | 5 - .../validation/InjectBindingRegistryImpl.java | 4 +- .../codegen/validation/InjectValidator.java | 619 +++++++++--------- 6 files changed, 319 insertions(+), 331 deletions(-) diff --git a/java/dagger/internal/codegen/bindinggraphvalidation/InjectBindingValidator.java b/java/dagger/internal/codegen/bindinggraphvalidation/InjectBindingValidator.java index 21512958dca..6455f2985fd 100644 --- a/java/dagger/internal/codegen/bindinggraphvalidation/InjectBindingValidator.java +++ b/java/dagger/internal/codegen/bindinggraphvalidation/InjectBindingValidator.java @@ -33,7 +33,7 @@ final class InjectBindingValidator extends ValidationBindingGraphPlugin { @Inject InjectBindingValidator(InjectValidator injectValidator) { - this.injectValidator = injectValidator.whenGeneratingCode(); + this.injectValidator = injectValidator; } @Override @@ -50,7 +50,8 @@ public void visitGraph(BindingGraph bindingGraph, DiagnosticReporter diagnosticR private void validateInjectionBinding(Binding node, DiagnosticReporter diagnosticReporter) { ValidationReport typeReport = - injectValidator.validate(node.key().type().xprocessing().getTypeElement()); + injectValidator.validateWhenGeneratingCode( + node.key().type().xprocessing().getTypeElement()); for (Item item : typeReport.allItems()) { diagnosticReporter.reportBinding(item.kind(), node, item.message()); } diff --git a/java/dagger/internal/codegen/compileroption/CompilerOptions.java b/java/dagger/internal/codegen/compileroption/CompilerOptions.java index b34e8e1d6b0..62db93ba7f8 100644 --- a/java/dagger/internal/codegen/compileroption/CompilerOptions.java +++ b/java/dagger/internal/codegen/compileroption/CompilerOptions.java @@ -54,15 +54,6 @@ public final boolean doCheckForNulls() { */ public abstract boolean includeStacktraceWithDeferredErrorMessages(); - /** - * If {@code true}, Dagger will generate factories and components even if some members-injected - * types have {@code private} or {@code static} {@code @Inject}-annotated members. - * - *

This should only ever be enabled by the TCK tests. Disabling this validation could lead to - * generating code that does not compile. - */ - public abstract boolean ignorePrivateAndStaticInjectionForComponent(); - public abstract ValidationType scopeCycleValidationType(); /** diff --git a/java/dagger/internal/codegen/compileroption/ProcessingEnvironmentCompilerOptions.java b/java/dagger/internal/codegen/compileroption/ProcessingEnvironmentCompilerOptions.java index bd112126a9c..7e9eb41595e 100644 --- a/java/dagger/internal/codegen/compileroption/ProcessingEnvironmentCompilerOptions.java +++ b/java/dagger/internal/codegen/compileroption/ProcessingEnvironmentCompilerOptions.java @@ -29,7 +29,6 @@ import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.FLOATING_BINDS_METHODS; import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.FORMAT_GENERATED_SOURCE; import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.GENERATED_CLASS_EXTENDS_COMPONENT; -import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.IGNORE_PRIVATE_AND_STATIC_INJECTION_FOR_COMPONENT; import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.IGNORE_PROVISION_KEY_WILDCARDS; import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.INCLUDE_STACKTRACE_WITH_DEFERRED_ERROR_MESSAGES; import static dagger.internal.codegen.compileroption.ProcessingEnvironmentCompilerOptions.Feature.PLUGINS_VISIT_FULL_BINDING_GRAPHS; @@ -139,11 +138,6 @@ public boolean includeStacktraceWithDeferredErrorMessages() { return isEnabled(INCLUDE_STACKTRACE_WITH_DEFERRED_ERROR_MESSAGES); } - @Override - public boolean ignorePrivateAndStaticInjectionForComponent() { - return isEnabled(IGNORE_PRIVATE_AND_STATIC_INJECTION_FOR_COMPONENT); - } - @Override public ValidationType scopeCycleValidationType() { return parseOption(DISABLE_INTER_COMPONENT_SCOPE_VALIDATION); @@ -325,8 +319,6 @@ enum Feature implements EnumOption { INCLUDE_STACKTRACE_WITH_DEFERRED_ERROR_MESSAGES, - IGNORE_PRIVATE_AND_STATIC_INJECTION_FOR_COMPONENT, - EXPERIMENTAL_AHEAD_OF_TIME_SUBCOMPONENTS, FORCE_USE_SERIALIZED_COMPONENT_IMPLEMENTATIONS, diff --git a/java/dagger/internal/codegen/javac/JavacPluginCompilerOptions.java b/java/dagger/internal/codegen/javac/JavacPluginCompilerOptions.java index c2f42506680..a833c02e5f0 100644 --- a/java/dagger/internal/codegen/javac/JavacPluginCompilerOptions.java +++ b/java/dagger/internal/codegen/javac/JavacPluginCompilerOptions.java @@ -66,11 +66,6 @@ public boolean includeStacktraceWithDeferredErrorMessages() { return false; } - @Override - public boolean ignorePrivateAndStaticInjectionForComponent() { - return false; - } - @Override public ValidationType scopeCycleValidationType() { return NONE; diff --git a/java/dagger/internal/codegen/validation/InjectBindingRegistryImpl.java b/java/dagger/internal/codegen/validation/InjectBindingRegistryImpl.java index 152a4804914..ab2a5711a81 100644 --- a/java/dagger/internal/codegen/validation/InjectBindingRegistryImpl.java +++ b/java/dagger/internal/codegen/validation/InjectBindingRegistryImpl.java @@ -84,7 +84,6 @@ final class InjectBindingRegistryImpl implements InjectBindingRegistry { private final XProcessingEnv processingEnv; private final XMessager messager; private final InjectValidator injectValidator; - private final InjectValidator injectValidatorWhenGeneratingCode; private final KeyFactory keyFactory; private final BindingFactory bindingFactory; private final CompilerOptions compilerOptions; @@ -106,7 +105,7 @@ void generateBindings(SourceFileGenerator generator) throws SourceFileGenerat checkState(!binding.unresolved().isPresent()); XType type = binding.key().type().xprocessing(); if (!isDeclared(type) - || injectValidatorWhenGeneratingCode.validate(type.getTypeElement()).isClean()) { + || injectValidator.validateWhenGeneratingCode(type.getTypeElement()).isClean()) { generator.generate(binding); } materializedBindingKeys.add(binding.key()); @@ -223,7 +222,6 @@ private void tryToCacheBinding(B binding) { this.processingEnv = processingEnv; this.messager = messager; this.injectValidator = injectValidator; - this.injectValidatorWhenGeneratingCode = injectValidator.whenGeneratingCode(); this.keyFactory = keyFactory; this.bindingFactory = bindingFactory; this.compilerOptions = compilerOptions; diff --git a/java/dagger/internal/codegen/validation/InjectValidator.java b/java/dagger/internal/codegen/validation/InjectValidator.java index a3af669b4c5..135839dd92c 100644 --- a/java/dagger/internal/codegen/validation/InjectValidator.java +++ b/java/dagger/internal/codegen/validation/InjectValidator.java @@ -56,7 +56,6 @@ import javax.inject.Inject; import javax.inject.Singleton; import javax.tools.Diagnostic; -import javax.tools.Diagnostic.Kind; /** * A {@linkplain ValidationReport validator} for {@link Inject}-annotated elements and the types @@ -66,14 +65,12 @@ public final class InjectValidator implements ClearableCache { private final XProcessingEnv processingEnv; - private final CompilerOptions compilerOptions; private final DependencyRequestValidator dependencyRequestValidator; - private final Optional privateAndStaticInjectionDiagnosticKind; private final InjectionAnnotations injectionAnnotations; private final DaggerSuperficialValidation superficialValidation; - private final Map provisionReports = new HashMap<>(); - private final Map membersInjectionReports = new HashMap<>(); private final MethodSignatureFormatter methodSignatureFormatter; + private final InternalValidator validator; + private final InternalValidator validatorWhenGeneratingCode; @Inject InjectValidator( @@ -83,376 +80,390 @@ public final class InjectValidator implements ClearableCache { InjectionAnnotations injectionAnnotations, DaggerSuperficialValidation superficialValidation, MethodSignatureFormatter methodSignatureFormatter) { - this( - processingEnv, - compilerOptions, - dependencyRequestValidator, - Optional.empty(), - injectionAnnotations, - superficialValidation, - methodSignatureFormatter); - } - - private InjectValidator( - XProcessingEnv processingEnv, - CompilerOptions compilerOptions, - DependencyRequestValidator dependencyRequestValidator, - Optional privateAndStaticInjectionDiagnosticKind, - InjectionAnnotations injectionAnnotations, - DaggerSuperficialValidation superficialValidation, - MethodSignatureFormatter methodSignatureFormatter) { this.processingEnv = processingEnv; - this.compilerOptions = compilerOptions; this.dependencyRequestValidator = dependencyRequestValidator; - this.privateAndStaticInjectionDiagnosticKind = privateAndStaticInjectionDiagnosticKind; this.injectionAnnotations = injectionAnnotations; this.superficialValidation = superficialValidation; this.methodSignatureFormatter = methodSignatureFormatter; + + // When validating types that require a generated factory class we need to error on private and + // static inject members even if the compiler options are set to not error. + this.validatorWhenGeneratingCode = + new InternalValidator(Diagnostic.Kind.ERROR, Diagnostic.Kind.ERROR); + + // When validating types that might not require a generated factory we can take the user flags + // for private and static inject members into account, but try to reuse the existing one if the + // diagnostic kinds are the same. + this.validator = + (compilerOptions.privateMemberValidationKind() == Diagnostic.Kind.ERROR + && compilerOptions.staticMemberValidationKind() == Diagnostic.Kind.ERROR) + ? validatorWhenGeneratingCode + : new InternalValidator( + compilerOptions.privateMemberValidationKind(), + compilerOptions.staticMemberValidationKind()); } @Override public void clearCache() { - provisionReports.clear(); - membersInjectionReports.clear(); + validator.clearCache(); + validatorWhenGeneratingCode.clearCache(); } - /** - * Returns a new validator that performs the same validation as this one, but is strict about - * rejecting optionally-specified JSR 330 behavior that Dagger doesn't support (unless {@code - * -Adagger.ignorePrivateAndStaticInjectionForComponent=enabled} was set in the javac options). - */ - public InjectValidator whenGeneratingCode() { - return compilerOptions.ignorePrivateAndStaticInjectionForComponent() - ? this - : new InjectValidator( - processingEnv, - compilerOptions, - dependencyRequestValidator, - Optional.of(Diagnostic.Kind.ERROR), - injectionAnnotations, - superficialValidation, - methodSignatureFormatter); + public ValidationReport validate(XTypeElement typeElement) { + return validator.validate(typeElement); } - public ValidationReport validate(XTypeElement typeElement) { - return reentrantComputeIfAbsent(provisionReports, typeElement, this::validateUncached); + public ValidationReport validateForMembersInjection(XTypeElement typeElement) { + return validator.validateForMembersInjection(typeElement); } - private ValidationReport validateUncached(XTypeElement typeElement) { - ValidationReport.Builder builder = ValidationReport.about(typeElement); - builder.addSubreport(validateForMembersInjectionInternal(typeElement)); - - ImmutableSet injectConstructors = - ImmutableSet.builder() - .addAll(injectedConstructors(typeElement)) - .addAll(assistedInjectedConstructors(typeElement)) - .build(); - - switch (injectConstructors.size()) { - case 0: - break; // Nothing to validate. - case 1: - builder.addSubreport(validateConstructor(getOnlyElement(injectConstructors))); - break; - default: - builder.addError( - String.format( - "Type %s may only contain one injected constructor. Found: %s", - typeElement.getQualifiedName(), - injectConstructors.stream() - .map(methodSignatureFormatter::format) - .collect(toImmutableList())), - typeElement); + /** + * Validates {@code typeElement} that requires a factory to be generated. + * + *

In this case, the validator will have stricter validation for private and static injection + * since the generated factory doesn't support those types. + */ + public ValidationReport validateWhenGeneratingCode(XTypeElement typeElement) { + if (typeElement.getPackageName().startsWith("org.atinject.tck")) { + // The Technology Compatibility Kit (TCK) package is a special package for testing the JSR330 + // spec, which includes optional features like supporting static/private inject members. Even + // though Dagger doesn't support this, we allow it for this one case for the test coverage + // purposes. Use the normal validator which takes the user's compiler flags into account. + return validator.validate(typeElement); } - - return builder.build(); + return validatorWhenGeneratingCode.validate(typeElement); } - private ValidationReport validateConstructor(XConstructorElement constructorElement) { - superficialValidation.validateTypeOf(constructorElement); - ValidationReport.Builder builder = - ValidationReport.about(constructorElement.getEnclosingElement()); + private final class InternalValidator { + private final Diagnostic.Kind privateMemberDiagnosticKind; + private final Diagnostic.Kind staticMemberDiagnosticKind; + private final Map provisionReports = new HashMap<>(); + private final Map membersInjectionReports = new HashMap<>(); - if (InjectionAnnotations.hasInjectAnnotation(constructorElement) - && constructorElement.hasAnnotation(TypeNames.ASSISTED_INJECT)) { - builder.addError("Constructors cannot be annotated with both @Inject and @AssistedInject"); + InternalValidator( + Diagnostic.Kind privateMemberDiagnosticKind, Diagnostic.Kind staticMemberDiagnosticKind) { + this.privateMemberDiagnosticKind = privateMemberDiagnosticKind; + this.staticMemberDiagnosticKind = staticMemberDiagnosticKind; } - ClassName injectAnnotation = - getAnyAnnotation( - constructorElement, - TypeNames.INJECT, - TypeNames.INJECT_JAVAX, - TypeNames.ASSISTED_INJECT) - .map(XAnnotations::getClassName) - .get(); - - if (constructorElement.isPrivate()) { - builder.addError( - "Dagger does not support injection into private constructors", constructorElement); + void clearCache() { + provisionReports.clear(); + membersInjectionReports.clear(); } - // If this type has already been processed in a previous round or compilation unit then there - // is no reason to recheck for invalid scope annotations since it's already been checked. - // This allows us to skip superficial validation of constructor annotations in subsequent - // compilations where the annotation types may no longer be on the classpath. - if (!processedInPreviousRoundOrCompilationUnit(constructorElement)) { - superficialValidation.validateAnnotationsOf(constructorElement); - for (XAnnotation qualifier : injectionAnnotations.getQualifiers(constructorElement)) { - builder.addError( - String.format( - "@Qualifier annotations are not allowed on @%s constructors", - injectAnnotation.simpleName()), - constructorElement, - qualifier); + ValidationReport validate(XTypeElement typeElement) { + return reentrantComputeIfAbsent(provisionReports, typeElement, this::validateUncached); + } + + private ValidationReport validateUncached(XTypeElement typeElement) { + ValidationReport.Builder builder = ValidationReport.about(typeElement); + builder.addSubreport(validateForMembersInjectionInternal(typeElement)); + + ImmutableSet injectConstructors = + ImmutableSet.builder() + .addAll(injectedConstructors(typeElement)) + .addAll(assistedInjectedConstructors(typeElement)) + .build(); + + switch (injectConstructors.size()) { + case 0: + break; // Nothing to validate. + case 1: + builder.addSubreport(validateConstructor(getOnlyElement(injectConstructors))); + break; + default: + builder.addError( + String.format( + "Type %s may only contain one injected constructor. Found: %s", + typeElement.getQualifiedName(), + injectConstructors.stream() + .map(methodSignatureFormatter::format) + .collect(toImmutableList())), + typeElement); } - String scopeErrorMsg = - String.format( - "@Scope annotations are not allowed on @%s constructors", - injectAnnotation.simpleName()); + return builder.build(); + } + + private ValidationReport validateConstructor(XConstructorElement constructorElement) { + superficialValidation.validateTypeOf(constructorElement); + ValidationReport.Builder builder = + ValidationReport.about(constructorElement.getEnclosingElement()); - if (injectAnnotation.equals(TypeNames.INJECT) - || injectAnnotation.equals(TypeNames.INJECT_JAVAX)) { - scopeErrorMsg += "; annotate the class instead"; + if (InjectionAnnotations.hasInjectAnnotation(constructorElement) + && constructorElement.hasAnnotation(TypeNames.ASSISTED_INJECT)) { + builder.addError("Constructors cannot be annotated with both @Inject and @AssistedInject"); } - for (Scope scope : injectionAnnotations.getScopes(constructorElement)) { - builder.addError(scopeErrorMsg, constructorElement, scope.scopeAnnotation().xprocessing()); + ClassName injectAnnotation = + getAnyAnnotation( + constructorElement, + TypeNames.INJECT, + TypeNames.INJECT_JAVAX, + TypeNames.ASSISTED_INJECT) + .map(XAnnotations::getClassName) + .get(); + + if (constructorElement.isPrivate()) { + builder.addError( + "Dagger does not support injection into private constructors", constructorElement); } - } - for (XExecutableParameterElement parameter : constructorElement.getParameters()) { - superficialValidation.validateTypeOf(parameter); - validateDependencyRequest(builder, parameter); - } + // If this type has already been processed in a previous round or compilation unit then there + // is no reason to recheck for invalid scope annotations since it's already been checked. + // This allows us to skip superficial validation of constructor annotations in subsequent + // compilations where the annotation types may no longer be on the classpath. + if (!processedInPreviousRoundOrCompilationUnit(constructorElement)) { + superficialValidation.validateAnnotationsOf(constructorElement); + for (XAnnotation qualifier : injectionAnnotations.getQualifiers(constructorElement)) { + builder.addError( + String.format( + "@Qualifier annotations are not allowed on @%s constructors", + injectAnnotation.simpleName()), + constructorElement, + qualifier); + } - if (throwsCheckedExceptions(constructorElement)) { - builder.addItem( - String.format( - "Dagger does not support checked exceptions on @%s constructors", - injectAnnotation.simpleName()), - privateMemberDiagnosticKind(), - constructorElement); - } + String scopeErrorMsg = + String.format( + "@Scope annotations are not allowed on @%s constructors", + injectAnnotation.simpleName()); - checkInjectIntoPrivateClass(constructorElement, builder); + if (injectAnnotation.equals(TypeNames.INJECT) + || injectAnnotation.equals(TypeNames.INJECT_JAVAX)) { + scopeErrorMsg += "; annotate the class instead"; + } - XTypeElement enclosingElement = constructorElement.getEnclosingElement(); - if (enclosingElement.isAbstract()) { - builder.addError( - String.format( - "@%s is nonsense on the constructor of an abstract class", - injectAnnotation.simpleName()), - constructorElement); - } + for (Scope scope : injectionAnnotations.getScopes(constructorElement)) { + builder.addError( + scopeErrorMsg, constructorElement, scope.scopeAnnotation().xprocessing()); + } + } - if (enclosingElement.isNested() && !enclosingElement.isStatic()) { - builder.addError( - String.format( - "@%s constructors are invalid on inner classes. " - + "Did you mean to make the class static?", - injectAnnotation.simpleName()), - constructorElement); - } + for (XExecutableParameterElement parameter : constructorElement.getParameters()) { + superficialValidation.validateTypeOf(parameter); + validateDependencyRequest(builder, parameter); + } - // Note: superficial validation of the annotations is done as part of getting the scopes. - ImmutableSet scopes = - injectionAnnotations.getScopes(constructorElement.getEnclosingElement()); - if (injectAnnotation.equals(TypeNames.ASSISTED_INJECT)) { - for (Scope scope : scopes) { + if (throwsCheckedExceptions(constructorElement)) { + builder.addItem( + String.format( + "Dagger does not support checked exceptions on @%s constructors", + injectAnnotation.simpleName()), + privateMemberDiagnosticKind, + constructorElement); + } + + checkInjectIntoPrivateClass(constructorElement, builder); + + XTypeElement enclosingElement = constructorElement.getEnclosingElement(); + if (enclosingElement.isAbstract()) { builder.addError( - "A type with an @AssistedInject-annotated constructor cannot be scoped", - enclosingElement, - scope.scopeAnnotation().xprocessing()); + String.format( + "@%s is nonsense on the constructor of an abstract class", + injectAnnotation.simpleName()), + constructorElement); } - } else if (scopes.size() > 1) { - for (Scope scope : scopes) { + + if (enclosingElement.isNested() && !enclosingElement.isStatic()) { builder.addError( - "A single binding may not declare more than one @Scope", - enclosingElement, - scope.scopeAnnotation().xprocessing()); + String.format( + "@%s constructors are invalid on inner classes. " + + "Did you mean to make the class static?", + injectAnnotation.simpleName()), + constructorElement); } - } - return builder.build(); - } + // Note: superficial validation of the annotations is done as part of getting the scopes. + ImmutableSet scopes = + injectionAnnotations.getScopes(constructorElement.getEnclosingElement()); + if (injectAnnotation.equals(TypeNames.ASSISTED_INJECT)) { + for (Scope scope : scopes) { + builder.addError( + "A type with an @AssistedInject-annotated constructor cannot be scoped", + enclosingElement, + scope.scopeAnnotation().xprocessing()); + } + } else if (scopes.size() > 1) { + for (Scope scope : scopes) { + builder.addError( + "A single binding may not declare more than one @Scope", + enclosingElement, + scope.scopeAnnotation().xprocessing()); + } + } - private ValidationReport validateField(XFieldElement fieldElement) { - superficialValidation.validateTypeOf(fieldElement); - ValidationReport.Builder builder = ValidationReport.about(fieldElement); - if (fieldElement.isFinal()) { - builder.addError("@Inject fields may not be final", fieldElement); + return builder.build(); } - if (fieldElement.isPrivate()) { - builder.addItem( - "Dagger does not support injection into private fields", - privateMemberDiagnosticKind(), - fieldElement); - } + private ValidationReport validateField(XFieldElement fieldElement) { + superficialValidation.validateTypeOf(fieldElement); + ValidationReport.Builder builder = ValidationReport.about(fieldElement); + if (fieldElement.isFinal()) { + builder.addError("@Inject fields may not be final", fieldElement); + } - if (fieldElement.isStatic()) { - builder.addItem( - "Dagger does not support injection into static fields", - staticMemberDiagnosticKind(), - fieldElement); - } + if (fieldElement.isPrivate()) { + builder.addItem( + "Dagger does not support injection into private fields", + privateMemberDiagnosticKind, + fieldElement); + } - if (fieldElement.isProtected() - && fieldElement.getEnclosingElement().isFromKotlin() - ) { - builder.addError( - "Dagger injector does not have access to kotlin protected fields", fieldElement); - } + if (fieldElement.isStatic()) { + builder.addItem( + "Dagger does not support injection into static fields", + staticMemberDiagnosticKind, + fieldElement); + } - validateDependencyRequest(builder, fieldElement); + if (fieldElement.isProtected() + && fieldElement.getEnclosingElement().isFromKotlin() + ) { + builder.addError( + "Dagger injector does not have access to kotlin protected fields", fieldElement); + } - return builder.build(); - } + validateDependencyRequest(builder, fieldElement); - private ValidationReport validateMethod(XMethodElement methodElement) { - superficialValidation.validateTypeOf(methodElement); - ValidationReport.Builder builder = ValidationReport.about(methodElement); - if (methodElement.isAbstract()) { - builder.addError("Methods with @Inject may not be abstract", methodElement); + return builder.build(); } - if (methodElement.isPrivate()) { - builder.addItem( - "Dagger does not support injection into private methods", - privateMemberDiagnosticKind(), - methodElement); - } + private ValidationReport validateMethod(XMethodElement methodElement) { + superficialValidation.validateTypeOf(methodElement); + ValidationReport.Builder builder = ValidationReport.about(methodElement); + if (methodElement.isAbstract()) { + builder.addError("Methods with @Inject may not be abstract", methodElement); + } - if (methodElement.isStatic()) { - builder.addItem( - "Dagger does not support injection into static methods", - staticMemberDiagnosticKind(), - methodElement); - } + if (methodElement.isPrivate()) { + builder.addItem( + "Dagger does not support injection into private methods", + privateMemberDiagnosticKind, + methodElement); + } - // No need to resolve type parameters since we're only checking existence. - if (hasTypeParameters(methodElement)) { - builder.addError("Methods with @Inject may not declare type parameters", methodElement); - } + if (methodElement.isStatic()) { + builder.addItem( + "Dagger does not support injection into static methods", + staticMemberDiagnosticKind, + methodElement); + } - // No need to resolve thrown types since we're only checking existence. - if (!methodElement.getThrownTypes().isEmpty()) { - builder.addError( - "Methods with @Inject may not throw checked exceptions. " - + "Please wrap your exceptions in a RuntimeException instead.", - methodElement); - } + // No need to resolve type parameters since we're only checking existence. + if (hasTypeParameters(methodElement)) { + builder.addError("Methods with @Inject may not declare type parameters", methodElement); + } - for (XExecutableParameterElement parameter : methodElement.getParameters()) { - superficialValidation.validateTypeOf(parameter); - validateDependencyRequest(builder, parameter); - } + // No need to resolve thrown types since we're only checking existence. + if (!methodElement.getThrownTypes().isEmpty()) { + builder.addError( + "Methods with @Inject may not throw checked exceptions. " + + "Please wrap your exceptions in a RuntimeException instead.", + methodElement); + } - return builder.build(); - } + for (XExecutableParameterElement parameter : methodElement.getParameters()) { + superficialValidation.validateTypeOf(parameter); + validateDependencyRequest(builder, parameter); + } - private void validateDependencyRequest( - ValidationReport.Builder builder, XVariableElement parameter) { - dependencyRequestValidator.validateDependencyRequest(builder, parameter, parameter.getType()); - dependencyRequestValidator.checkNotProducer(builder, parameter); - } + return builder.build(); + } - public ValidationReport validateForMembersInjection(XTypeElement typeElement) { - return !processedInPreviousRoundOrCompilationUnit(typeElement) - ? validate(typeElement) // validate everything - : validateForMembersInjectionInternal(typeElement); // validate only inject members - } + private void validateDependencyRequest( + ValidationReport.Builder builder, XVariableElement parameter) { + dependencyRequestValidator.validateDependencyRequest(builder, parameter, parameter.getType()); + dependencyRequestValidator.checkNotProducer(builder, parameter); + } - private ValidationReport validateForMembersInjectionInternal(XTypeElement typeElement) { - return reentrantComputeIfAbsent( - membersInjectionReports, typeElement, this::validateForMembersInjectionInternalUncached); - } + public ValidationReport validateForMembersInjection(XTypeElement typeElement) { + return !processedInPreviousRoundOrCompilationUnit(typeElement) + ? validate(typeElement) // validate everything + : validateForMembersInjectionInternal(typeElement); // validate only inject members + } + + private ValidationReport validateForMembersInjectionInternal(XTypeElement typeElement) { + return reentrantComputeIfAbsent( + membersInjectionReports, typeElement, this::validateForMembersInjectionInternalUncached); + } - private ValidationReport validateForMembersInjectionInternalUncached(XTypeElement typeElement) { - superficialValidation.validateTypeOf(typeElement); - // TODO(beder): This element might not be currently compiled, so this error message could be - // left in limbo. Find an appropriate way to display the error message in that case. - ValidationReport.Builder builder = ValidationReport.about(typeElement); - boolean hasInjectedMembers = false; - for (XFieldElement field : typeElement.getDeclaredFields()) { - if (InjectionAnnotations.hasInjectAnnotation(field)) { - hasInjectedMembers = true; - ValidationReport report = validateField(field); - if (!report.isClean()) { - builder.addSubreport(report); + private ValidationReport validateForMembersInjectionInternalUncached(XTypeElement typeElement) { + superficialValidation.validateTypeOf(typeElement); + // TODO(beder): This element might not be currently compiled, so this error message could be + // left in limbo. Find an appropriate way to display the error message in that case. + ValidationReport.Builder builder = ValidationReport.about(typeElement); + boolean hasInjectedMembers = false; + for (XFieldElement field : typeElement.getDeclaredFields()) { + if (InjectionAnnotations.hasInjectAnnotation(field)) { + hasInjectedMembers = true; + ValidationReport report = validateField(field); + if (!report.isClean()) { + builder.addSubreport(report); + } } } - } - for (XMethodElement method : typeElement.getDeclaredMethods()) { - if (InjectionAnnotations.hasInjectAnnotation(method)) { - hasInjectedMembers = true; - ValidationReport report = validateMethod(method); - if (!report.isClean()) { - builder.addSubreport(report); + for (XMethodElement method : typeElement.getDeclaredMethods()) { + if (InjectionAnnotations.hasInjectAnnotation(method)) { + hasInjectedMembers = true; + ValidationReport report = validateMethod(method); + if (!report.isClean()) { + builder.addSubreport(report); + } } } - } - - if (hasInjectedMembers) { - checkInjectIntoPrivateClass(typeElement, builder); - checkInjectIntoKotlinObject(typeElement, builder); - } - - Optional.ofNullable(typeElement.getSuperType()) - .filter(supertype -> !supertype.getTypeName().equals(TypeName.OBJECT)) - .ifPresent( - supertype -> { - superficialValidation.validateSuperTypeOf(typeElement); - ValidationReport report = validateForMembersInjection(supertype.getTypeElement()); - if (!report.isClean()) { - builder.addSubreport(report); - } - }); - - return builder.build(); - } - /** Returns true if the given method element declares a checked exception. */ - private boolean throwsCheckedExceptions(XConstructorElement constructorElement) { - XType runtimeException = processingEnv.findType(TypeNames.RUNTIME_EXCEPTION); - XType error = processingEnv.findType(TypeNames.ERROR); - superficialValidation.validateThrownTypesOf(constructorElement); - return !constructorElement.getThrownTypes().stream() - .allMatch(type -> isSubtype(type, runtimeException) || isSubtype(type, error)); - } + if (hasInjectedMembers) { + checkInjectIntoPrivateClass(typeElement, builder); + checkInjectIntoKotlinObject(typeElement, builder); + } - private void checkInjectIntoPrivateClass(XElement element, ValidationReport.Builder builder) { - if (!Accessibility.isElementAccessibleFromOwnPackage(closestEnclosingTypeElement(element))) { - builder.addItem( - "Dagger does not support injection into private classes", - privateMemberDiagnosticKind(), - element); + Optional.ofNullable(typeElement.getSuperType()) + .filter(supertype -> !supertype.getTypeName().equals(TypeName.OBJECT)) + .ifPresent( + supertype -> { + superficialValidation.validateSuperTypeOf(typeElement); + ValidationReport report = validateForMembersInjection(supertype.getTypeElement()); + if (!report.isClean()) { + builder.addSubreport(report); + } + }); + + return builder.build(); } - } - private void checkInjectIntoKotlinObject(XTypeElement element, ValidationReport.Builder builder) { - if (element.isKotlinObject() || element.isCompanionObject()) { - builder.addError("Dagger does not support injection into Kotlin objects", element); + /** Returns true if the given method element declares a checked exception. */ + private boolean throwsCheckedExceptions(XConstructorElement constructorElement) { + XType runtimeException = processingEnv.findType(TypeNames.RUNTIME_EXCEPTION); + XType error = processingEnv.findType(TypeNames.ERROR); + superficialValidation.validateThrownTypesOf(constructorElement); + return !constructorElement.getThrownTypes().stream() + .allMatch(type -> isSubtype(type, runtimeException) || isSubtype(type, error)); } - } - private Diagnostic.Kind privateMemberDiagnosticKind() { - return privateAndStaticInjectionDiagnosticKind.orElse( - compilerOptions.privateMemberValidationKind()); - } + private void checkInjectIntoPrivateClass(XElement element, ValidationReport.Builder builder) { + if (!Accessibility.isElementAccessibleFromOwnPackage(closestEnclosingTypeElement(element))) { + builder.addItem( + "Dagger does not support injection into private classes", + privateMemberDiagnosticKind, + element); + } + } - private Diagnostic.Kind staticMemberDiagnosticKind() { - return privateAndStaticInjectionDiagnosticKind.orElse( - compilerOptions.staticMemberValidationKind()); - } + private void checkInjectIntoKotlinObject( + XTypeElement element, ValidationReport.Builder builder) { + if (element.isKotlinObject() || element.isCompanionObject()) { + builder.addError("Dagger does not support injection into Kotlin objects", element); + } + } - private boolean processedInPreviousRoundOrCompilationUnit(XConstructorElement injectConstructor) { - return processingEnv.findTypeElement(factoryNameForElement(injectConstructor)) != null; - } + private boolean processedInPreviousRoundOrCompilationUnit( + XConstructorElement injectConstructor) { + return processingEnv.findTypeElement(factoryNameForElement(injectConstructor)) != null; + } - private boolean processedInPreviousRoundOrCompilationUnit(XTypeElement membersInjectedType) { - return processingEnv.findTypeElement(membersInjectorNameForType(membersInjectedType)) != null; + private boolean processedInPreviousRoundOrCompilationUnit(XTypeElement membersInjectedType) { + return processingEnv.findTypeElement(membersInjectorNameForType(membersInjectedType)) != null; + } } }