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

Compiler error on code that compiles with groovyc #863

Closed
mauromol opened this issue Mar 27, 2019 · 12 comments
Closed

Compiler error on code that compiles with groovyc #863

mauromol opened this issue Mar 27, 2019 · 12 comments
Assignees
Labels
Milestone

Comments

@mauromol
Copy link

Today I found GROOVY-9058, but I also found that the following variation compiles with groovyc but still generates an error with Greclipse:

Consider this Java class:

package test51;

import java.util.List;

public class Foo {
	public List bar() { return null; }
}

and this Groovy class:

package test51

import groovy.transform.CompileStatic

@CompileStatic
class Test51 {
	protected void foo() {
		List<Object[]> foo = new Foo().bar() // (A)
		foo.each { Object[] row -> 
			def o = row[0]  // (B)
		}
	}
	
	List bar() {
	}
}

No compiler error in (A), although, strictly speaking, it should. However groovyc does not complain either. I would assume foo to be inferred as List<Object[]>, given its declaration. Hovering the mouse pointer over row seems to confirm this and groovyc can compile this code. However, in Greclipse I get an error at (B), because row is inferred as Object even if foo is declared as List<Object[]> and row is declared as Object[].

@eric-milles
Copy link
Member

The reason for GROOVY-9058 is this check near the end of StaticTypeCheckingVisitor.doInferClosureParameterTypes:

                    boolean lastArg = i == length - 1;
                    if (lastArg && inferredType.isArray()) {
                        if (inferredType.getComponentType().equals(originType)) {
                            inferredType = originType;
                        }
                    }

STC has determined that the type of the closure parameter is Object[] however, lastArg is true and the array's component type matches the origin type of the parameter (Object since no explicit type was given in the source). So Object is saved instead of Object[].

@eric-milles
Copy link
Member

I'm trying to isolate why the editor is having one behavior for this and the builder is getting another result. Also, you may want to give some feedback on GROOVY-9058. From the looks of the code in Groovy core, STC ignores the declared type of variable declarations in a lot of cases. I'm not sure what the overall reasoning is for this, but we need to prompt some kind of explanation/response to help guide what kind of changes fit that design.

@eric-milles eric-milles self-assigned this Apr 2, 2019
@mauromol
Copy link
Author

mauromol commented Apr 3, 2019

What about asking on the Groovy dev mailing list? Probably you can do it better than I could, otherwise let me know and I will ask there to take a look at GROOVY-9058...

@eric-milles
Copy link
Member

I added notes to GROOVY-9058 indicating that the very first comment contains the one and only suggested change to fix closure param inferencing under STC. I also created GROOVY-9064 for the case you describe here. I have tried some different possibilities and think I have a small change that will address this issue.

You can lobby Groovy core to test and merge the changes if you wish. I will do some additional testing and commit the changes here if all goes well.

eric-milles added a commit that referenced this issue Apr 3, 2019
@eric-milles
Copy link
Member

Ready to test

@eric-milles eric-milles added this to the v3.4.0 milestone Apr 4, 2019
@mauromol
Copy link
Author

mauromol commented Apr 5, 2019

Hi Eric,
after all your investigations, it's not clear to me what I should test here. If I consider my report, I should expect Greclipse to behave like groovyc, hence not giving any error at (B) in this specific case. However, after updating to 3.4.0.v201904041901-e1812, I'm getting the exact same result, a compilation error at (B):

Groovy:[Static type checking] - Cannot find matching method java.lang.Object#getAt(int). Please check if the declared type is correct and if the method exists.

@eric-milles
Copy link
Member

eric-milles commented Apr 5, 2019

I made fixes in Groovy 2.5 and 3 -- I assumed you were using Groovy 2.5 from the bug metadata. The change was a little risky to patch Groovy 2.4 IMO. The outcome should be no compiler error for the case indicated here as well as in GROOVY-9058.

In general, under static type checking (STC) if you declare the type of a variable explicitly, that is what gets used as the inferred type. If you use def then you can get the old behavior. I haven't received much feedback on this, but I did find it more intuitive for the projects I tested against locally.

@mauromol
Copy link
Author

mauromol commented Apr 5, 2019

Ok, when I set Groovy 2.5 compiler and switch to Groovy 2.5 libraries for my project, it now works as groovyc. But the problem is still there when I use Groovy 2.4 (whose groovyc behaves correctly instead).

@eric-milles
Copy link
Member

I can try for a minimal fix for Groovy 2.4. I had mixed results with the compiler vs the editor without the additional changes.

@eric-milles
Copy link
Member

So I went back to your example verbatim and discovered why Groovyc and Greclipse have a different result. The return type of the method bar() is seen as java.util.List -> java.util.List<E> by Groovyc and simply java.util.List by Greclipse. This difference in the representation of the raw type prevents entering this block, where the type param E gets filled in by the receiver type List<Object[]>.

            if (lType.isUsingGenerics() && missesGenericsTypes(resultType) && isAssignment(op)) {
                // unchecked assignment
                // examples:
                // List<A> list = []
                // List<A> list = new LinkedList()
                // Iterable<A> list = new LinkedList()

                // in that case, the inferred type of the binary expression is the type of the RHS
                // "completed" with generics type information available in the LHS
                ClassNode completedType = GenericsUtils.parameterizeType(lType, resultType.getPlainNodeReference());

                resultType = completedType;
            }

@eric-milles
Copy link
Member

Ready to re-test

@mauromol
Copy link
Author

mauromol commented Apr 8, 2019

I verified it works with 3.4.0.v201904051949-e1812 and Groovy 2.4 compiler, thank you! 👍

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

No branches or pull requests

2 participants