Skip to content

Commit

Permalink
Check if user added methods have name that is a java reserved keyword.
Browse files Browse the repository at this point in the history
Fixes #2576

RELNOTES=Check if user added methods have name that is a java reserved keyword.
PiperOrigin-RevId: 384781488
  • Loading branch information
java-team-github-bot authored and Dagger Team committed Jul 14, 2021
1 parent e152ea0 commit 436ac2e
Show file tree
Hide file tree
Showing 7 changed files with 209 additions and 5 deletions.
15 changes: 15 additions & 0 deletions java/dagger/internal/codegen/kotlin/KotlinMetadataUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import static com.google.auto.common.AnnotationMirrors.getAnnotatedAnnotations;
import static com.google.auto.common.MoreElements.isAnnotationPresent;
import static com.google.common.base.Preconditions.checkState;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableMap;
import static dagger.internal.codegen.langmodel.DaggerElements.closestEnclosingTypeElement;
import static kotlinx.metadata.Flag.Class.IS_COMPANION_OBJECT;
import static kotlinx.metadata.Flag.Class.IS_DATA;
Expand All @@ -26,7 +28,9 @@

import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import dagger.internal.codegen.extension.DaggerCollectors;
import dagger.internal.codegen.kotlin.KotlinMetadata.FunctionMetadata;
import java.lang.annotation.Annotation;
import java.util.Optional;
import javax.inject.Inject;
Expand Down Expand Up @@ -162,6 +166,17 @@ public boolean containsConstructorWithDefaultParam(TypeElement typeElement) {
&& metadataFactory.create(typeElement).containsConstructorWithDefaultParam();
}

/**
* Returns a map mapping all method signatures within the given class element, including methods
* that it inherits from its ancestors, to their method names.
*/
public ImmutableMap<String, String> getAllMethodNamesBySignature(TypeElement element) {
checkState(
hasMetadata(element), "Can not call getAllMethodNamesBySignature for non-Kotlin class");
return metadataFactory.create(element).classMetadata().functionsBySignature().values().stream()
.collect(toImmutableMap(FunctionMetadata::signature, FunctionMetadata::name));
}

