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

Static Type Checking on Map Iteration Shows Incorrect Compile Errors #872

Closed
ssadedin opened this issue Apr 14, 2019 · 12 comments
Closed

Static Type Checking on Map Iteration Shows Incorrect Compile Errors #872

ssadedin opened this issue Apr 14, 2019 · 12 comments
Assignees
Labels
Milestone

Comments

@ssadedin
Copy link

ssadedin commented Apr 14, 2019

In the last few days my existing code is suddenly showing compile errors whenever I have enabled static compilation and a Map is being iterated. For example:

image

This code results in this error:

Expected parameter of type java.util.Map$Entry <java.lang.Object, java.lang.Object> but got java.util.Map$Entry <String, Object>

This is happening across many different instances of iterating over Maps, it is occurring with both the two argument closure and single argument closure. The same code compiles successfully when compiled with groovyc.

The only way I seem to be able to get things to work is to remove iteration using closures from the code.

I am running this version of groovy-eclipse:

  Eclipse Groovy Development Tools	3.4.0.v201904131343-e1903	org.codehaus.groovy.eclipse.feature.feature.group	Pivotal Software, Inc.

I am using the 2.4 groovy compiler.

@eric-milles
Copy link
Member

This has to do with #863 and the linked Groovy bugs. The way STC behaves in this situation is not very intuitive IMO due to flow typing.

In the statement Map<String,Object> props = (Map)ois.readObject(), Groovy actually uses the RHS type, which is Map<Object, Object>. This causes many developers to insert closure parameter types and typecasts, as you have done in the subsequent code.

For best compatibility with current and future STC and compilers/editors, I suggest writing your example like this:

def props = (Map<String, Object>) ois.readObject()
props.each { entry ->
  setProperty(entry.key, entry.value)
}

In this form, the type for props and entry is controlled by a single type choice.

@ssadedin
Copy link
Author

Thanks for the followup! I will read those links a bit more in detail, but just wanted to relate that I'm not sure this is correct behavior as even with your suggestions, I am still getting the error - here is a contrived example:

    @CompileStatic
    void compileBug() {
        Map m = [foo: 'cat', bar: 'hat']
        def x = (Map<String,String>)m
        x.each { e ->
            println(e.value)
        }
    }

In this case, the error is:

Groovy:[Static type checking] - Cannot call <K,V> java.util.Map <String, String>#each(groovy.lang.Closure) with arguments [groovy.lang.Closure] 	

As I mentioned before - this all used to compile fine in Eclipse until some recent update, and it still compiles fine with groovyc. I would also tentatively note it seems to work ok with the 2.5 compiler (unfortunately, I'm stuck using 2.4 for this project). Could it be a regression related to the changes to handle 2.4 in #863?

@ssadedin
Copy link
Author

Apologies, one more update: I found the following does work -

    @CompileStatic
    private void compileBug() {
        Map m = [foo: 'cat', bar: 'hat']
        def x = (Map<String,Object>)m // <== instead of Map<String,String>
        x.each { e ->
            println(e.value)
        }
    }

ie: the trouble seems to be coming from having anything other than Object as the second generic type parameter. The first type parameter seems to be correctly inferred and respected inside the closure.

@eric-milles
Copy link
Member

I am not seeing the original issue for the current 2.4. However I am seeing your second issue and will have a look. The message seems very strange; not sure exactly what it's describing.

image

@eric-milles
Copy link
Member

There is definitely something strange going on here because the inferred type of the entry parameter changes:

@CompileStatic
void meth() {
  def map = [foo: 'cat', bar: 'hat']
  map.each { entry -> // entry seen as Map<String,String>
    println entry.value // entry seen as Map<String,Object>
  }
}

@eric-milles
Copy link
Member

Compatibility check of the DGM is breaking down because typeof(map) is LinkedHashMap<String,String> and typeof(1st param of each) is Map<String, Object>.

@eric-milles
Copy link
Member

It appears the extension method cache is getting polluted somehow.

@eric-milles
Copy link
Member

eric-milles commented Apr 15, 2019

It looks like any DGM that uses @ClosureParams(MapEntryOrKeyValue.class) and possibly others are being modified in the extension method cache when the source is viewed in the editor or searched. I have a fix for this testing now.

@eric-milles
Copy link
Member

ready to test

@ssadedin
Copy link
Author

Fantastic - thank you for working on it!

Let me know if anything I can do to help / test / etc.

@eric-milles
Copy link
Member

Yes, please update to the latest snapshot and try out your source/project with errors.

@ssadedin
Copy link
Author

It works! All the errors have gone away. Thanks so much!

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