Skip to content

Commit

Permalink
fix: 🚑 [BUG] Generic type with concrete type parameters generate non-…
Browse files Browse the repository at this point in the history
…compiling code (#139)

* Create draft PR for #138
[skip ci]

* fix: Generates records with complicated Generics structure

---------

Co-authored-by: pawellabaj <pawellabaj@users.noreply.github.com>
Co-authored-by: Pawel_Labaj <Pawel_Labaj@epam.com>
  • Loading branch information
3 people authored Nov 11, 2023
1 parent 92850f8 commit 708cf28
Show file tree
Hide file tree
Showing 17 changed files with 454 additions and 125 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package pl.com.labaj.autorecord.generic;

/*-
* Copyright © 2023 Auto Record
*
* 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.
*/

import com.google.testing.compile.Compiler;
import com.google.testing.compile.JavaFileObjects;
import org.junit.jupiter.api.Test;
import pl.com.labaj.autorecord.processor.AutoRecordProcessor;

import java.util.List;

import static com.google.testing.compile.Compiler.javac;
import static org.junit.jupiter.api.Assertions.assertAll;
import static pl.com.labaj.autorecord.test.TestUtils.assertThat;
import static pl.com.labaj.autorecord.test.TestUtils.expectedGenearatedRecordName;
import static pl.com.labaj.autorecord.test.TestUtils.expectedResourceName;
import static pl.com.labaj.autorecord.test.TestUtils.inputResourceName;

class GenericGenerationTest {

private final Compiler compiler = javac().withProcessors(new AutoRecordProcessor());

@Test
void shouldGenerateRecordWithProperGenerics() {
//given
var inputInterfaces = List.of(
JavaFileObjects.forResource(inputResourceName("BaseType")),
JavaFileObjects.forResource(inputResourceName("ConcreteType")),
JavaFileObjects.forResource(inputResourceName("WithGeneric")),
JavaFileObjects.forResource(inputResourceName("WithSubGeneric")));
var expectedRecord = JavaFileObjects.forResource(expectedResourceName("WithSubGeneric"));

//when
var compilation = compiler.compile(inputInterfaces);

//then
assertAll(
() -> assertThat(compilation).succeeded(),
() -> assertThat(compilation).generatedSourceFile(expectedGenearatedRecordName("WithSubGeneric")).hasSourceEquivalentTo(expectedRecord)
);
}
}
20 changes: 20 additions & 0 deletions modules/auto-record-tests/src/test/resources/in/BaseType.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package pl.com.labaj.autorecord.testcase;

/*-
* Copyright © 2023 Auto Record
*
* 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.
*/

interface BaseType {
}
20 changes: 20 additions & 0 deletions modules/auto-record-tests/src/test/resources/in/ConcreteType.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package pl.com.labaj.autorecord.testcase;

/*-
* Copyright © 2023 Auto Record
*
* 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.
*/

interface ConcreteType extends BaseType {
}
37 changes: 37 additions & 0 deletions modules/auto-record-tests/src/test/resources/in/WithGeneric.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package pl.com.labaj.autorecord.testcase;

/*-
* Copyright © 2023 Auto Record
*
* 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.
*/

import pl.com.labaj.autorecord.Memoized;

import java.util.List;

interface WithGeneric<A extends BaseType, B> {
A a();
List<A> listOfA();
B b();

@Memoized
default A memA() {
return a();
}

@Memoized
default String fromAB(A anA, B anB) {
return String.valueOf(anA) + String.valueOf(anB);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package pl.com.labaj.autorecord.testcase;

/*-
* Copyright © 2023 Auto Record
*
* 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.
*/

import pl.com.labaj.autorecord.AutoRecord;
import pl.com.labaj.autorecord.Memoized;

import java.util.Collection;
import java.util.List;

@AutoRecord
interface WithSubGeneric<X, Y extends Collection<X>> extends WithGeneric<ConcreteType, X> {
Y y();

@Memoized
default Y memY() {
return y();
}

@Memoized
String toString();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package pl.com.labaj.autorecord.testcase;

/*-
* Copyright © 2023 Auto Record
*
* 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.
*/

import static java.util.Objects.requireNonNull;
import static java.util.Objects.requireNonNullElseGet;

import java.lang.Override;
import java.lang.String;
import java.util.Collection;
import java.util.List;
import javax.annotation.Nullable;
import javax.annotation.processing.Generated;

import pl.com.labaj.autorecord.GeneratedWithAutoRecord;
import pl.com.labaj.autorecord.Memoized;
import pl.com.labaj.autorecord.memoizer.Memoizer;

@Generated("pl.com.labaj.autorecord.AutoRecord")
@GeneratedWithAutoRecord
record WithSubGenericRecord<X, Y extends Collection<X>>(ConcreteType a,
List<ConcreteType> listOfA,
X b,
Y y,
@Nullable Memoizer<ConcreteType> memAMemoizer,
@Nullable Memoizer<String> fromABMemoizer,
@Nullable Memoizer<Y> memYMemoizer,
@Nullable Memoizer<String> toStringMemoizer) implements WithSubGeneric<X, Y> {
WithSubGenericRecord {
requireNonNull(a, "a must not be null");
requireNonNull(listOfA, "listOfA must not be null");
requireNonNull(b, "b must not be null");
requireNonNull(y, "y must not be null");

memAMemoizer = requireNonNullElseGet(memAMemoizer, Memoizer::new);
fromABMemoizer = requireNonNullElseGet(fromABMemoizer, Memoizer::new);
memYMemoizer = requireNonNullElseGet(memYMemoizer, Memoizer::new);
toStringMemoizer = requireNonNullElseGet(toStringMemoizer, Memoizer::new);
}

WithSubGenericRecord(ConcreteType a, List<ConcreteType> listOfA, X b, Y y) {
this(a, listOfA, b, y, new Memoizer<>(), new Memoizer<>(), new Memoizer<>(), new Memoizer<>());
}

@Memoized
@Override
public ConcreteType memA() {
return memAMemoizer.computeIfAbsent(WithSubGeneric.super::memA);
}

@Memoized
@Override
public String fromAB(ConcreteType anA, X anB) {
return fromABMemoizer.computeIfAbsent(() -> WithSubGeneric.super.fromAB(anA, anB));
}

@Memoized
@Override
public Y memY() {
return memYMemoizer.computeIfAbsent(WithSubGeneric.super::memY);
}

@Memoized
@Override
public String toString() {
return toStringMemoizer.computeIfAbsent(this::_toString);
}

private String _toString() {
return "WithSubGenericRecord[" +
"a = " + a + ", " +
"listOfA = " + listOfA + ", " +
"b = " + b + ", " +
"y = " + y +
"]";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@

import pl.com.labaj.autorecord.context.RecordComponent;
import pl.com.labaj.autorecord.processor.AutoRecordProcessorException;
import pl.com.labaj.autorecord.processor.utils.Methods;

import javax.lang.model.element.ExecutableElement;
import java.util.List;
import java.util.Set;
import java.util.function.Predicate;
Expand All @@ -34,28 +32,28 @@ class ComponentsFinder {

public static final String ERROR_INDICATOR = "<any>";

List<RecordComponent> getComponents(List<ExecutableElement> allMethods, Predicate<ExecutableElement> isNotSpecial) {
List<RecordComponent> getComponents(List<Method> allMethods, Predicate<Method> isNotSpecial) {
return allMethods.stream()
.filter(Methods::isAbstract)
.filter(Method::isAbstract)
.filter(isNotSpecial)
.filter(Methods::hasNoParameters)
.filter(Methods::isNotVoid)
.filter(Method::hasNoParameters)
.filter(Method::isNotVoid)
.filter(InternalMethod::isNotInternal)
.map(this::toRecordComponent)
.toList();
}

private RecordComponent toRecordComponent(ExecutableElement method) {
var returnType = method.getReturnType();
private RecordComponent toRecordComponent(Method method) {
var name = method.name();
var returnType = method.returnType();

if (returnType.getKind() == ERROR && returnType.toString().equals(ERROR_INDICATOR)) {
throw new AutoRecordProcessorException("Cannot infer type of " + method.getSimpleName() + "() method. " +
throw new AutoRecordProcessorException("Cannot infer type of " + name + "() method. " +
"Probably it is generic and not in classpath or sourcepath yet. " +
"Try to move the type class into classpath or remove generic clause from " + method.getSimpleName() + "() method.");
"Try to move the type class into classpath or remove generic clause from " + name + "() method.");
}

var name = method.getSimpleName().toString();
var annotations = annotationsAllowedFor(method.getAnnotationMirrors(), Set.of(PARAMETER, RECORD_COMPONENT));
var annotations = annotationsAllowedFor(method.annotations(), Set.of(PARAMETER, RECORD_COMPONENT));

return new ProcessorRecordComponent(returnType, name, annotations);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import io.soabase.recordbuilder.core.RecordBuilder;
import pl.com.labaj.autorecord.AutoRecord;
import pl.com.labaj.autorecord.extension.AutoRecordExtension;
import pl.com.labaj.autorecord.processor.utils.Methods;

import javax.annotation.Nullable;
import javax.annotation.processing.ProcessingEnvironment;
Expand All @@ -30,6 +29,7 @@
import java.util.Map;
import java.util.function.Consumer;

import static javax.lang.model.element.ElementKind.METHOD;
import static javax.lang.model.element.Modifier.PUBLIC;
import static pl.com.labaj.autorecord.processor.utils.Annotations.createAnnotationIfNeeded;

Expand Down Expand Up @@ -61,16 +61,18 @@ public ProcessorContext buildContext(TypeElement sourceInterface,
var nonNullBuilderOptions = createAnnotationIfNeeded(builderOptions, RecordBuilder.Options.class, getBuilderOptionsEnforcedValues(extensions));

var elementUtils = processingEnv.getElementUtils();
var typeUtils = processingEnv.getTypeUtils();
var allMethods = elementUtils.getAllMembers(sourceInterface).stream()
.filter(Methods::isMethod)
.filter(element -> element.getKind() == METHOD)
.map(ExecutableElement.class::cast)
.map(method -> Method.from(method, typeUtils, sourceInterface))
.toList();

boolean isPublic = sourceInterface.getModifiers().contains(PUBLIC);
var typeParameters = getTypeParameters(sourceInterface);

var specialMethodAnnotations = specialMethodsFinder.findSpecialMethods(allMethods);
var memoizationItems = memoizationFinder.findMemoizationItems(allMethods, nonNullRecordOptions, specialMethodsFinder::isSpecial);
var memoizationItems = memoizationFinder.findMemoizationItems(allMethods, nonNullRecordOptions, specialMethodsFinder::isSpecial, logger);
var components = componentsFinder.getComponents(allMethods, specialMethodsFinder::isNotSpecial);

return new ProcessorContext(processingEnv,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import pl.com.labaj.autorecord.processor.AutoRecordProcessorException;

import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.type.TypeVisitor;
Expand Down Expand Up @@ -80,12 +79,11 @@ public static Stream<InternalMethod> allInternalMethods() {
return ALL_METHODS.stream();
}

public static boolean isInternal(ExecutableElement method) {
var methodName = method.getSimpleName().toString();
return METHOD_NAMES.contains(methodName) && method.getParameters().isEmpty();
public static boolean isInternal(Method method) {
return METHOD_NAMES.contains(method.name()) && method.hasNoParameters();
}

public static boolean isNotInternal(ExecutableElement method) {
public static boolean isNotInternal(Method method) {
return !isInternal(method);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public boolean isPresent() {
return !isEmpty();
}

public record Item(TypeMirror type, String name, List<AnnotationMirror> annotations, boolean internal) {
public record Item(TypeMirror type, String name, List<AnnotationMirror> annotations, List<Method.Parameter> parameters, boolean internal) {
public String getMemoizerName() {
return name + "Memoizer";
}
Expand All @@ -66,7 +66,8 @@ public Item mergeWith(Item otherItem) {
var mergedAnnotations = Stream.concat(annotations.stream(), otherItem.annotations.stream())
.distinct()
.toList();
return new Item(type, name, mergedAnnotations, internal || otherItem.internal);

return new Item(type, name, mergedAnnotations, parameters, internal || otherItem.internal);
}
}
}
Loading

0 comments on commit 708cf28

Please sign in to comment.