/**
* Returns {@code true} if the <code>@JvmStatic</code> annotation is present in the given element.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static dagger.internal.codegen.base.Util.reentrantComputeIfAbsent;
import static dagger.internal.codegen.binding.ComponentCreatorAnnotation.getCreatorAnnotations;
import static dagger.internal.codegen.langmodel.DaggerElements.isAnnotationPresent;
import static javax.lang.model.SourceVersion.isKeyword;
import static javax.lang.model.element.Modifier.ABSTRACT;
import static javax.lang.model.element.Modifier.PRIVATE;
import static javax.lang.model.element.Modifier.STATIC;
Expand All @@ -34,6 +35,7 @@
import dagger.internal.codegen.binding.ErrorMessages;
import dagger.internal.codegen.binding.ErrorMessages.ComponentCreatorMessages;
import dagger.internal.codegen.javapoet.TypeNames;
import dagger.internal.codegen.kotlin.KotlinMetadataUtil;
import dagger.internal.codegen.langmodel.DaggerElements;
import dagger.internal.codegen.langmodel.DaggerTypes;
import java.util.HashMap;
Expand All @@ -58,11 +60,14 @@ public final class ComponentCreatorValidator implements ClearableCache {
private final DaggerElements elements;
private final DaggerTypes types;
private final Map<TypeElement, ValidationReport<TypeElement>> reports = new HashMap<>();
private final KotlinMetadataUtil metadataUtil;

@Inject
ComponentCreatorValidator(DaggerElements elements, DaggerTypes types) {
ComponentCreatorValidator(
DaggerElements elements, DaggerTypes types, KotlinMetadataUtil metadataUtil) {
this.elements = elements;
this.types = types;
this.metadataUtil = metadataUtil;
}

@Override
Expand Down Expand Up @@ -206,6 +211,7 @@ private void validateTypeRequirements() {
}

private void validateBuilder() {
validateClassMethodName();
ExecutableElement buildMethod = null;
for (ExecutableElement method : elements.getUnimplementedMethods(type)) {
switch (method.getParameters().size()) {
Expand Down Expand Up @@ -244,6 +250,21 @@ private void validateBuilder() {
}
}

private void validateClassMethodName() {
// Only Kotlin class can have method name the same as a Java reserved keyword, so only check
// the method name if this class is a Kotlin class.
if (metadataUtil.hasMetadata(type)) {
metadataUtil
.getAllMethodNamesBySignature(type)
.forEach(
(signature, name) -> {
if (isKeyword(name)) {
report.addError("Can not use a Java keyword as method name: " + signature);
}
});
}
}

private void validateSetterMethod(ExecutableElement method) {
TypeMirror returnType = types.resolveExecutableType(method, type.asType()).getReturnType();
if (returnType.getKind() != TypeKind.VOID && !types.isSubtype(type.asType(), returnType)) {
Expand Down
21 changes: 20 additions & 1 deletion java/dagger/internal/codegen/validation/ComponentValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import dagger.internal.codegen.binding.MethodSignatureFormatter;
import dagger.internal.codegen.binding.ModuleKind;
import dagger.internal.codegen.javapoet.TypeNames;
import dagger.internal.codegen.kotlin.KotlinMetadataUtil;
import dagger.internal.codegen.langmodel.DaggerElements;
import dagger.internal.codegen.langmodel.DaggerTypes;
import dagger.producers.ProductionComponent;
Expand All @@ -79,6 +80,7 @@
import java.util.Set;
import javax.inject.Inject;
import javax.inject.Singleton;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
Expand All @@ -105,6 +107,7 @@ public final class ComponentValidator implements ClearableCache {
private final MethodSignatureFormatter methodSignatureFormatter;
private final DependencyRequestFactory dependencyRequestFactory;
private final Map<TypeElement, ValidationReport<TypeElement>> reports = new HashMap<>();
private final KotlinMetadataUtil metadataUtil;

@Inject
ComponentValidator(
Expand All @@ -115,7 +118,8 @@ public final class ComponentValidator implements ClearableCache {
DependencyRequestValidator dependencyRequestValidator,
MembersInjectionValidator membersInjectionValidator,
MethodSignatureFormatter methodSignatureFormatter,
DependencyRequestFactory dependencyRequestFactory) {
DependencyRequestFactory dependencyRequestFactory,
KotlinMetadataUtil metadataUtil) {
this.elements = elements;
this.types = types;
this.moduleValidator = moduleValidator;
Expand All @@ -124,6 +128,7 @@ public final class ComponentValidator implements ClearableCache {
this.membersInjectionValidator = membersInjectionValidator;
this.methodSignatureFormatter = methodSignatureFormatter;
this.dependencyRequestFactory = dependencyRequestFactory;
this.metadataUtil = metadataUtil;
}

@Override
Expand Down Expand Up @@ -241,11 +246,25 @@ private void validateNoReusableAnnotation() {
}

private void validateComponentMethods() {
validateClassMethodName();
elements.getUnimplementedMethods(component).stream()
.map(ComponentMethodValidator::new)
.forEachOrdered(ComponentMethodValidator::validateMethod);
}

private void validateClassMethodName() {
if (metadataUtil.hasMetadata(component)) {
metadataUtil
.getAllMethodNamesBySignature(component)
.forEach(
(signature, name) -> {
if (SourceVersion.isKeyword(name)) {
report.addError("Can not use a Java keyword as method name: " + signature);
}
});
}
}

private class ComponentMethodValidator {
private final ExecutableElement method;
private final ExecutableType resolvedMethod;
Expand Down
2 changes: 2 additions & 0 deletions java/dagger/testing/compile/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,7 @@ java_library(
"//java/dagger/internal/guava:collect",
"//java/dagger/internal/guava:io",
"@google_bazel_common//third_party/java/compile_testing",
"//java/dagger/internal/codegen:processor",
"@maven//:com_github_tschuchortdev_kotlin_compile_testing",
],
)
18 changes: 15 additions & 3 deletions java/dagger/testing/compile/CompilerTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@
import java.nio.file.Paths;
import java.util.Map;
import java.util.NoSuchElementException;
import com.tschuchort.compiletesting.KotlinCompilation;
import dagger.internal.codegen.ComponentProcessor;

/**
* A helper class for working with java compiler tests.
*/
/** A helper class for working with java compiler tests. */
public final class CompilerTests {
private CompilerTests() {}

Expand All @@ -53,6 +53,18 @@ public static Compiler compiler() {
return javac().withClasspath(ImmutableList.of(compilerDepsJar()));
}

public static KotlinCompilation kotlinCompiler() {
KotlinCompilation compilation = new KotlinCompilation();
compilation.setAnnotationProcessors(ImmutableList.of(new ComponentProcessor()));
compilation.setClasspaths(
ImmutableList.<java.io.File>builder()
.addAll(compilation.getClasspaths())
.add(compilerDepsJar())
.build()
);
return compilation;
}

private static File getRunfilesDir() {
return getRunfilesPath().toFile();
}
Expand Down
29 changes: 29 additions & 0 deletions javatests/dagger/internal/codegen/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library")
load("//:build_defs.bzl", "DOCLINT_HTML_AND_SYNTAX")
load("//:test_defs.bzl", "GenJavaTests")
load("//java/dagger/testing/compile:macros.bzl", "compiler_test")

package(default_visibility = ["//:src"])

Expand Down Expand Up @@ -58,6 +59,7 @@ GenJavaTests(
"CompilerMode.java",
"Compilers.java",
"JavaFileBuilder.java",
"ComponentValidationKtTest.java",
],
),
functional = False,
Expand Down Expand Up @@ -96,3 +98,30 @@ GenJavaTests(
"@google_bazel_common//third_party/java/truth",
],
)

