Skip to content

Commit

Permalink
Fix type inference issues with generated assisted factory in Java 7.
Browse files Browse the repository at this point in the history
To test this, I've updated our gradle tests to allow inputing a Java version as a cmd-line argument, and I've added a regression test for this issue.

Fixes #2309

RELNOTES=Fixes #2309: Fixes type inference issues with generated assisted factory in Java 7.
PiperOrigin-RevId: 354200817
  • Loading branch information
bcorso authored and Dagger Team committed Jan 28, 2021
1 parent 229c99f commit 7ce9d0c
Show file tree
Hide file tree
Showing 10 changed files with 341 additions and 11 deletions.
9 changes: 6 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@ jobs:
name: local-snapshot
path: ~/.m2/repository/com/google/dagger
artifact-local-tests:
name: 'Artifact local tests'
name: 'Artifact local tests (Java ${{ matrix.java_version }})'
needs: bazel-test
runs-on: ubuntu-latest
strategy:
matrix:
java_version: ['VERSION_1_7', 'VERSION_1_8']
steps:
- name: 'Check out repository'
uses: actions/checkout@v2
Expand All @@ -45,8 +48,8 @@ jobs:
with:
name: local-snapshot
path: ~/.m2/repository/com/google/dagger
- name: 'Gradle tests'
run: ./util/run-local-gradle-tests.sh
- name: 'Gradle tests (Java ${{ matrix.java_version }})'
run: ./util/run-local-gradle-tests.sh "${{ matrix.java_version }}"
shell: bash
artifact-emulator-tests:
name: 'Artifact emulator tests (AGP ${{ matrix.agp }})'
Expand Down
15 changes: 11 additions & 4 deletions java/dagger/internal/codegen/AssistedFactoryProcessingStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -299,15 +299,18 @@ public Element originatingElement(ProvisionBinding binding) {
@Override
public Optional<TypeSpec.Builder> write(ProvisionBinding binding) {
TypeElement factory = asType(binding.bindingElement().get());
DeclaredType factoryType = asDeclared(binding.key().type());
ExecutableElement factoryMethod =
AssistedInjectionAnnotations.assistedFactoryMethod(factory, elements, types);
ExecutableType factoryMethodType =
asExecutable(types.asMemberOf(asDeclared(binding.key().type()), factoryMethod));
asExecutable(types.asMemberOf(factoryType, factoryMethod));
TypeElement returnElement = asTypeElement(factoryMethodType.getReturnType());
ParameterSpec delegateFactoryParam =
ParameterSpec.builder(delegateFactoryTypeName(returnElement), "delegateFactory").build();

ClassName name = nameGeneratedType(binding);
TypeSpec.Builder builder =
TypeSpec.classBuilder(nameGeneratedType(binding))
TypeSpec.classBuilder(name)
.addModifiers(PUBLIC, FINAL)
.addTypeVariables(
factory.getTypeParameters().stream()
Expand Down Expand Up @@ -349,9 +352,13 @@ public Optional<TypeSpec.Builder> write(ProvisionBinding binding) {
.collect(toImmutableList()))
.returns(providerOf(TypeName.get(factory.asType())))
.addStatement(
"return $T.create(new $T($N))",
"return $T.$Lcreate(new $T($N))",
INSTANCE_FACTORY,
nameGeneratedType(binding),
// Java 7 type inference requires the method call provide the exact type here.
sourceVersion.compareTo(SourceVersion.RELEASE_7) <= 0
? CodeBlock.of("<$T>", types.accessibleType(factoryType, name))
: CodeBlock.of(""),
name,
delegateFactoryParam)
.build());
return Optional.of(builder);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* 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.example.gradle.assistedinject;

import dagger.Component;
import dagger.assisted.Assisted;
import dagger.assisted.AssistedFactory;
import dagger.assisted.AssistedInject;
import javax.inject.Inject;

// This is a regression test for https://github.com/google/dagger/issues/2309
/** A simple, skeletal application that defines an assisted inject binding. */
public class AssistedInjectApplication {
@Component
interface MyComponent {
FooFactory fooFactory();

ParameterizedFooFactory<Bar, String> parameterizedFooFactory();
}

static final class Bar {
@Inject
Bar() {}
}

static class Foo {
@AssistedInject
Foo(Bar bar, @Assisted String str) {}
}

@AssistedFactory
interface FooFactory {
Foo create(String str);
}

static class ParameterizedFoo<T1, T2> {
@AssistedInject
ParameterizedFoo(T1 t1, @Assisted T2 t2) {}
}

@AssistedFactory
interface ParameterizedFooFactory<T1, T2> {
ParameterizedFoo<T1, T2> create(T2 t2);
}

public static void main(String[] args) {
Foo foo = DaggerAssistedInjectApplication_MyComponent.create().fooFactory().create("");

ParameterizedFoo<Bar, String> parameterizedFoo =
DaggerAssistedInjectApplication_MyComponent.create()
.parameterizedFooFactory()
.create("");
}
}
48 changes: 48 additions & 0 deletions javatests/artifacts/dagger/assistedinject/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* 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.
*/

plugins {
id 'java'
id 'application'
}

ext {
java_version = System.getenv('JAVA_VERSION') ?: JavaVersion.VERSION_1_8
}
repositories {
mavenCentral()
mavenLocal()
}

sourceSets {
main {
java {
srcDir '.'
}
}
}

java {
sourceCompatibility = java_version
targetCompatibility = java_version
}

dependencies {
implementation 'com.google.dagger:dagger:LOCAL-SNAPSHOT'
annotationProcessor 'com.google.dagger:dagger-compiler:LOCAL-SNAPSHOT'
}

mainClassName = 'dagger.example.gradle.assistedinject.AssistedInjectApplication'
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-6.7-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
188 changes: 188 additions & 0 deletions javatests/artifacts/dagger/assistedinject/gradlew
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
#!/usr/bin/env sh

#
# Copyright 2015 the original author or 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.
#

##############################################################################
##
## Gradle start up script for UN*X
##
##############################################################################

# Attempt to set APP_HOME
# Resolve links: $0 may be a link
PRG="$0"
# Need this for relative symlinks.
while [ -h "$PRG" ] ; do
ls=`ls -ld "$PRG"`
link=`expr "$ls" : '.*-> \(.*\)$'`
if expr "$link" : '/.*' > /dev/null; then
PRG="$link"
else
PRG=`dirname "$PRG"`"/$link"
fi
done
SAVED="`pwd`"
cd "`dirname \"$PRG\"`/" >/dev/null
APP_HOME="`pwd -P`"
cd "$SAVED" >/dev/null

APP_NAME="Gradle"
APP_BASE_NAME=`basename "$0"`

# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'

# Use the maximum available, or set MAX_FD != -1 to use that value.
MAX_FD="maximum"

warn () {
echo "$*"
}

die () {
echo
echo "$*"
echo
exit 1
}

# OS specific support (must be 'true' or 'false').
cygwin=false
msys=false
darwin=false
nonstop=false
case "`uname`" in
CYGWIN* )
cygwin=true
;;
Darwin* )
darwin=true
;;
MINGW* )
msys=true
;;
NONSTOP* )
nonstop=true
;;
esac

