From 740a7c6e10eb88bc676e8064eae7f490a0e33310 Mon Sep 17 00:00:00 2001 From: Kuan-Ying Chou Date: Thu, 12 Oct 2023 07:36:39 -0700 Subject: [PATCH] Add check to prevent injecting assisted factories for HiltViewModels RELNOTES=Add check to prevent injecting assisted factories for HiltViewModels PiperOrigin-RevId: 572901826 --- .../processor/internal/viewmodel/BUILD | 1 + .../viewmodel/ViewModelValidationPlugin.kt | 91 +++++++- .../processor/internal/viewmodel/BUILD | 32 +++ ...lValidationPluginWithAssistedInjectTest.kt | 217 ++++++++++++++++++ 4 files changed, 338 insertions(+), 3 deletions(-) create mode 100644 javatests/dagger/hilt/android/processor/internal/viewmodel/ViewModelValidationPluginWithAssistedInjectTest.kt diff --git a/java/dagger/hilt/android/processor/internal/viewmodel/BUILD b/java/dagger/hilt/android/processor/internal/viewmodel/BUILD index e5c3eaf50b8..90760ea3140 100644 --- a/java/dagger/hilt/android/processor/internal/viewmodel/BUILD +++ b/java/dagger/hilt/android/processor/internal/viewmodel/BUILD @@ -64,6 +64,7 @@ kt_jvm_library( "//:spi", "//java/dagger/hilt/android/processor/internal:android_classnames", "//java/dagger/hilt/processor/internal:dagger_models", + "//java/dagger/internal/codegen/xprocessing", "//third_party/java/auto:service", "//third_party/java/guava/graph", "//third_party/java/javapoet", diff --git a/java/dagger/hilt/android/processor/internal/viewmodel/ViewModelValidationPlugin.kt b/java/dagger/hilt/android/processor/internal/viewmodel/ViewModelValidationPlugin.kt index 0c472d708cf..69094c9a5f4 100644 --- a/java/dagger/hilt/android/processor/internal/viewmodel/ViewModelValidationPlugin.kt +++ b/java/dagger/hilt/android/processor/internal/viewmodel/ViewModelValidationPlugin.kt @@ -14,26 +14,45 @@ * limitations under the License. */ +@file:OptIn(ExperimentalProcessingApi::class) + package dagger.hilt.android.processor.internal.viewmodel +import androidx.room.compiler.processing.ExperimentalProcessingApi +import androidx.room.compiler.processing.XMethodElement +import androidx.room.compiler.processing.XProcessingEnv +import androidx.room.compiler.processing.XProcessingEnv.Companion.create +import androidx.room.compiler.processing.XType +import androidx.room.compiler.processing.XTypeElement +import androidx.room.compiler.processing.compat.XConverters.toXProcessing import com.google.auto.service.AutoService import com.google.common.graph.EndpointPair import com.google.common.graph.ImmutableNetwork import dagger.hilt.android.processor.internal.AndroidClassNames import dagger.hilt.processor.internal.getQualifiedName import dagger.hilt.processor.internal.hasAnnotation +import dagger.internal.codegen.xprocessing.XTypeElements import dagger.spi.model.Binding import dagger.spi.model.BindingGraph import dagger.spi.model.BindingGraph.Edge import dagger.spi.model.BindingGraph.Node import dagger.spi.model.BindingGraphPlugin import dagger.spi.model.BindingKind +import dagger.spi.model.DaggerProcessingEnv +import dagger.spi.model.DaggerType import dagger.spi.model.DiagnosticReporter import javax.tools.Diagnostic.Kind /** Plugin to validate users do not inject @HiltViewModel classes. */ @AutoService(BindingGraphPlugin::class) class ViewModelValidationPlugin : BindingGraphPlugin { + + private lateinit var env: XProcessingEnv + + override fun init(processingEnv: DaggerProcessingEnv, options: MutableMap) { + env = processingEnv.toXProcessingEnv() + } + override fun visitGraph(bindingGraph: BindingGraph, diagnosticReporter: DiagnosticReporter) { if (bindingGraph.rootComponentNode().isSubcomponent()) { // This check does not work with partial graphs since it needs to take into account the source @@ -46,9 +65,10 @@ class ViewModelValidationPlugin : BindingGraphPlugin { val pair: EndpointPair = network.incidentNodes(edge) val target: Node = pair.target() val source: Node = pair.source() - if ( - target is Binding && isHiltViewModelBinding(target) && !isInternalHiltViewModelUsage(source) - ) { + if (target !is Binding) { + return@forEach + } + if (isHiltViewModelBinding(target) && !isInternalHiltViewModelUsage(source)) { diagnosticReporter.reportDependency( Kind.ERROR, edge, @@ -57,6 +77,17 @@ class ViewModelValidationPlugin : BindingGraphPlugin { "(e.g. ViewModelProvider) instead." + "\nInjected ViewModel: ${target.key().type()}\n" ) + } else if ( + isViewModelAssistedFactory(target) && !isInternalViewModelAssistedFactoryUsage(source) + ) { + diagnosticReporter.reportDependency( + Kind.ERROR, + edge, + "\nInjection of an assisted factory for Hilt ViewModel is prohibited since it " + + "can not be used to create a ViewModel instance correctly.\nAccess the ViewModel via " + + "the Android APIs (e.g. ViewModelProvider) instead." + + "\nInjected factory: ${target.key().type()}\n" + ) } } } @@ -84,4 +115,58 @@ class ViewModelValidationPlugin : BindingGraphPlugin { AndroidClassNames.HILT_VIEW_MODEL_MAP_QUALIFIER.canonicalName() && source.key().multibindingContributionIdentifier().isPresent() } + + private fun isViewModelAssistedFactory(target: Binding): Boolean { + if (target.kind() != BindingKind.ASSISTED_FACTORY) return false + val factoryType = target.key().type() + return getAssistedInjectTypeElement(factoryType.toXType(env)) + .hasAnnotation(AndroidClassNames.HILT_VIEW_MODEL) + } + + private fun getAssistedInjectTypeElement(factoryType: XType): XTypeElement = + // The factory method and the type element for its return type cannot be + // null as the BindingGraph won't be created if the + // @AssistedFactory-annotated class is invalid. + getAssistedFactoryMethods(factoryType.typeElement) + .single() + .asMemberOf(factoryType) + .returnType + .typeElement!! + + private fun getAssistedFactoryMethods(factory: XTypeElement?): List { + return XTypeElements.getAllNonPrivateInstanceMethods(factory) + .filter { it.isAbstract() } + .filter { !it.isJavaDefault() } + } + + private fun isInternalViewModelAssistedFactoryUsage(source: Node): Boolean { + // We expect the only usage of the assisted factory for a Hilt ViewModel is in the + // code we generate: + // @Binds + // @IntoMap + // @StringKey(...) + // @HiltViewModelAssistedMap + // public abstract Object bind(FooFactory factory); + return source is Binding && + source.key().qualifier().isPresent() && + source.key().qualifier().get().getQualifiedName() == + AndroidClassNames.HILT_VIEW_MODEL_ASSISTED_FACTORY_MAP_QUALIFIER.canonicalName() && + source.key().multibindingContributionIdentifier().isPresent() + } +} + +private fun DaggerType.toXType(processingEnv: XProcessingEnv): XType { + return when (backend()) { + DaggerProcessingEnv.Backend.JAVAC -> javac().toXProcessing(processingEnv) + DaggerProcessingEnv.Backend.KSP -> ksp().toXProcessing(processingEnv) + else -> error("Backend ${ backend() } not supported yet.") + } +} + +private fun DaggerProcessingEnv.toXProcessingEnv(): XProcessingEnv { + return when (backend()) { + DaggerProcessingEnv.Backend.JAVAC -> create(javac()) + DaggerProcessingEnv.Backend.KSP -> create(ksp(), resolver()) + else -> error("Backend ${ backend() } not supported yet.") + } } diff --git a/javatests/dagger/hilt/android/processor/internal/viewmodel/BUILD b/javatests/dagger/hilt/android/processor/internal/viewmodel/BUILD index 53bc73c8c8e..eb73b3f0991 100644 --- a/javatests/dagger/hilt/android/processor/internal/viewmodel/BUILD +++ b/javatests/dagger/hilt/android/processor/internal/viewmodel/BUILD @@ -70,6 +70,38 @@ kt_compiler_test( ], ) +kt_compiler_test( + name = "ViewModelValidationPluginWithAssistedInjectTest", + srcs = [ + "ViewModelValidationPluginWithAssistedInjectTest.kt", + ], + compiler_deps = [ + "@androidsdk//:platforms/android-32/android.jar", + "@maven//:androidx_lifecycle_lifecycle_viewmodel", + "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", + "//third_party/java/compile_testing", + "//third_party/java/truth", + "//java/dagger/hilt/android/lifecycle:hilt_view_model", + "//java/dagger/hilt/android:android_entry_point", + "//java/dagger/hilt/android:hilt_android_app", + ], + resources = glob(["goldens/*"]), + deps = [ + ":test_utils", + "//:compiler_internals", + "//java/dagger/hilt/android/processor/internal/viewmodel:processor_lib", + "//java/dagger/hilt/android/processor/internal/viewmodel:validation_plugin_lib", + "//java/dagger/hilt/android/testing/compile", + "//java/dagger/internal/codegen/xprocessing", + "//java/dagger/internal/codegen/xprocessing:xprocessing-testing", + "//java/dagger/testing/golden", + "//third_party/java/compile_testing", + "//third_party/java/guava/collect", + "//third_party/java/junit", + "//third_party/java/truth", + ], +) + kt_jvm_library( name = "test_utils", srcs = [ diff --git a/javatests/dagger/hilt/android/processor/internal/viewmodel/ViewModelValidationPluginWithAssistedInjectTest.kt b/javatests/dagger/hilt/android/processor/internal/viewmodel/ViewModelValidationPluginWithAssistedInjectTest.kt new file mode 100644 index 00000000000..f8f04fe77bf --- /dev/null +++ b/javatests/dagger/hilt/android/processor/internal/viewmodel/ViewModelValidationPluginWithAssistedInjectTest.kt @@ -0,0 +1,217 @@ +/* + * Copyright (C) 2023 The Dagger Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package dagger.hilt.android.processor.internal.viewmodel + +import androidx.room.compiler.processing.ExperimentalProcessingApi +import androidx.room.compiler.processing.util.Source +import com.google.common.collect.ImmutableList +import com.google.common.collect.ImmutableMap +import dagger.hilt.android.testing.compile.HiltCompilerTests +import dagger.internal.codegen.ComponentProcessor +import dagger.internal.codegen.KspComponentProcessor +import dagger.testing.compile.CompilerTests.* +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@ExperimentalProcessingApi +@RunWith(JUnit4::class) +class ViewModelValidationPluginWithAssistedInjectTest { + + private fun testCompiler(vararg sources: Source): HiltCompilerTests.HiltCompiler = + HiltCompilerTests.hiltCompiler(ImmutableList.copyOf(sources)) + .withAdditionalJavacProcessors( + ComponentProcessor.withTestPlugins(ViewModelValidationPlugin()), + ViewModelProcessor() + ) + .withAdditionalKspProcessors( + KspComponentProcessor.Provider.withTestPlugins(ViewModelValidationPlugin()), + KspViewModelProcessor.Provider() + ) + .withProcessorOptions(ImmutableMap.of("dagger.hilt.enableAssistedInjectViewModels", "true")) + + private val hiltAndroidApp = + """ + package dagger.hilt.android.test; + + import android.app.Application; + import dagger.hilt.android.HiltAndroidApp; + + @HiltAndroidApp(Application.class) + public class TestApplication extends Hilt_TestApplication {} + """ + .toJFO("dagger.hilt.android.test.TestApplication") + + @Test + fun injectViewModelAssistedFactoryProhibited() { + val hiltActivity = + """ + package dagger.hilt.android.test; + + import androidx.fragment.app.FragmentActivity; + import dagger.hilt.android.AndroidEntryPoint; + import javax.inject.Inject; + + @AndroidEntryPoint(FragmentActivity.class) + public class TestActivity extends Hilt_TestActivity { + @Inject Foo foo; + } + """ + .toJFO("dagger.hilt.android.test.TestActivity") + val hiltViewModel = + """ + package dagger.hilt.android.test; + + import androidx.lifecycle.ViewModel; + import dagger.assisted.Assisted; + import dagger.assisted.AssistedFactory; + import dagger.assisted.AssistedInject; + import dagger.hilt.android.lifecycle.HiltViewModel; + import javax.inject.Inject; + + @HiltViewModel(assistedFactory = MyViewModel.Factory.class) + class MyViewModel extends ViewModel { + @AssistedInject MyViewModel(@Assisted String s) { } + @AssistedFactory interface Factory { + MyViewModel create(String s); + } + } + """ + .toJFO("dagger.hilt.android.test.MyViewModel") + val foo = + """ + package dagger.hilt.android.test; + + import javax.inject.Inject; + + final class Foo { + @Inject Foo(MyViewModel.Factory factory) {} + } + """ + .toJFO("dagger.hilt.android.test.Foo") + + testCompiler(foo, hiltViewModel, hiltAndroidApp, hiltActivity).compile { subject -> + subject.compilationDidFail() + subject.hasErrorCount(1) + subject.hasErrorContaining( + "Injection of an assisted factory for Hilt ViewModel is prohibited since it can not be " + + "used to create a ViewModel instance correctly.\n" + + "Access the ViewModel via the Android APIs (e.g. ViewModelProvider) instead.\n" + + "Injected factory: dagger.hilt.android.test.MyViewModel.Factory" + ) + } + } + + @Test + fun fieldInjectViewModelAssistedFactoryProhibited() { + val hiltActivity = + """ + package dagger.hilt.android.test; + + import androidx.fragment.app.FragmentActivity; + import dagger.hilt.android.AndroidEntryPoint; + import javax.inject.Inject; + + @AndroidEntryPoint(FragmentActivity.class) + public class TestActivity extends Hilt_TestActivity { + @Inject MyViewModel.Factory factory; + } + """ + .toJFO("dagger.hilt.android.test.TestActivity") + val hiltViewModel = + """ + package dagger.hilt.android.test; + + import androidx.lifecycle.ViewModel; + import dagger.assisted.Assisted; + import dagger.assisted.AssistedFactory; + import dagger.assisted.AssistedInject; + import dagger.hilt.android.lifecycle.HiltViewModel; + import javax.inject.Inject; + + @HiltViewModel(assistedFactory = MyViewModel.Factory.class) + class MyViewModel extends ViewModel { + @AssistedInject MyViewModel(@Assisted String s) { } + @AssistedFactory interface Factory { + MyViewModel create(String s); + } + } + """ + .toJFO("dagger.hilt.android.test.MyViewModel") + + testCompiler(hiltViewModel, hiltAndroidApp, hiltActivity).compile { subject -> + subject.compilationDidFail() + subject.hasErrorCount(1) + subject.hasErrorContaining( + "Injection of an assisted factory for Hilt ViewModel is prohibited since it can not be " + + "used to create a ViewModel instance correctly.\n" + + "Access the ViewModel via the Android APIs (e.g. ViewModelProvider) instead.\n" + + "Injected factory: dagger.hilt.android.test.MyViewModel.Factory" + ) + } + } + + @Test + fun injectGenericViewModelAssistedFactoryProhibited() { + val hiltActivity = + """ + package dagger.hilt.android.test; + + import androidx.fragment.app.FragmentActivity; + import dagger.hilt.android.AndroidEntryPoint; + import javax.inject.Inject; + + @AndroidEntryPoint(FragmentActivity.class) + public class TestActivity extends Hilt_TestActivity { + @Inject MyViewModel.Factory factory; + } + """ + .toJFO("dagger.hilt.android.test.TestActivity") + val hiltViewModel = + """ + package dagger.hilt.android.test; + + import androidx.lifecycle.ViewModel; + import dagger.assisted.Assisted; + import dagger.assisted.AssistedFactory; + import dagger.assisted.AssistedInject; + import dagger.hilt.android.lifecycle.HiltViewModel; + import javax.inject.Inject; + + @HiltViewModel(assistedFactory = MyViewModel.Factory.class) + class MyViewModel extends ViewModel { + @AssistedInject MyViewModel(@Assisted String s) { } + interface SingleAssistedFactory { + T create(A a); + } + @AssistedFactory interface Factory extends SingleAssistedFactory {} + } + """ + .toJFO("dagger.hilt.android.test.MyViewModel") + + testCompiler(hiltViewModel, hiltAndroidApp, hiltActivity).compile { subject -> + subject.compilationDidFail() + subject.hasErrorCount(1) + subject.hasErrorContaining( + "Injection of an assisted factory for Hilt ViewModel is prohibited since it can not be " + + "used to create a ViewModel instance correctly.\n" + + "Access the ViewModel via the Android APIs (e.g. ViewModelProvider) instead.\n" + + "Injected factory: dagger.hilt.android.test.MyViewModel.Factory" + ) + } + } +}