compiler_test(
name = "ComponentValidationKtTest",
srcs = ["ComponentValidationKtTest.java"],
compiler_deps = [
"//java/dagger:core",
"//java/dagger/internal/codegen:package_info",
"//java/dagger/internal/codegen:processor",
"//java/dagger/internal/codegen/base",
"//java/dagger/internal/codegen/binding",
"//java/dagger/internal/codegen/bindinggraphvalidation",
"//java/dagger/internal/codegen/compileroption",
"//java/dagger/internal/codegen/javapoet",
"//java/dagger/internal/codegen/langmodel",
"//java/dagger/internal/codegen/validation",
"//java/dagger/internal/codegen/writing",
"//java/dagger/model/testing",
"//java/dagger/producers",
"//java/dagger/spi",
],
deps = [
"//java/dagger/internal/guava:collect",
"@google_bazel_common//third_party/java/junit",
"@google_bazel_common//third_party/java/truth",
"@maven//:com_github_tschuchortdev_kotlin_compile_testing",
],
)
106 changes: 106 additions & 0 deletions javatests/dagger/internal/codegen/ComponentValidationKtTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright (C) 2021 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.internal.codegen;

import static com.google.common.truth.Truth.assertThat;
import static dagger.testing.compile.CompilerTests.kotlinCompiler;

import com.google.common.collect.ImmutableList;
import com.tschuchort.compiletesting.KotlinCompilation;
import com.tschuchort.compiletesting.KotlinCompilation.ExitCode;
import com.tschuchort.compiletesting.SourceFile;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public final class ComponentValidationKtTest {
@Test
public void creatorMethodNameIsJavaKeyword_compilationError() {
SourceFile componentSrc =
SourceFile.Companion.kotlin(
"FooComponent.kt",
String.join(
"\n",
"package test",
"",
"import dagger.BindsInstance",
"import dagger.Component",
"",
"@Component",
"interface FooComponent {",
" @Component.Builder",
" interface Builder {",
" @BindsInstance public fun int(str: Int): Builder",
" public fun build(): FooComponent",
" }",
"}"),
false);
KotlinCompilation compilation = kotlinCompiler();
compilation.setSources(ImmutableList.of(componentSrc));

KotlinCompilation.Result result = compilation.compile();

// TODO(b/192396673): Add error count when the feature request is fulfilled.
assertThat(result.getExitCode()).isEqualTo(ExitCode.COMPILATION_ERROR);
assertThat(result.getMessages())
.contains("Can not use a Java keyword as method name: int(I)Ltest/FooComponent$Builder");
}

@Test
public void componentMethodNameIsJavaKeyword_compilationError() {
SourceFile componentSrc =
SourceFile.Companion.kotlin(
"FooComponent.kt",
String.join(
"\n",
"package test",
"",
"import dagger.BindsInstance",
"import dagger.Component",
"",
"@Component(modules = [TestModule::class])",
"interface FooComponent {",
" fun int(str: Int): String",
"}"),
false);
SourceFile moduleSrc =
SourceFile.Companion.kotlin(
"TestModule.kt",
String.join(
"\n",
"package test",
"",
"import dagger.Module",
"",
"@Module",
"interface TestModule {",
" fun providesString(): String {",
" return \"test\"",
" }",
"}"),
false);
KotlinCompilation compilation = kotlinCompiler();
compilation.setSources(ImmutableList.of(componentSrc, moduleSrc));

KotlinCompilation.Result result = compilation.compile();

assertThat(result.getExitCode()).isEqualTo(ExitCode.COMPILATION_ERROR);
assertThat(result.getMessages())
.contains("Can not use a Java keyword as method name: int(I)Ljava/lang/String");
}
}

0 comments on commit 436ac2e

Please sign in to comment.