CLASSPATH=$APP_HOME/gradle/wrapper/gradle-wrapper.jar

# Determine the Java command to use to start the JVM.
if [ -n "$JAVA_HOME" ] ; then
if [ -x "$JAVA_HOME/jre/sh/java" ] ; then
# IBM's JDK on AIX uses strange locations for the executables
JAVACMD="$JAVA_HOME/jre/sh/java"
else
JAVACMD="$JAVA_HOME/bin/java"
fi
if [ ! -x "$JAVACMD" ] ; then
die "ERROR: JAVA_HOME is set to an invalid directory: $JAVA_HOME
Please set the JAVA_HOME variable in your environment to match the
location of your Java installation."
fi
else
JAVACMD="java"
which java >/dev/null 2>&1 || die "ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
Please set the JAVA_HOME variable in your environment to match the
location of your Java installation."
fi

# Increase the maximum file descriptors if we can.
if [ "$cygwin" = "false" -a "$darwin" = "false" -a "$nonstop" = "false" ] ; then
MAX_FD_LIMIT=`ulimit -H -n`
if [ $? -eq 0 ] ; then
if [ "$MAX_FD" = "maximum" -o "$MAX_FD" = "max" ] ; then
MAX_FD="$MAX_FD_LIMIT"
fi
ulimit -n $MAX_FD
if [ $? -ne 0 ] ; then
warn "Could not set maximum file descriptor limit: $MAX_FD"
fi
else
warn "Could not query maximum file descriptor limit: $MAX_FD_LIMIT"
fi
fi

