Skip to content

Commit

Permalink
Allow kotlin source to use java keywords as parameter names in KSP.
Browse files Browse the repository at this point in the history
Using keywords as parameter names has been working with KAPT because KAPT just renames the parameters to something that is valid Java in the generated Java stubs. However, in order to support this in KSP we need to switch over to use `XExecutableParameterElement#jvmName` rather than `XExecutableParameterElement#name`.

This CL also requires updates from XProcessing, so I've updated Dagger's XProcessing jars as well.

Fixes #3995: Allow kotlin source to use java keywords as parameter names in KSP

RELNOTES=Fixed #3995: Allow kotlin source to use java keywords as parameter names in KSP
PiperOrigin-RevId: 557565286
  • Loading branch information
bcorso authored and Dagger Team committed Aug 16, 2023
1 parent cca174b commit 2c31d66
Show file tree
Hide file tree
Showing 13 changed files with 196 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet;
import static dagger.internal.codegen.xprocessing.XElements.asConstructor;
import static dagger.internal.codegen.xprocessing.XElements.asTypeElement;
import static dagger.internal.codegen.xprocessing.XElements.getSimpleName;

import androidx.room.compiler.processing.XConstructorElement;
import androidx.room.compiler.processing.XConstructorType;
import androidx.room.compiler.processing.XElement;
import androidx.room.compiler.processing.XExecutableParameterElement;
import androidx.room.compiler.processing.XHasModifiers;
import androidx.room.compiler.processing.XMethodElement;
import androidx.room.compiler.processing.XMethodType;
Expand Down Expand Up @@ -90,14 +90,15 @@ public static ImmutableList<ParameterSpec> assistedParameterSpecs(Binding bindin
}

private static ImmutableList<ParameterSpec> assistedParameterSpecs(
List<? extends XVariableElement> paramElements, List<XType> paramTypes) {
List<? extends XExecutableParameterElement> paramElements, List<XType> paramTypes) {
ImmutableList.Builder<ParameterSpec> assistedParameterSpecs = ImmutableList.builder();
for (int i = 0; i < paramElements.size(); i++) {
XVariableElement paramElement = paramElements.get(i);
XExecutableParameterElement paramElement = paramElements.get(i);
XType paramType = paramTypes.get(i);
if (isAssistedParameter(paramElement)) {
assistedParameterSpecs.add(
ParameterSpec.builder(paramType.getTypeName(), getSimpleName(paramElement)).build());
ParameterSpec.builder(paramType.getTypeName(), paramElement.getJvmName())
.build());
}
}
return assistedParameterSpecs.build();
Expand Down Expand Up @@ -133,7 +134,7 @@ public static ImmutableSet<XConstructorElement> assistedInjectedConstructors(XTy
.collect(toImmutableSet());
}

