Skip to content
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.

Sign up for GitHub

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

Cannot close over local variable #924

Closed
wants to merge 3 commits into from
Closed

Cannot close over local variable #924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4270597
lookup concrete method parameter types instead of class type parameters
atorstling Oct 19, 2015
fc9f5fc
cleanup imports
atorstling Oct 19, 2015
076c6cf
remove wildcard imports
atorstling Oct 19, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions java/src/main/java/cucumber/runtime/java/Java8StepDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
import gherkin.formatter.model.Step;

import java.lang.reflect.Method;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;
Expand All @@ -39,21 +39,20 @@ public Java8StepDefinition(Pattern pattern, long timeoutMillis, StepdefBody body

Class<? extends StepdefBody> bodyClass = body.getClass();

Type genericInterface = bodyClass.getGenericInterfaces()[0];
Type[] argumentTypes;
if (genericInterface instanceof ParameterizedType) {
argumentTypes = ((ParameterizedType) genericInterface).getActualTypeArguments();
} else {
argumentTypes = typeIntrospector.getGenericTypes(bodyClass);
ArrayList<Method> acceptMethods = new ArrayList<Method>();
for (Method method : bodyClass.getDeclaredMethods()) {
if (!method.isBridge() && !method.isSynthetic() && "accept".equals(method.getName())) {
acceptMethods.add(method);
}
}
if (acceptMethods.size() != 1) {
throw new IllegalStateException(String.format("Expected single 'accept' method on body class, found " +
"'%s'", acceptMethods));
}
this.method = acceptMethods.get(0);
Type[] argumentTypes = method.getGenericParameterTypes();
verifyNotListOrMap(argumentTypes);
this.parameterInfos = ParameterInfo.fromTypes(argumentTypes);

Class[] parameterTypes = new Class[parameterInfos.size()];
for (int i = 0; i < parameterInfos.size(); i++) {
parameterTypes[i] = Object.class;
}
this.method = bodyClass.getDeclaredMethod("accept", parameterTypes);
}

private void verifyNotListOrMap(Type[] argumentTypes) {
Expand Down
7 changes: 6 additions & 1 deletion java8/src/test/java/cucumber/runtime/java8/test/LambdaStepdefs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package cucumber.runtime.java8.test;

import cucumber.api.DataTable;
import cucumber.api.PendingException;
import cucumber.api.Scenario;
import cucumber.api.java8.En;
import cucumber.api.java8.StepdefBody;

import java.util.List;

Expand All @@ -17,11 +19,14 @@ public LambdaStepdefs() {
assertNotSame(this, lastInstance);
lastInstance = this;
});

Given("^this data table:$", (DataTable peopleTable) -> {
List<Person> people = peopleTable.asList(Person.class);
assertEquals("Hellesøy", people.get(0).last);
});
Integer alreadyHadThisManyCukes = 1;
Given("^I have 42 cukes in my belly$", () -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like your change breaks arguments. Try changing this line to the following to reproduce:

Given("^I have (\\d+) cukes in my belly$", (Integer n) -> {

Any thoughts about how to deal with that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I didn't expect it to break that easily. What happens? I can have a look in a couple of days, but not right now unfortunately.

Copy link
Contributor

Choose a reason for hiding this comment

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

The lambda parameters are reported as java.lang.Object, so the type conversion breaks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh well, so much for that technique then ;) If its not caused by the interface methods being declared accepting Object, I have no immediate ideas for workarounds. Maybe merge the other fix and I'll see if there is a way around it at nearest occation. Should have tested it better :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's funny though - the documentation for getGenericParameterTypes says "If a formal parameter type is a parameterized type, the Type object returned for it must accurately reflect the actual type parameters used in the source code.".

assertEquals((Integer) 1, alreadyHadThisManyCukes);
});
}

public static class Person {
Expand Down