# For Darwin, add options to specify how the application appears in the dock
if $darwin; then
GRADLE_OPTS="$GRADLE_OPTS \"-Xdock:name=$APP_NAME\" \"-Xdock:icon=$APP_HOME/media/gradle.icns\""
fi

# For Cygwin, switch paths to Windows format before running java
if $cygwin ; then
APP_HOME=`cygpath --path --mixed "$APP_HOME"`
CLASSPATH=`cygpath --path --mixed "$CLASSPATH"`
JAVACMD=`cygpath --unix "$JAVACMD"`

# We build the pattern for arguments to be converted via cygpath
ROOTDIRSRAW=`find -L / -maxdepth 1 -mindepth 1 -type d 2>/dev/null`
SEP=""
for dir in $ROOTDIRSRAW ; do
ROOTDIRS="$ROOTDIRS$SEP$dir"
SEP="|"
done
OURCYGPATTERN="(^($ROOTDIRS))"
# Add a user-defined pattern to the cygpath arguments
if [ "$GRADLE_CYGPATTERN" != "" ] ; then
OURCYGPATTERN="$OURCYGPATTERN|($GRADLE_CYGPATTERN)"
fi
# Now convert the arguments - kludge to limit ourselves to /bin/sh
i=0
for arg in "$@" ; do
CHECK=`echo "$arg"|egrep -c "$OURCYGPATTERN" -`
CHECK2=`echo "$arg"|egrep -c "^-"` ### Determine if an option

if [ $CHECK -ne 0 ] && [ $CHECK2 -eq 0 ] ; then ### Added a condition
eval `echo args$i`=`cygpath --path --ignore --mixed "$arg"`
else
eval `echo args$i`="\"$arg\""
fi
i=$((i+1))
done
case $i in
(0) set -- ;;
(1) set -- "$args0" ;;
(2) set -- "$args0" "$args1" ;;
(3) set -- "$args0" "$args1" "$args2" ;;
(4) set -- "$args0" "$args1" "$args2" "$args3" ;;
(5) set -- "$args0" "$args1" "$args2" "$args3" "$args4" ;;
(6) set -- "$args0" "$args1" "$args2" "$args3" "$args4" "$args5" ;;
(7) set -- "$args0" "$args1" "$args2" "$args3" "$args4" "$args5" "$args6" ;;
(8) set -- "$args0" "$args1" "$args2" "$args3" "$args4" "$args5" "$args6" "$args7" ;;
(9) set -- "$args0" "$args1" "$args2" "$args3" "$args4" "$args5" "$args6" "$args7" "$args8" ;;
esac
fi

# Escape application args
save () {
for i do printf %s\\n "$i" | sed "s/'/'\\\\''/g;1s/^/'/;\$s/\$/' \\\\/" ; done
echo " "
}
APP_ARGS=$(save "$@")

# Collect all arguments for the java command, following the shell quoting and substitution rules
eval set -- $DEFAULT_JVM_OPTS $JAVA_OPTS $GRADLE_OPTS "\"-Dorg.gradle.appname=$APP_BASE_NAME\"" -classpath "\"$CLASSPATH\"" org.gradle.wrapper.GradleWrapperMain "$APP_ARGS"

# by default we should be in the correct project dir, but when run from Finder on Mac, the cwd is wrong
if [ "$(uname)" = "Darwin" ] && [ "$HOME" = "$PWD" ]; then
cd "$(dirname "$0")"
fi

exec "$JAVACMD" "$@"
8 changes: 8 additions & 0 deletions javatests/artifacts/dagger/simple/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ plugins {
id 'application'
}

ext {
java_version = System.getenv('JAVA_VERSION') ?: JavaVersion.VERSION_1_8
}
repositories {
mavenCentral()
mavenLocal()
Expand All @@ -32,6 +35,11 @@ sourceSets {
}
}

java {
sourceCompatibility = java_version
targetCompatibility = java_version
}

dependencies {
implementation 'com.google.dagger:dagger:LOCAL-SNAPSHOT'
annotationProcessor 'com.google.dagger:dagger-compiler:LOCAL-SNAPSHOT'
Expand Down
Loading

0 comments on commit 7ce9d0c

Please sign in to comment.