Skip to content

[Java8] Add lambda parameter type annotations #1768

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

Merged
merged 42 commits into from
Oct 9, 2019

Conversation

rasklaad
Copy link

@rasklaad rasklaad commented Sep 10, 2019

Details

  • @DocStringType lambda implementation
  • @ParameterType lambda implementation
  • @DataTableType lambda inplementation
  • ParameterType with multiple capture groups
  • Javadocs

Motivation and Context

Closes: #1764

How Has This Been Tested?

  • docstring tests
  • parametertype tests
  • datatable type tests

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@rasklaad rasklaad changed the title WIP Lambda equivalent for @DocStringType annotation WIP Lambda equivalent for @ParameterType annotation Sep 10, 2019
@rasklaad
Copy link
Author

I am kinda stuck with DataTableType implementation, can't get lambda method return type, to register it with LambdaGlueRegistry.
There is a test which represent this feature:

Scenario: define data table type by lambda
    Given data table, defined with lambda
      |name  | surname       | famousBook          |
      |Fedor | Dostoevsky    |Crime and Punishment |

DataTableType((Map<String, String> entry) -> {
            return new Author(entry.get("name"), entry.get("surname"), entry.get("famousBook"));
        });

Given("data table, defined with lambda", (Author author) -> {
            assertThat(author.name, equalTo("Fedor"));
            assertThat(author.surname, equalTo("Dostoevsky"));
            assertThat(author.famousBook, equalTo("Crime and Punishment"));
        });

Also, I still don't know how to implement ParameterType with multiple capture groups, without creating a lot of functional interfaces with different amount of String arguments.
I mean something like

class ParameterTypeDefinitionBody1<T> {
    T accept(String arg) throws Throwable;
}
class ParameterTypeDefinitionBody2<T> {
    T accept(String arg1, String arg2) throws Throwable;
}
class ParameterTypeDefinitionBody3<T> {
    T accept(String arg1, String arg2, String arg3) throws Throwable;
}

etc

@mpkorstanje
Copy link
Contributor

can't get lambda method return type, to register it with LambdaGlueRegistry.

You can't get at the return type from Method.getGenericReturnType because DataTableDefinitionBody<T> is generic. So the class only knows that it has a generic T type variable.

However if you have an instance of that class you can get resolved types by using the type-tools dependency with TypeResolver.resolveRawArguments.resolveRawArguments(instance, instance.getClass()). You can find an example of this in Java8StepDefinition.

Internally it looks into the constant types pool which is a JVM implementation detail but currently also the only way to provide this functionality.

Also, I still don't know how to implement ParameterType with multiple capture groups, without creating a lot of functional interfaces with different amount of String arguments.

There is no other way in Java. Best thing you can do is provide up to 9 parameters as we've done in StepdefBody.

@mpkorstanje mpkorstanje mentioned this pull request Sep 22, 2019
6 tasks
mpkorstanje added a commit that referenced this pull request Sep 22, 2019
Introduces the backend module. With this module backend
implementations only need to use dependencies from
`io.cucumber.core.backend`. This will allow us to introduce the
module system later on 

The implementation is not yet perfect. Classpath scanning and
resource loading is located in other modules. Removing this
depends on #1526 and removal of type registry configurer which
depends on #1768 .

Fixes #1386 because steps can now see if an exception came from
user code or actual backend.
Removes timeout which would close #1695 earlier then expected.
Anton Deryabin and others added 2 commits September 23, 2019 19:05
lambda-type-annotations

Conflicts:
	java8/src/main/java/io/cucumber/java8/Java8Backend.java
	java8/src/main/java/io/cucumber/java8/LambdaGlue.java
	java8/src/test/java/io/cucumber/java8/Java8LambdaStepDefinitionMarksCorrectStackElementTest.java
	java8/src/test/java/io/cucumber/java8/LambdaGlueTest.java
@timtebeek
Copy link
Contributor

Work continued on #1782 ; Aside from merging master, here are my changes isolated from @rasklaad :
edb0b77...lambda-type-annotations-continued
So mostly changing Java8ParameterTypeDefinition to function as CaptureGroupTransformer for String[] arguments, to support the additional parameter types with up to 9 arguments. Also: JavaDocs + tests.

@coveralls
Copy link

coveralls commented Sep 28, 2019

Coverage Status

Coverage increased (+0.01%) to 87.177% when pulling 0dd6ffd on lambda-type-annotations into 669dd85 on master.

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cheers! I've merged both PR's so I can discuss them as a whole.

@timtebeek
Copy link
Contributor

Addressed a few of the comments already; don't think I'll find time over the next few days to fix the remaining issues.. Anyone welcome to pick those up.

@rasklaad rasklaad marked this pull request as ready for review October 9, 2019 09:12
@mpkorstanje mpkorstanje changed the title WIP Lambda equivalent for @ParameterType annotation [Java8] Add lambda parameter type annotations Oct 9, 2019
@mpkorstanje mpkorstanje merged commit f2354e8 into master Oct 9, 2019
@mpkorstanje mpkorstanje deleted the lambda-type-annotations branch October 9, 2019 19:24
@mpkorstanje
Copy link
Contributor

Whoo all done! Thanks @rasklaad and @timtebeek! Now I've got to update the release notes again! 😆

@mpkorstanje mpkorstanje added this to the 5.0.0 milestone Oct 9, 2019
@rasklaad
Copy link
Author

@timtebeek Thank you for the participation

@timtebeek
Copy link
Contributor

@rasklaad No problem at all; glad to help, and thanks for the final push to finish the work! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add lambda equivalent for @ParameterType annotation
4 participants