Skip to content

Commit

Permalink
Type validation for before sources in RewriteTest (#3564)
Browse files Browse the repository at this point in the history
* Apply type validations to before sources

Rename `SourceSpec.EachResult` to `SourceSpec.ValidateSource` as it now applies to before sources as well.

Currently, the before source validation can only be completely disabled (no fine-grained options independent of the validation of the after sources).

* Fix failing `TabsAndIndents` test case

* Use separate options to validate before source

* Default the `before` validations to the `after` validations

To avoid many "regressions".

* Make factory methods package private

* Correct more type attribution errors and adjust some tests

* Also fix tzpe attribution in other Javadoc parsers

* Rename `beforeTypeValidationOptions()` to `typeValidationOptions()`

... and `typeValidationOptions()` to `afterTypeValidationOptions()`.

* Follow through with `RecipeSpec` method rename

* Exclude 3 test cases from Java 8

* Correct `@MinimumJava11` annotation

Was annotated as `@Tag("java17")` rather than `@Tag("java11")`.
  • Loading branch information
knutwannheden authored Oct 27, 2023
1 parent 3e333fe commit 0d884d4
Show file tree
Hide file tree
Showing 36 changed files with 269 additions and 177 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.TypeValidation;

import java.util.Optional;

Expand Down Expand Up @@ -189,7 +190,8 @@ void onlyIfUsingCompileScope(String onlyIfUsing) {
@ValueSource(strings = {"com.google.common.math.*", "com.google.common.math.IntMath"})
void onlyIfUsingMultipleScopes(String onlyIfUsing) {
rewriteRun(
spec -> spec.recipe(addDependency("com.google.guava:guava:29.0-jre", onlyIfUsing)),
spec -> spec.recipe(addDependency("com.google.guava:guava:29.0-jre", onlyIfUsing))
.typeValidationOptions(TypeValidation.none()),
mavenProject("project",
srcMainJava(
java(usingGuavaIntMath)
Expand Down Expand Up @@ -230,7 +232,8 @@ void onlyIfUsingMultipleScopes(String onlyIfUsing) {
void usedInMultipleSourceSetsUsingExplicitSourceSet(String onlyIfUsing) {
AddDependency addDep = new AddDependency("com.google.guava", "guava", "29.0-jre", null, null, onlyIfUsing, null, null, null, Boolean.TRUE);
rewriteRun(
spec -> spec.recipe(addDep),
spec -> spec.recipe(addDep)
.typeValidationOptions(TypeValidation.none()),
mavenProject("project",
srcMainJava(
java(usingGuavaIntMath)
Expand Down Expand Up @@ -287,7 +290,8 @@ void usedInMultipleSourceSetsUsingExplicitSourceSet(String onlyIfUsing) {
void usedInTransitiveSourceSet() {
AddDependency addDep = new AddDependency("com.google.guava", "guava", "29.0-jre", null, null, "com.google.common.math.IntMath", null, null, null, Boolean.TRUE);
rewriteRun(
spec -> spec.recipe(addDep),
spec -> spec.recipe(addDep)
.typeValidationOptions(TypeValidation.none()),
mavenProject("project",
srcSmokeTestJava(
java(usingGuavaIntMath)
Expand Down Expand Up @@ -341,7 +345,8 @@ void usedInTransitiveSourceSet() {
void addDependencyIfNotUsedInATransitive() {
AddDependency addDep = new AddDependency("com.google.guava", "guava", "29.0-jre", null, null, "com.google.common.math.IntMath", null, null, null, Boolean.TRUE);
rewriteRun(
spec -> spec.recipe(addDep),
spec -> spec.recipe(addDep)
.typeValidationOptions(TypeValidation.none()),
mavenProject("project",
srcSmokeTestJava(
java(usingGuavaIntMath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class TaskTest implements RewriteTest {
@Test
void declareTaskOldStyle() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.none()),
spec -> spec.afterTypeValidationOptions(TypeValidation.none()),
buildGradle(
"""
task(testWithCloud, type: Test) {
Expand All @@ -44,7 +44,7 @@ void declareTaskOldStyle() {
@Test
void testDsl() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.none()),
spec -> spec.afterTypeValidationOptions(TypeValidation.none()),
buildGradle(
"""
plugins {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,11 @@ private JavaType.Method methodReferenceType(DCTree.DCReference ref, @Nullable Ja
for (JavaType testParamType : method.getParameterTypes()) {
Type paramType = attr.attribType(param, symbol);
if (testParamType instanceof JavaType.GenericTypeVariable) {
for (JavaType bound : ((JavaType.GenericTypeVariable) testParamType).getBounds()) {
List<JavaType> bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds();
if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) {
return method;
}
for (JavaType bound : bounds) {
if (paramTypeMatches(bound, paramType)) {
return method;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,11 @@ private JavaType.Method methodReferenceType(DCTree.DCReference ref, @Nullable Ja
for (JavaType testParamType : method.getParameterTypes()) {
Type paramType = attr.attribType(param, symbol);
if (testParamType instanceof JavaType.GenericTypeVariable) {
for (JavaType bound : ((JavaType.GenericTypeVariable) testParamType).getBounds()) {
List<JavaType> bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds();
if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) {
return method;
}
for (JavaType bound : bounds) {
if (paramTypeMatches(bound, paramType)) {
return method;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,11 @@ private JavaType.Method methodReferenceType(DCTree.DCReference ref, @Nullable Ja
for (JavaType testParamType : method.getParameterTypes()) {
Type paramType = attr.attribType(param, symbol);
if (testParamType instanceof JavaType.GenericTypeVariable) {
for (JavaType bound : ((JavaType.GenericTypeVariable) testParamType).getBounds()) {
List<JavaType> bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds();
if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) {
return method;
}
for (JavaType bound : bounds) {
if (paramTypeMatches(bound, paramType)) {
return method;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,11 @@ private JavaType.Method methodReferenceType(DCTree.DCReference ref, @Nullable Ja
for (JavaType testParamType : method.getParameterTypes()) {
Type paramType = attr.attribType(param, symbol);
if (testParamType instanceof JavaType.GenericTypeVariable) {
for (JavaType bound : ((JavaType.GenericTypeVariable) testParamType).getBounds()) {
List<JavaType> bounds = ((JavaType.GenericTypeVariable) testParamType).getBounds();
if (bounds.isEmpty() && paramType.tsym != null && "java.lang.Object".equals(paramType.tsym.getQualifiedName().toString())) {
return method;
}
for (JavaType bound : bounds) {
if (paramTypeMatches(bound, paramType)) {
return method;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.openrewrite.java.tree.JavaType.Parameterized;
import org.openrewrite.java.tree.TypeUtils;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.TypeValidation;

import java.util.concurrent.atomic.AtomicReference;

Expand Down Expand Up @@ -140,8 +141,10 @@ class TypeC<T extends String> extends java.util.ArrayList<T> {

@Issue("https://github.com/openrewrite/rewrite/issues/1762")
@Test
@MinimumJava11
void methodInvocationWithUnknownTypeSymbol() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().constructorInvocations(false).build()),
java(
"""
import java.util.ArrayList;
Expand Down Expand Up @@ -174,6 +177,7 @@ List<Parent> method(List<Parent> values) {
@Test
void methodInvocationOnUnknownType() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()),
java(
"""
import java.util.ArrayList;
Expand Down Expand Up @@ -237,7 +241,7 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations
import java.util.List;
import java.util.stream.Collectors;
@SuppressWarningsWarnings("ALL")
@SuppressWarnings("ALL")
class MakeEasyToFind {
void method(List<MultiMap> multiMaps) {
List<Integer> ints;
Expand Down Expand Up @@ -302,7 +306,7 @@ public J.Lambda visitLambda(J.Lambda lambda, ExecutionContext executionContext)
import java.util.List;
import java.util.stream.Collectors;
@SuppressWarningsWarnings("ALL")
@SuppressWarnings("ALL")
class MakeEasyToFind {
void method(List<MultiMap> multiMaps) {
Object obj = multiMaps.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@

@Retention(RUNTIME)
@Target({TYPE, METHOD})
@Tag("java17")
@Tag("java11")
public @interface MinimumJava11 {
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ void endOfLineBreaks() {
rewriteRun(
java(
"""
import java.util.Objects;
class Test {
void test() {
boolean b = Objects.equals(1, 2) //
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@ void statementTerminatorForSingleLineForLoops() {
java(
"""
class Test {
void test() {
void test(int[] n) {
for(Integer i : n) test();
}
void test() {
}
}
"""
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ void formatLoopNoInit() {
java(
"""
class Test {
void test() {
void test(int i) {
for ( ; i < 10 ; i++ ) {}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void instanceOf() {
java(
"""
class Test {
void test() {
void test(Object o) {
boolean b = o instanceof String;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@

import org.junit.jupiter.api.Test;
import org.openrewrite.Issue;
import org.openrewrite.java.MinimumJava11;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.TypeValidation;

import static org.openrewrite.java.Assertions.java;

Expand Down Expand Up @@ -389,6 +391,7 @@ class Test {
@Test
void exception() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()),
java(
"""
public class A {
Expand Down Expand Up @@ -721,6 +724,7 @@ List<String> method() throws Exception {
@Test
void multipleReferenceParameters() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()),
java(
"""
class Test {
Expand Down Expand Up @@ -1064,6 +1068,7 @@ interface Test {
@Test
void methodNotFound() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().methodInvocations(false).build()),
java(
"""
interface Test {
Expand All @@ -1082,6 +1087,7 @@ interface Test {
@Test
void typeNotFound() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()),
java(
"""
interface Test {
Expand Down Expand Up @@ -1650,6 +1656,7 @@ void arrayTypeLiterals() {

@Test
@Issue("https://github.com/openrewrite/rewrite/issues/3530")
@MinimumJava11
void arrayTypeLiterals2() {
rewriteRun(
java("" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void multipleParameters() {
rewriteRun(
java(
"""
import java.util.function.BiFunction;
import java.util.function.BiConsumer;
class Test {
void test() {
BiConsumer<String, String> a = (s1, s2) -> { };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.junit.jupiter.api.Test;
import org.openrewrite.Issue;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.TypeValidation;

import static org.openrewrite.java.Assertions.java;

Expand All @@ -28,6 +29,7 @@ class MemberReferenceTest implements RewriteTest {
@Test
void unknownDeclaringType() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).methodInvocations(false).build()),
java(
"""
package com.company.pkg;
Expand Down Expand Up @@ -130,11 +132,13 @@ static void func(String s) {}
@Test
void constructorMethodReference() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().methodInvocations(false).build()),
java(
"""
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Stream;
import java.util.Set;
import java.util.stream.Stream;
class Test {
Stream<Integer> n = Stream.of(1, 2);
Set<Integer> n2 = n.collect(HashSet < Integer > :: new, HashSet :: add);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ void anonymousClass() {
rewriteRun(
java(
"""
import java.util.ArrayList;
import java.util.List;
class Test {
List<Integer> l = new ArrayList<Integer>() {
/** Javadoc */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import org.junit.jupiter.api.Test;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.TypeValidation;

import static org.openrewrite.java.Assertions.java;

Expand All @@ -25,6 +26,7 @@ class TypeParameterAndWildcardTest implements RewriteTest {
@Test
void annotatedTypeParametersOnWildcardBounds() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()),
java(
"""
import java.util.List;
Expand All @@ -40,6 +42,7 @@ class A {
@Test
void annotatedTypeParametersOnReturnTypeExpression() {
rewriteRun(
spec -> spec.typeValidationOptions(TypeValidation.builder().identifiers(false).build()),
java(
"""
import java.util.List;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import org.junit.jupiter.api.Test;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.MinimumJava11;
import org.openrewrite.java.MinimumJava17;
import org.openrewrite.test.RewriteTest;

Expand Down Expand Up @@ -59,6 +60,7 @@ class Test {
}

@Test
@MinimumJava11
void finalVar() {
rewriteRun(
java(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -840,23 +840,23 @@ void dontAddImportForStaticImportsIndirectlyReferenced() {
spec -> spec.recipe(toRecipe(() -> new JavaIsoVisitor<>() {
@Override
public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) {
maybeAddImport("com.fasterxml.jackson.databind.ObjectMapper");
maybeAddImport("java.io.File");
return super.visitCompilationUnit(cu, ctx);
}
})).parser(JavaParser.fromJavaVersion().classpath("jackson-databind")),
})),
java(
"""
import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.File;
class Helper {
static ObjectMapper OBJECT_MAPPER;
static File FILE;
}
"""
),
java(
"""
class Test {
void test() {
Helper.OBJECT_MAPPER.writer();
Helper.FILE.exists();
}
}
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public J visitLiteral(J.Literal literal, ExecutionContext executionContext) {
}
}))
// custom missing type validation
.typeValidationOptions(TypeValidation.none())
.afterTypeValidationOptions(TypeValidation.none())
.afterRecipe(run -> run.getChangeset().getAllResults().forEach(r -> assertTypeAttribution((J) r.getAfter())));
}

Expand Down
Loading

0 comments on commit 0d884d4

Please sign in to comment.