public static ImmutableList<XVariableElement> assistedParameters(Binding binding) {
public static ImmutableList<XExecutableParameterElement> assistedParameters(Binding binding) {
return binding.kind() == BindingKind.ASSISTED_INJECTION
? asConstructor(binding.bindingElement().get()).getParameters().stream()
.filter(AssistedInjectionAnnotations::isAssistedParameter)
Expand Down Expand Up @@ -184,18 +185,21 @@ public static AssistedFactoryMetadata create(XType factoryType) {
public abstract ImmutableList<AssistedParameter> assistedFactoryAssistedParameters();

@Memoized
public ImmutableMap<AssistedParameter, XVariableElement> assistedInjectAssistedParametersMap() {
ImmutableMap.Builder<AssistedParameter, XVariableElement> builder = ImmutableMap.builder();
public ImmutableMap<AssistedParameter, XExecutableParameterElement>
assistedInjectAssistedParametersMap() {
ImmutableMap.Builder<AssistedParameter, XExecutableParameterElement> builder =
ImmutableMap.builder();
for (AssistedParameter assistedParameter : assistedInjectAssistedParameters()) {
builder.put(assistedParameter, assistedParameter.element());
}
return builder.build();
}

@Memoized
public ImmutableMap<AssistedParameter, XVariableElement>
public ImmutableMap<AssistedParameter, XExecutableParameterElement>
assistedFactoryAssistedParametersMap() {
ImmutableMap.Builder<AssistedParameter, XVariableElement> builder = ImmutableMap.builder();
ImmutableMap.Builder<AssistedParameter, XExecutableParameterElement> builder =
ImmutableMap.builder();
for (AssistedParameter assistedParameter : assistedFactoryAssistedParameters()) {
builder.put(assistedParameter, assistedParameter.element());
}
Expand All @@ -211,7 +215,8 @@ public ImmutableMap<AssistedParameter, XVariableElement> assistedInjectAssistedP
*/
@AutoValue
public abstract static class AssistedParameter {
public static AssistedParameter create(XVariableElement parameter, XType parameterType) {
public static AssistedParameter create(
XExecutableParameterElement parameter, XType parameterType) {
AssistedParameter assistedParameter =
new AutoValue_AssistedInjectionAnnotations_AssistedParameter(
Optional.ofNullable(parameter.getAnnotation(TypeNames.ASSISTED))
Expand All @@ -223,7 +228,7 @@ public static AssistedParameter create(XVariableElement parameter, XType paramet
return assistedParameter;
}

private XVariableElement parameterElement;
private XExecutableParameterElement parameterElement;
private XType parameterType;

/** Returns the string qualifier from the {@link Assisted#value()}. */
Expand All @@ -237,7 +242,7 @@ public final XType type() {
return parameterType;
}

public final XVariableElement element() {
public final XExecutableParameterElement element() {
return parameterElement;
}

Expand All @@ -260,7 +265,7 @@ public static ImmutableList<AssistedParameter> assistedInjectAssistedParameters(

ImmutableList.Builder<AssistedParameter> builder = ImmutableList.builder();
for (int i = 0; i < assistedInjectConstructor.getParameters().size(); i++) {
XVariableElement parameter = assistedInjectConstructor.getParameters().get(i);
XExecutableParameterElement parameter = assistedInjectConstructor.getParameters().get(i);
XType parameterType = assistedInjectConstructorType.getParameterTypes().get(i);
if (parameter.hasAnnotation(TypeNames.ASSISTED)) {
builder.add(AssistedParameter.create(parameter, parameterType));
Expand All @@ -273,7 +278,7 @@ private static ImmutableList<AssistedParameter> assistedFactoryAssistedParameter
XMethodElement factoryMethod, XMethodType factoryMethodType) {
ImmutableList.Builder<AssistedParameter> builder = ImmutableList.builder();
for (int i = 0; i < factoryMethod.getParameters().size(); i++) {
XVariableElement parameter = factoryMethod.getParameters().get(i);
XExecutableParameterElement parameter = factoryMethod.getParameters().get(i);
XType parameterType = factoryMethodType.getParameterTypes().get(i);
builder.add(AssistedParameter.create(parameter, parameterType));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ public ImmutableList<TypeSpec.Builder> topLevelTypes(ProvisionBinding binding) {
// use the parameter names of the @AssistedFactory method.
metadata.assistedInjectAssistedParameters().stream()
.map(metadata.assistedFactoryAssistedParametersMap()::get)
.map(param -> CodeBlock.of("$L", getSimpleName(param)))
.map(param -> CodeBlock.of("$L", param.getJvmName()))
.collect(toParametersCodeBlock()))
.build())
.addMethod(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@

import androidx.room.compiler.processing.XConstructorElement;
import androidx.room.compiler.processing.XConstructorType;
import androidx.room.compiler.processing.XExecutableParameterElement;
import androidx.room.compiler.processing.XMethodType;
import androidx.room.compiler.processing.XType;
import androidx.room.compiler.processing.XTypeElement;
import androidx.room.compiler.processing.XVariableElement;
import com.google.common.collect.ImmutableList;
import com.squareup.javapoet.ParameterSpec;
import dagger.internal.codegen.binding.AssistedInjectionAnnotations;
Expand Down Expand Up @@ -79,12 +79,12 @@ public static ImmutableList<ParameterSpec> assistedParameterSpecs(
}

private static ImmutableList<ParameterSpec> assistedParameterSpecs(
List<? extends XVariableElement> paramElements,
List<XExecutableParameterElement> paramElements,
List<XType> paramTypes,
ShardImplementation shardImplementation) {
ImmutableList.Builder<ParameterSpec> assistedParameterSpecs = ImmutableList.builder();
for (int i = 0; i < paramElements.size(); i++) {
XVariableElement paramElement = paramElements.get(i);
XExecutableParameterElement paramElement = paramElements.get(i);
XType paramType = paramTypes.get(i);
if (AssistedInjectionAnnotations.isAssistedParameter(paramElement)) {
assistedParameterSpecs.add(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import static javax.lang.model.element.Modifier.STATIC;
import static javax.tools.Diagnostic.Kind.ERROR;

import androidx.room.compiler.processing.XExecutableParameterElement;
import androidx.room.compiler.processing.XMessager;
import androidx.room.compiler.processing.XMethodElement;
import androidx.room.compiler.processing.XProcessingEnv;
Expand Down Expand Up @@ -648,12 +649,12 @@ String getUniqueAssistedParamName(String name) {
return assistedParamNames.getUniqueName(name);
}

public String getUniqueFieldNameForAssistedParam(XVariableElement element) {
if (uniqueAssistedName.containsKey(element)) {
return uniqueAssistedName.get(element);
public String getUniqueFieldNameForAssistedParam(XExecutableParameterElement parameter) {
if (uniqueAssistedName.containsKey(parameter)) {
return uniqueAssistedName.get(parameter);
}
String name = getUniqueAssistedParamName(getSimpleName(element));
uniqueAssistedName.put(element, name);
String name = getUniqueAssistedParamName(parameter.getJvmName());
uniqueAssistedName.put(parameter, name);
return name;
}

Expand Down
7 changes: 3 additions & 4 deletions java/dagger/internal/codegen/writing/FactoryGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,15 @@
import static dagger.internal.codegen.model.BindingKind.INJECTION;
import static dagger.internal.codegen.model.BindingKind.PROVISION;
import static dagger.internal.codegen.writing.GwtCompatibility.gwtIncompatibleAnnotation;
import static dagger.internal.codegen.xprocessing.XElements.getSimpleName;
import static javax.lang.model.element.Modifier.FINAL;
import static javax.lang.model.element.Modifier.PRIVATE;
import static javax.lang.model.element.Modifier.PUBLIC;
import static javax.lang.model.element.Modifier.STATIC;

import androidx.room.compiler.processing.XElement;
import androidx.room.compiler.processing.XExecutableParameterElement;
import androidx.room.compiler.processing.XFiler;
import androidx.room.compiler.processing.XProcessingEnv;
import androidx.room.compiler.processing.XVariableElement;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
Expand Down Expand Up @@ -236,15 +235,15 @@ private MethodSpec getMethod(ProvisionBinding binding) {
UniqueNameSet uniqueFieldNames = new UniqueNameSet();
ImmutableMap<DependencyRequest, FieldSpec> frameworkFields = frameworkFields(binding);
frameworkFields.values().forEach(field -> uniqueFieldNames.claim(field.name));
ImmutableMap<XVariableElement, ParameterSpec> assistedParameters =
ImmutableMap<XExecutableParameterElement, ParameterSpec> assistedParameters =
assistedParameters(binding).stream()
.collect(
toImmutableMap(
parameter -> parameter,
parameter ->
ParameterSpec.builder(
parameter.getType().getTypeName(),
uniqueFieldNames.getUniqueName(getSimpleName(parameter)))
uniqueFieldNames.getUniqueName(parameter.getJvmName()))
.build()));
TypeName providedTypeName = providedTypeName(binding);
MethodSpec.Builder getMethod =
Expand Down
28 changes: 8 additions & 20 deletions java/dagger/internal/codegen/writing/InjectionMethods.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static androidx.room.compiler.processing.XElementKt.isConstructor;
import static androidx.room.compiler.processing.XElementKt.isMethod;
import static androidx.room.compiler.processing.XElementKt.isMethodParameter;
import static androidx.room.compiler.processing.XTypeKt.isVoid;
import static com.google.common.base.CaseFormat.LOWER_CAMEL;
import static com.google.common.base.CaseFormat.UPPER_CAMEL;
Expand All @@ -27,7 +28,6 @@
import static dagger.internal.codegen.binding.SourceFiles.generatedClassNameForBinding;
import static dagger.internal.codegen.binding.SourceFiles.memberInjectedFieldSignatureForVariable;
import static dagger.internal.codegen.binding.SourceFiles.membersInjectorNameForType;
import static dagger.internal.codegen.binding.SourceFiles.protectAgainstKeywords;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableList;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableMap;
import static dagger.internal.codegen.javapoet.CodeBlocks.makeParametersCodeBlock;
Expand Down Expand Up @@ -81,7 +81,6 @@
import java.util.List;
import java.util.Optional;
import java.util.function.Function;
import javax.lang.model.SourceVersion;

/** Convenience methods for creating and invoking {@link InjectionMethod}s. */
final class InjectionMethods {
Expand Down Expand Up @@ -145,7 +144,7 @@ static MethodSpec create(ProvisionBinding binding, CompilerOptions compilerOptio
static CodeBlock invoke(
ProvisionBinding binding,
Function<DependencyRequest, CodeBlock> dependencyUsage,
Function<XVariableElement, String> uniqueAssistedParameterName,
Function<XExecutableParameterElement, String> uniqueAssistedParameterName,
ClassName requestingClass,
Optional<CodeBlock> moduleReference,
CompilerOptions compilerOptions) {
Expand All @@ -162,7 +161,7 @@ static CodeBlock invoke(
static ImmutableList<CodeBlock> invokeArguments(
ProvisionBinding binding,
Function<DependencyRequest, CodeBlock> dependencyUsage,
Function<XVariableElement, String> uniqueAssistedParameterName) {
Function<XExecutableParameterElement, String> uniqueAssistedParameterName) {
ImmutableMap<XExecutableParameterElement, DependencyRequest> dependencyRequestMap =
binding.provisionDependencies().stream()
.collect(
Expand Down Expand Up @@ -464,7 +463,11 @@ private static CodeBlock copyParameters(
return parameters.stream()
.map(
parameter -> {
String name = parameterNameSet.getUniqueName(validJavaName(getSimpleName(parameter)));
String name =
parameterNameSet.getUniqueName(
isMethodParameter(parameter)
? asMethodParameter(parameter).getJvmName()
: getSimpleName(parameter));
boolean useObject = !isRawTypePubliclyAccessible(parameter.getType());
return copyParameter(methodBuilder, parameter.getType(), name, useObject);
})
Expand All @@ -488,19 +491,4 @@ private static CodeBlock copyInstance(
// If we had to cast the instance add an extra parenthesis incase we're calling a method on it.
return useObject ? CodeBlock.of("($L)", instance) : instance;
}

private static String validJavaName(CharSequence name) {
if (SourceVersion.isIdentifier(name)) {
return protectAgainstKeywords(name.toString());
}

StringBuilder newName = new StringBuilder(name.length());
char firstChar = name.charAt(0);
if (!Character.isJavaIdentifierStart(firstChar)) {
newName.append('_');
}

name.chars().forEach(c -> newName.append(Character.isJavaIdentifierPart(c) ? c : '_'));
return newName.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package dagger.internal.codegen.writing;

import static com.google.common.collect.Iterables.getOnlyElement;
import static dagger.internal.codegen.xprocessing.XElements.getSimpleName;

import androidx.room.compiler.processing.XExecutableParameterElement;
import androidx.room.compiler.processing.XMethodElement;
Expand Down Expand Up @@ -56,7 +55,7 @@ protected Expression getDependencyExpressionForComponentMethod(
XMethodElement methodElement = componentMethod.methodElement();
XExecutableParameterElement parameter = getOnlyElement(methodElement.getParameters());
return membersInjectionMethods.getInjectExpression(
binding.key(), CodeBlock.of("$L", getSimpleName(parameter)), component.name());
binding.key(), CodeBlock.of("$L", parameter.getJvmName()), component.name());
}

// TODO(bcorso): Consider making this a method on all RequestRepresentations.
Expand Down
3 changes: 1 addition & 2 deletions java/dagger/internal/codegen/xprocessing/JavaPoetExt.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ public static TypeSpec.Builder avoidClashesWithNestedClasses(
}

public static ParameterSpec toParameterSpec(XExecutableParameterElement param) {
return ParameterSpec.builder(param.getType().getTypeName(), XElements.getSimpleName(param))
.build();
return ParameterSpec.builder(param.getType().getTypeName(), param.getJvmName()).build();
}

private JavaPoetExt() {}
Expand Down
3 changes: 1 addition & 2 deletions java/dagger/internal/codegen/xprocessing/MethodSpecs.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package dagger.internal.codegen.xprocessing;

import static dagger.internal.codegen.xprocessing.XElements.getSimpleName;
import static javax.lang.model.element.Modifier.PROTECTED;
import static javax.lang.model.element.Modifier.PUBLIC;

Expand Down Expand Up @@ -47,7 +46,7 @@ public static MethodSpec.Builder overriding(XMethodElement method, XType owner)
builder.addModifiers(PROTECTED);
}
for (int i = 0; i < methodType.getParameterTypes().size(); i++) {
String parameterName = getSimpleName(method.getParameters().get(i));
String parameterName = method.getParameters().get(i).getJvmName();
TypeName parameterType = methodType.getParameterTypes().get(i).getTypeName();
builder.addParameter(ParameterSpec.builder(parameterType, parameterName).build());
}
Expand Down
Binary file modified java/dagger/internal/codegen/xprocessing/xprocessing.jar
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* 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.functional.kotlinsrc.assisted

import com.google.common.truth.Truth.assertThat
import dagger.BindsInstance
import dagger.Component
import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.JUnit4

// Regression test for https://github.com/google/dagger/issues/3995.
@RunWith(JUnit4::class)
internal class AssistedFactoryWithJavaKeyWordNamesTest {
@Component
interface MyComponent {
fun myAssistedFactory(): MyAssistedFactory

@Component.Builder
interface Builder {
@BindsInstance fun addInteger(int: Int): Builder
fun build(): MyComponent
}
}

data class MyAssistedClass @AssistedInject constructor(
val int: Int,
@Assisted val string: String,
@Assisted val long: Long
)

@AssistedFactory
interface MyAssistedFactory {
fun create(long: Long, string: String): MyAssistedClass
}

@Test
fun testParametersWithJavaKeywordNames() {
val int = 1
val long = 2L
val string = "string"
val myAssistedFactory =
DaggerAssistedFactoryWithJavaKeyWordNamesTest_MyComponent.builder()
.addInteger(int)
.build()
.myAssistedFactory()
.create(long, string)
assertThat(myAssistedFactory.int).isEqualTo(int)
assertThat(myAssistedFactory.long).isEqualTo(long)
assertThat(myAssistedFactory.string).isEqualTo(string)
}
}
Loading

0 comments on commit 2c31d66

Please sign in to comment.