You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by avafanasiev <gi...@git.apache.org> on 2017/12/11 13:05:38 UTC

[GitHub] groovy pull request #643: GROOVY-8241 SAM parameter type inference for expli...

GitHub user avafanasiev opened a pull request:

    https://github.com/apache/groovy/pull/643

    GROOVY-8241 SAM parameter type inference for explicit parameter

    It should fix GROOVY-8241, GROOVY-7061, GROOVY-8317.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/avafanasiev/groovy master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/groovy/pull/643.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #643
    
----
commit dcbc41e591a65895e4a4f47eca78488d8ea0603b
Author: alexey.afanasiev <al...@jetbrains.com>
Date:   2017-12-11T13:01:02Z

    GROOVY-8241 SAM parameter type inference for explicit parameter

----


---

[GitHub] groovy pull request #643: GROOVY-8241 SAM parameter type inference for expli...

Posted by daniel-huss <gi...@git.apache.org>.
Github user daniel-huss commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/643#discussion_r157355618
  
    --- Diff: src/test/groovy/transform/stc/MethodCallsSTCTest.groovy ---
    @@ -383,6 +406,94 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
                 ''', 'Reference to method is ambiguous'
         }
     
    +    void testShouldFailWithMultiplePossibleMethods2() {
    +        shouldFailWithMessages '''
    +                static String foo(String s) {
    +                    'String'
    +                }
    +                static String foo(Integer s) {
    +                    'Integer'
    +                }
    +                static String foo(Boolean s) {
    +                    'Boolean'
    +                }
    +                ['foo',123,true].each { argument ->
    +                    if (argument instanceof String || argument instanceof Boolean || argument instanceof Integer) {
    +                        foo(argument)
    +                    }
    +                }
    +            ''', 'Reference to method is ambiguous'
    +    }
    +
    +    void testInstanceOfOnExplicitParameter() {
    +        assertScript '''
    +                1.with { obj ->
    +                    if (obj instanceof String) {
    +                        obj.toUpperCase() 
    +                    }
    +                }
    +            '''
    +    }
    +
    +    void testSAMWithExplicitParameter() {
    +        assertScript '''
    +            public interface SAM {
    +                boolean run(String var1, Thread th);
    +            }
    +            
    +            static boolean foo(SAM sam) {
    +               sam.run("foo",  new Thread())
    +            }
    +            
    +            static def callSAM() {
    +                foo { str, th ->
    +                    str.toUpperCase().equals(th.getName())
    +                }
    +            }
    +            '''
    +    }
    +
    +    void testGroovy8241() {
    +        assertScript '''
    +            import java.util.function.Predicate
    +            
    +            static boolean foo(Predicate<? super String> p) {
    +                p.test("foo")
    +            }
    +            
    +            static def testPredicate() {
    +                foo { it ->
    +                    it.toUpperCase()
    --- End diff --
    
    I'm confused :) This works because `foo` happens to always pass a String to `p::test`, but what if it passed `new Object()` instead? If testPredicate were  `@CompileStatic` I would expect a compile error here. Is this disparity intentional?


---

[GitHub] groovy pull request #643: GROOVY-8241 SAM parameter type inference for expli...

Posted by daniel-huss <gi...@git.apache.org>.
Github user daniel-huss commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/643#discussion_r168272512
  
    --- Diff: src/test/groovy/transform/stc/MethodCallsSTCTest.groovy ---
    @@ -383,6 +406,94 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
                 ''', 'Reference to method is ambiguous'
         }
     
    +    void testShouldFailWithMultiplePossibleMethods2() {
    +        shouldFailWithMessages '''
    +                static String foo(String s) {
    +                    'String'
    +                }
    +                static String foo(Integer s) {
    +                    'Integer'
    +                }
    +                static String foo(Boolean s) {
    +                    'Boolean'
    +                }
    +                ['foo',123,true].each { argument ->
    +                    if (argument instanceof String || argument instanceof Boolean || argument instanceof Integer) {
    +                        foo(argument)
    +                    }
    +                }
    +            ''', 'Reference to method is ambiguous'
    +    }
    +
    +    void testInstanceOfOnExplicitParameter() {
    +        assertScript '''
    +                1.with { obj ->
    +                    if (obj instanceof String) {
    +                        obj.toUpperCase() 
    +                    }
    +                }
    +            '''
    +    }
    +
    +    void testSAMWithExplicitParameter() {
    +        assertScript '''
    +            public interface SAM {
    +                boolean run(String var1, Thread th);
    +            }
    +            
    +            static boolean foo(SAM sam) {
    +               sam.run("foo",  new Thread())
    +            }
    +            
    +            static def callSAM() {
    +                foo { str, th ->
    +                    str.toUpperCase().equals(th.getName())
    +                }
    +            }
    +            '''
    +    }
    +
    +    void testGroovy8241() {
    +        assertScript '''
    +            import java.util.function.Predicate
    +            
    +            static boolean foo(Predicate<? super String> p) {
    +                p.test("foo")
    +            }
    +            
    +            static def testPredicate() {
    +                foo { it ->
    +                    it.toUpperCase()
    --- End diff --
    
    Oh, you're right. I forgot Groovy's interpretation of <? super X> is simply <X> :)


---

[GitHub] groovy pull request #643: GROOVY-8241 SAM parameter type inference for expli...

Posted by avafanasiev <gi...@git.apache.org>.
Github user avafanasiev commented on a diff in the pull request:

    https://github.com/apache/groovy/pull/643#discussion_r157445258
  
    --- Diff: src/test/groovy/transform/stc/MethodCallsSTCTest.groovy ---
    @@ -383,6 +406,94 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
                 ''', 'Reference to method is ambiguous'
         }
     
    +    void testShouldFailWithMultiplePossibleMethods2() {
    +        shouldFailWithMessages '''
    +                static String foo(String s) {
    +                    'String'
    +                }
    +                static String foo(Integer s) {
    +                    'Integer'
    +                }
    +                static String foo(Boolean s) {
    +                    'Boolean'
    +                }
    +                ['foo',123,true].each { argument ->
    +                    if (argument instanceof String || argument instanceof Boolean || argument instanceof Integer) {
    +                        foo(argument)
    +                    }
    +                }
    +            ''', 'Reference to method is ambiguous'
    +    }
    +
    +    void testInstanceOfOnExplicitParameter() {
    +        assertScript '''
    +                1.with { obj ->
    +                    if (obj instanceof String) {
    +                        obj.toUpperCase() 
    +                    }
    +                }
    +            '''
    +    }
    +
    +    void testSAMWithExplicitParameter() {
    +        assertScript '''
    +            public interface SAM {
    +                boolean run(String var1, Thread th);
    +            }
    +            
    +            static boolean foo(SAM sam) {
    +               sam.run("foo",  new Thread())
    +            }
    +            
    +            static def callSAM() {
    +                foo { str, th ->
    +                    str.toUpperCase().equals(th.getName())
    +                }
    +            }
    +            '''
    +    }
    +
    +    void testGroovy8241() {
    +        assertScript '''
    +            import java.util.function.Predicate
    +            
    +            static boolean foo(Predicate<? super String> p) {
    +                p.test("foo")
    +            }
    +            
    +            static def testPredicate() {
    +                foo { it ->
    +                    it.toUpperCase()
    --- End diff --
    
    Actually you can't pass anything  except String to Predicate<? super String> p . Cause it should be able to be assigned to captured type '? super String' . This behavior matches java's lambda behavior.


---

[GitHub] groovy pull request #643: GROOVY-8241 SAM parameter type inference for expli...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/groovy/pull/643


---