-
Notifications
You must be signed in to change notification settings - Fork 96
Add unit test showing null Methodinvocation type #870
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
src/test/java/org/openrewrite/java/testing/cleanup/KotlinTestMethodShouldBeUnitTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/cleanup/KotlinTestMethodShouldBeUnitTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/cleanup/KotlinTestMethodShouldBeUnitTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/cleanup/KotlinTestMethodShouldBeUnitTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/testing/cleanup/KotlinTestMethodShouldBeUnitTest.java
Outdated
Show resolved
Hide resolved
| @Test | ||
| void showNullMethodInvocationType() { | ||
| //language=kotlin | ||
| SourceSpec<?> source = getOnly( | ||
| kotlin( | ||
| """ | ||
| class ExampleTest { | ||
| fun myTest() = run { | ||
| return 1 | ||
| } | ||
| } | ||
| """ | ||
| )); | ||
| Parser parser = KotlinParser.builder().build(); | ||
| Path sourcePath = parser.sourcePathFromSourceText(source.getDir(), source.getBefore()); | ||
| ExecutionContext ctx = new InMemoryExecutionContext(); | ||
| Parser.Input input = Parser.Input.fromString( | ||
| sourcePath, source.getBefore(), parser.getCharset(ctx)); | ||
| SourceFile sourceFile = getOnly( | ||
| parser.parseInputs(singleton(input), null, ctx).toList()); | ||
| new FindMissingTypesVisitor().visit(sourceFile, ctx); | ||
| } | ||
|
|
||
| private static class FindMissingTypesVisitor extends KotlinIsoVisitor<ExecutionContext> { | ||
|
|
||
| @Override | ||
| public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, | ||
| ExecutionContext ctx) { | ||
| assertNotNull(method.getMethodType()); | ||
| return method; | ||
| } | ||
| } | ||
|
|
||
| private static <T> T getOnly(Iterable<T> iterable) { | ||
| Iterator<T> iter = iterable.iterator(); | ||
| T only = iter.next(); | ||
| assertFalse(iter.hasNext()); | ||
| return only; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @Test | |
| void showNullMethodInvocationType() { | |
| //language=kotlin | |
| SourceSpec<?> source = getOnly( | |
| kotlin( | |
| """ | |
| class ExampleTest { | |
| fun myTest() = run { | |
| return 1 | |
| } | |
| } | |
| """ | |
| )); | |
| Parser parser = KotlinParser.builder().build(); | |
| Path sourcePath = parser.sourcePathFromSourceText(source.getDir(), source.getBefore()); | |
| ExecutionContext ctx = new InMemoryExecutionContext(); | |
| Parser.Input input = Parser.Input.fromString( | |
| sourcePath, source.getBefore(), parser.getCharset(ctx)); | |
| SourceFile sourceFile = getOnly( | |
| parser.parseInputs(singleton(input), null, ctx).toList()); | |
| new FindMissingTypesVisitor().visit(sourceFile, ctx); | |
| } | |
| private static class FindMissingTypesVisitor extends KotlinIsoVisitor<ExecutionContext> { | |
| @Override | |
| public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, | |
| ExecutionContext ctx) { | |
| assertNotNull(method.getMethodType()); | |
| return method; | |
| } | |
| } | |
| private static <T> T getOnly(Iterable<T> iterable) { | |
| Iterator<T> iter = iterable.iterator(); | |
| T only = iter.next(); | |
| assertFalse(iter.hasNext()); | |
| return only; | |
| } | |
| @Override | |
| public void defaults(RecipeSpec spec) { | |
| spec | |
| .parser(KotlinParser.builder()); | |
| } | |
| @Test | |
| void showNullMethodInvocationType() { | |
| rewriteRun( | |
| //language=kotlin | |
| kotlin( | |
| """ | |
| class ExampleTest { | |
| fun myTest() = run { | |
| return 1 | |
| } | |
| } | |
| """ | |
| )); | |
| } |
I've simplified it to this and am indeed seeing it fail in rewrite-testing-frameworks but succeed in rewrite
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, that simple example will show the symptom, but I found the failure to be more cryptic. The intention of the PR was to reveal the underlying issue: that the type of the run method invocation is null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Laurens-W, there appears to be other code that "expects" the types of Kotlin method invocations to sometimes be null (example). If that is sometimes "expected" behavior, the following workaround might make sense: openrewrite/rewrite#6409
Thoughts?
What's changed?
What's your motivation?
Add unit test showing null Methodinvocation type. This test fails in this repo but the very same test passes when added to the
rewriterepo.Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
Any additional context
Checklist