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.

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

Fix IllegalArgumentException on JDK 1.8.0_60 #914

Closed
wants to merge 6 commits into from
Closed

Fix IllegalArgumentException on JDK 1.8.0_60 #914

wants to merge 6 commits into from

Conversation

dbcowboy
Copy link
Contributor

To determine actual types of parameters for class passed in as lambda to
step definition, code uses old Sun ConstantPool to look for dynamic
linking data. Previously these values were found at index size()-2;
however as of 1.8.0_60 the location appears to have changed to size()-3.
Given that the following indexes appear to have empty/invalid values,
instead iterate downward from end to find first valid value and use for
parameter type determination.

To determine actual types of parameters for class passed in as lambda to
step definition, code uses old Sun ConstantPool to look for dynamic
linking data.  Previously these values were found at index size()-2;
however as of 1.8.0_60 the location appears to have changed to size()-3.
Given that the following indexes appear to have empty/invalid values,
instead iterate downward from end to find first valid value and use for
parameter type determination.
@aslakhellesoy
Copy link
Contributor

Thanks! Can you follow the code standard please?

@dbcowboy
Copy link
Contributor Author

dbcowboy commented Oct 2, 2015

Thanks for pointing me to the code standards; sorry I didn't have that in place before I checked in.

Let me know if I should re-submit this change with correct formatting.

@rnaufal
Copy link

rnaufal commented Oct 27, 2015

I've just downloaded the latest SNAPSHOT build (1.2.5-SNAPSHOT) and the this change isn't included on it yet. When are you planning to release a new snapshot build?

@aslakhellesoy
Copy link
Contributor

@rnaufal it will be in a snaphot jar shortly after this PR is merged.

@rnaufal
Copy link

rnaufal commented Oct 27, 2015

Thanks @aslakhellesoy. Does it will get merged soon?

@aslakhellesoy
Copy link
Contributor

@rnaufal yeah RSN

aslakhellesoy added a commit that referenced this pull request Nov 14, 2015
@rnaufal
Copy link

rnaufal commented Dec 9, 2015

Hi @aslakhellesoy, did you merge this PR? Thanks in advance!

@aslakhellesoy
Copy link
Contributor

@rnaufal the PR is open, so no

for (int i = size - 1; i > -1; i--) {
try {
memberRef = constantPool.getMemberRefInfoAt(i);
break;
Copy link

Choose a reason for hiding this comment

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

Return should be here (instead of break).

@erran-r7
Copy link

A colleague @kprzerwa-r7 and I encountered this without being sure whether it was ever released. For the next person who stumbles across this issue it was fixed in 96d1e85 (which was released in 1.2.5).

[Java8] Fix IllegalArgumentException on JDK 1.8.0_60 (#912, #914 Michael Wilkerson)

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
@lock
Copy link

lock bot commented Oct 25, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants