[Java8, Kotlin Java8] Support java 8 method references #1140
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mpkorstanje
changed the title
mpkorstanje
changed the title
The implementation of t he `ConstantPoolTypeIntrospector` supported: - Reference to a static method - Reference to an instance method of a particular object - Reference to a constructor But did not support: - Reference to an instance method of an arbitrary object of a particular type Referencing instances methods would result in an index out of bounds exception as the step definition interface was expecting exactly 1 more argument then was bound to the body. We now use this discrepancy to detect if this is an instance method reference and include the the instance class as the first type. Related issues: - #1123 - #1126 This fixes #1126
mpkorstanje
force-pushed
the
feature/support-java-8-method-references
branch
7 times, most recently
from
168bdb3
to
762cafb
Compare
The implementation of t he `ConstantPoolTypeIntrospector` did not fully
support closures.
Using a closure without scoped variables would result
in a null pointer exception. E.g:
```java
Given("^A statement with a body expression$") { assertTrue(true) }
```
This is resolved check if the member reference method equals the magic
constant "INSTANCE" and return no type arguments.
Related issues:
- #1123
- #1126
This fixes #1123
mpkorstanje
force-pushed
the
feature/support-java-8-method-references
branch
from
762cafb
to
cd515d3
Compare
6 tasks
mpkorstanje
added a commit
that referenced
this pull request
Jul 14, 2017
To determine which argument types a lambda function requires we created a ConstantPoolTypeIntrospector. However it has never functioned quite correctly (#937, #1140, #957), was prone to breaking (#912, #914) and hasn't been tested much (#1048). It is important to understand that while we will get a properly functioning and tested replacement, TypeResolver uses the same ConstantPool and thus has the same potential to break. However because TypeResolver is used by a much larger audience I expect these problems to be shallow. Because this change the interface of Java8StepDefinition it made sense to refactor all the Java8 related stuff out of cucumber-java. This will make it easier in the future to add things like KotlinStepDefintions without creating a separate KotlinBackend. Related issues: - #912 - #914 - #937 - #957 - #1140 - #1048 - #1140 -
mpkorstanje
added a commit
that referenced
this pull request
Jul 14, 2017
To determine which argument types a lambda function requires we created a ConstantPoolTypeIntrospector. However it has never functioned quite correctly (#937, #1140, #957), was prone to breaking (#912, #914) and hasn't been tested much (#1048). It is important to understand that while we will get a properly functioning and tested replacement, TypeResolver uses the same ConstantPool and thus has the same potential to break. However because TypeResolver is used by a much larger audience I expect these problems to be shallow. Because this change the interface of Java8StepDefinition it made sense to refactor all the Java8 related stuff out of cucumber-java. This will make it easier in the future to add things like KotlinStepDefintions without creating a separate KotlinBackend. Related issues: - #912 - #914 - #937 - #957 - #1140 - #1048 - #1140
mpkorstanje
added a commit
that referenced
this pull request
Jul 14, 2017
To determine which argument types a lambda function requires we created a ConstantPoolTypeIntrospector. However it has never functioned quite correctly (#937, #1140, #957), was prone to breaking (#912, #914) and hasn't been tested much (#1048). It is important to understand that while we will get a properly functioning and tested replacement, TypeResolver uses the same ConstantPool and thus has the same potential to break. However because TypeResolver is used by a much larger audience I expect these problems to be shallow. Because this change the interface of Java8StepDefinition it made sense to refactor all the Java8 related stuff out of cucumber-java. This will make it easier in the future to add things like KotlinStepDefintions without creating a separate KotlinBackend. Related issues: - #912 - #914 - #937 - #957 - #1140 - #1048 - #1140
mpkorstanje
added a commit
that referenced
this pull request
Jul 14, 2017
To determine which argument types a lambda function requires we created a ConstantPoolTypeIntrospector. However it has never functioned quite correctly (#937, #1140, #957), was prone to breaking (#912, #914) and hasn't been tested much (#1048). It is important to understand that while we will get a properly functioning and tested replacement, TypeResolver uses the same ConstantPool and thus has the same potential to break. However because TypeResolver is used by a much larger audience I expect these problems to be shallow. Because this change the interface of Java8StepDefinition it made sense to refactor all the Java8 related stuff out of cucumber-java. This will make it easier in the future to add things like KotlinStepDefintions without creating a separate KotlinBackend. Related issues: - #912 - #914 - #937 - #957 - #1140 - #1048 - #1140 Closes #937
mpkorstanje
added a commit
that referenced
this pull request
Jul 15, 2017
To determine which argument types a lambda function requires we created a ConstantPoolTypeIntrospector. However it has never functioned quite correctly (#937, #1140, #957), was prone to breaking (#912, #914) and hasn't been tested much (#1048). It is important to understand that while we will get a properly functioning and tested replacement, TypeResolver uses the same ConstantPool and thus has the same potential to break. However because TypeResolver is used by a much larger audience I expect these problems to be shallow. Because this change the interface of Java8StepDefinition it made sense to refactor all the Java8 related stuff out of cucumber-java. This will make it easier in the future to add things like KotlinStepDefintions without creating a separate KotlinBackend. Related issues: - #912 - #914 - #937 - #957 - #1140 - #1048 - #1140 Closes #937 Closes #1048
mpkorstanje
added a commit
that referenced
this pull request
Jul 23, 2017
To determine which argument types a lambda function requires we created a ConstantPoolTypeIntrospector. However it has never functioned quite correctly (#937, #1140, #957), was prone to breaking (#912, #914) and hasn't been tested much (#1048). It is important to understand that while we will get a properly functioning and tested replacement, TypeResolver uses the same ConstantPool and thus has the same potential to break. However because TypeResolver is used by a much larger audience I expect these problems to be shallow. Because this change the interface of Java8StepDefinition it made sense to refactor all the Java8 related stuff out of cucumber-java. This will make it easier in the future to add things like KotlinStepDefintions without creating a separate KotlinBackend. Related issues: - #912 - #914 - #937 - #957 - #1140 - #1048 - #1140 Closes #937 Closes #1048
mpkorstanje
added a commit
that referenced
this pull request
Jul 29, 2017
To determine which argument types a lambda function requires we created a ConstantPoolTypeIntrospector. However it has never functioned quite correctly (#937, #1140, #957), was prone to breaking (#912, #914) and hasn't been tested much (#1048). It is important to understand that while we will get a properly functioning and tested replacement, TypeResolver uses the same ConstantPool and thus has the same potential to break. However because TypeResolver is used by a much larger audience I expect these problems to be shallow. Because this change the interface of Java8StepDefinition it made sense to refactor all the Java8 related stuff out of cucumber-java. This will make it easier in the future to add things like KotlinStepDefintions without creating a separate KotlinBackend. Related issues: - #912 - #914 - #937 - #957 - #1140 - #1048 - #1140 Closes #937 Closes #1048
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The ConstantPoolTypeIntrospector did not know how to handle references to an instance method of an arbitrary object of a particular type and Kotlin lambda expression without arguments or closure variables. This lead to issues:
Details
Updated the ConstantPoolTypeIntrospector to check for instance methods by counting the parameters expected by the interface and those bound to the anonymous class. Only iff this is an instance method the result will come up one short.
Resolved Kotlin specific problems by checking if the reference method equals the magic constant "INSTANCE".
How Has This Been Tested?
Extended examples in features to include kotlin and all 4 types of method references.
Still awaiting confirmation from:
Types of changes
Checklist: