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

java8: Create compilable test inputs with all possible cases for method references #2968

Closed
sabaka opened this issue Feb 24, 2016 · 15 comments
Closed

Comments

@sabaka
Copy link
Contributor

sabaka commented Feb 24, 2016

We need set of inputs to test our grammar changes for method references. They should cover all possible cases of using method references.
All inputs should be compilable with java 8.

You may find this link helpful: https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-MethodReference
You should "extract" grammar rules to possible cases in code.
According to this specification, method reference are referenced only from primary expression https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-PrimaryNoNewArray
It should help you to put method reference in right places

some initial commit: sabaka@45b0deb

@romani
Copy link
Member

romani commented Feb 24, 2016

@rnveach , can you help us with it ?

@rnveach
Copy link
Member

rnveach commented Feb 25, 2016

looking over this file as a guide while doing my search: https://github.com/sabaka/checkstyle/blob/e3964e7a8531c5660b630a4bbc41066acfb2cd96/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/grammars/java8/InputMethodReferences4.java

I have examples of foo::<bar>foobar;, foo::<fooo,barr>bar;, foo::<foo<bar>>foobar;, foo::<foo<bar<?>>>foobar, and foo::<@fooo Object>bar; from JDK9.
Should I make a PR for the ones I find?

foo::bar, foo::new, foo::<bar>new, foo[]::new have examples already in CS.

Eclipse gives me Wildcard is not allowed at this location for foo::<?>bar; and foo::<?>new;, but maybe Travis will accept them.

@sabaka
Copy link
Contributor Author

sabaka commented Feb 25, 2016

@rnveach , I didn't try to compile examples from this file. I only wrote them according to grammar.
The main point is javac should be able to compile input.

@rnveach
Copy link
Member

rnveach commented Feb 25, 2016

@sabaka Yes, what I was talking about was me playing with the same formats in the compiler.

Here are my current examples that compile in Eclipse: https://github.com/rnveach/checkstyle/blob/b7c8f04e6d96492f95b5e411d760e5d58956e46f/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/grammars/java8/InputMethodReferences4.java#L41

@romani
Copy link
Member

romani commented Feb 25, 2016

that compile in Eclipse

just a reminder: to recheck "javac" compilation of code. Eclipse has its own java compiler, mismatch on conner/complicated cases could happen. We need to support code that compilable by javac.

@romani
Copy link
Member

romani commented Mar 9, 2016

@rnveach , did you finish this issue ? do you have problems with smth? do you need help ?

@romani romani changed the title Create compilable test inputs with all possible cases for method references java8: Create compilable test inputs with all possible cases for method references Mar 11, 2016
rnveach added a commit to rnveach/checkstyle that referenced this issue Mar 21, 2016
rnveach added a commit to rnveach/checkstyle that referenced this issue Mar 21, 2016
rnveach added a commit to rnveach/checkstyle that referenced this issue Mar 21, 2016
@rnveach
Copy link
Member

rnveach commented Mar 21, 2016

Sorry for the delay.

Travis did not accept foo::<?>bar; and foo::<?>new;.
https://travis-ci.org/rnveach/checkstyle/builds/117596163#L151

This means it won't also accept foo::<? extends bar>foobar, foo::<? super bar>foobar, foo::<@fooo?>bar, foo::<@fooo? extends barr>bar, or foo::<@fooo? extends @fooo barr>bar because it doesn't allow ?s in the bracket.

@sabaka
Copy link
Contributor Author

sabaka commented Mar 22, 2016

It means, that jls has ambigous grammar and it's good for us - we have to
support less cases.

On Mon, Mar 21, 2016 at 11:46 PM rnveach notifications@github.com wrote:

Sorry for the delay.

Travis did not accept foo::bar; and foo::new;.
https://travis-ci.org/rnveach/checkstyle/builds/117596163#L151

This means it won't also accept foo::foobar, foo::foobar, foo::<@fooo?>bar, foo::<@fooo? extends barr>bar, or foo::<@fooo?
extends @fooo barr>bar because it doesn't allow ?s in the bracket.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#2968 (comment)

@romani
Copy link
Member

romani commented Mar 22, 2016

@rnveach , if that is possible in JDK grammar, most likely that is non developed feature yet, or ....

My recommendation is to keep such cases as commented if that is possible by JDK grammar but not by javac for now. It might be reasonable to try openjdk9 javac to compile that stuff.

If it is not referenced in grammar of JDK specification - just remove it.

@rnveach
Copy link
Member

rnveach commented Mar 23, 2016

@romani

if that is possible in JDK grammar, most likely that is non developed feature yet

It may be more something we are missing from the JLS.
I notice after the JLS defines the grammar, it then has text that mentions certain scenarios that will be acceptable to the grammar but still produce compile errors.

I think this is close to the section in question for the ones we are failing to compile:
JLS8 15.13 page 537: If a method reference expression has the form ArrayType :: new, then ArrayType must denote a type that is reifiable (§4.7), or a compile-time error occurs

Even though we are not working with arrays, the key area is must denote a type that is reifiable.

reifiable: a type whose type information is fully available at runtime.
**This includes primitives, non-generic types, raw types, and invocations
of unbound wildcards.**

I think <?> isn't reifiable for new, just like you can't create a object with it.
Example: new ArrayList<?>(); generates the error Cannot instantiate the type ArrayList<?> but is fine with new ArrayList<Object>();

Maybe @sabaka can confirm?
Otherwise I will just add the comments.

@rnveach
Copy link
Member

rnveach commented Nov 2, 2016

@romani PR was merged, questions have gone unanswered. Is there anything more we have to do here?

@romani
Copy link
Member

romani commented Nov 2, 2016

@rnveach , lets check this on openjdk9 , if such cases are not compilable by javac we could skip support of them and close the issue.

@rnveach
Copy link
Member

rnveach commented Nov 2, 2016

All compilable sources that we don't support from openjdk9 were identified in #3033 and have issues created to add their support. The failures were annotation locations, not method references.
I will still check if there are any new method references in openjdk9, but if there are, we already support them and having them missing in our unit tests would be odd.

@romani
Copy link
Member

romani commented Nov 3, 2016

@rnveach, Just launch javac of java9 on examples that you are mentioned as possible. If not compilable - close the issue.

@rnveach
Copy link
Member

rnveach commented Nov 24, 2016

@romani The examples and issues created inside the issue #3033 are compilable. They were tested at the time of their creation.
I am closing this issue as I don't feel there is anything left to do.

@rnveach rnveach closed this as completed Nov 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants