groovy-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Milles (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (GROOVY-9058) each parameter type not correctly inferenced
Date Tue, 02 Apr 2019 22:54:00 GMT

    [ https://issues.apache.org/jira/browse/GROOVY-9058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16808150#comment-16808150
] 

Eric Milles edited comment on GROOVY-9058 at 4/2/19 10:53 PM:
--------------------------------------------------------------

If I rewrite the upper half of {{getResultType}} (everything before {{isBoolIntrinsicOp}}),
like this:
{code:java}
    protected ClassNode getResultType(ClassNode left, int op, ClassNode right, BinaryExpression
expr) {
        ClassNode leftRedirect = left.redirect();
        ClassNode rightRedirect = right.redirect();

        Expression leftExpression = expr.getLeftExpression();
        Expression rightExpression = expr.getRightExpression();

        if (op == ASSIGN || op == ASSIGNMENT_OPERATOR) {
            if (rightExpression instanceof ClosureExpression && rightRedirect.isDerivedFrom(CLOSURE_TYPE)
&& isSAMType(leftRedirect)) {
                return inferSAMTypeGenericsInAssignment(left, findSAM(left), right, (ClosureExpression)
rightExpression);
            }

            if (leftExpression instanceof VariableExpression) {
                ClassNode initialType = getOriginalDeclarationType(leftExpression).redirect();

                if (isPrimitiveType(right) && initialType.isDerivedFrom(Number_TYPE))
{
                    return getWrapper(right);
                }

                if (isPrimitiveType(initialType) && rightRedirect.isDerivedFrom(Number_TYPE))
{
                    return getUnwrapper(right);
                }

                /* GRECLIPSE edit
                // as anything can be assigned to a String, Class or [Bb]oolean, return the
left type instead
                if (STRING_TYPE.equals(initialType)
                        || CLASS_Type.equals(initialType)
                        || Boolean_TYPE.equals(initialType)
                        || boolean_TYPE.equals(initialType)) {
                    return initialType;
                }
                */
                if (!((VariableExpression) leftExpression).isDynamicTyped()) {
                    return initialType;
                }
            }

            if (rightRedirect.implementsInterface(Collection_TYPE)) {
                if (leftRedirect.isArray()) {
                    return leftRedirect;
                }
                if (leftRedirect.implementsInterface(Collection_TYPE) &&
                        rightExpression instanceof ListExpression && isEmptyCollection(rightExpression))
{
                    return left;
                }
            }

            return right;
        }
{code}

I think it does everything it did before, except assignments to non-dynamic variables will
return the variable type instead of the RHS type.  _Can someone try this out in groovy core
and run the tests?_


With these changes, code like this will use `Object[]` instead of `PortableProperties[]` for
the type of `array`.  So I would change `Object[]` to `def` in this case.
{code:groovy}
        Object[] array = properties.getProperty(VALUES_PROPERTY_NAME) as PortableProperties[]
        if (array)
        {
            values = array.collect { PortableProperties pp -> ValueComparison.fromPortableProperties(pp)
}
        }
{code}


And code like this will use `Multimap<A, B>` instead of `LinkedHashMultimap` for the
type of `facets`.  So I would add typecasts to `entry.key` and `entry.value.values()` since
I am no longer putting to a raw-typed multimap.
{code:groovy}
        Multimap<A, B> facets = LinkedHashMultimap.create()
        for (entry in appliedAttributes)
        {
            if (entry.key instanceof A)
            {
                facets.putAll(entry.key, entry.value.values())
            }
        }
{code}

Since these are breaking changes to compilation/type-checking, I would suggest changes to
Groovy 3 only.


was (Author: emilles):
If I rewrite the upper half of {{getResultType}} (everything before {{isBoolIntrinsicOp}}),
like this:
{code:java}
    protected ClassNode getResultType(ClassNode left, int op, ClassNode right, BinaryExpression
expr) {
        ClassNode leftRedirect = left.redirect();
        ClassNode rightRedirect = right.redirect();

        Expression leftExpression = expr.getLeftExpression();
        Expression rightExpression = expr.getRightExpression();

        if (op == ASSIGN || op == ASSIGNMENT_OPERATOR) {
            if (rightExpression instanceof ClosureExpression && rightRedirect.isDerivedFrom(CLOSURE_TYPE)
&& isSAMType(leftRedirect)) {
                return inferSAMTypeGenericsInAssignment(left, findSAM(left), right, (ClosureExpression)
rightExpression);
            }

            if (leftExpression instanceof VariableExpression) {
                ClassNode initialType = getOriginalDeclarationType(leftExpression).redirect();

                if (isPrimitiveType(right) && initialType.isDerivedFrom(Number_TYPE))
{
                    return getWrapper(right);
                }

                if (isPrimitiveType(initialType) && rightRedirect.isDerivedFrom(Number_TYPE))
{
                    return getUnwrapper(right);
                }

                if (!((VariableExpression) leftExpression).isDynamicTyped()) {
                    return initialType;
                }
            }

            if (rightRedirect.implementsInterface(Collection_TYPE)) {
                if (leftRedirect.isArray()) {
                    return leftRedirect;
                }
                if (leftRedirect.implementsInterface(Collection_TYPE) &&
                        rightExpression instanceof ListExpression && isEmptyCollection(rightExpression))
{
                    return left;
                }
            }

            return right;
        }
{code}

I think it does everything it did before, except assignments to non-dynamic variables will
return the variable type instead of the RHS type.  _Can someone try this out in groovy core
and run the tests?_


With these changes, code like this will use `Object[]` instead of `PortableProperties[]` for
the type of `array`.  So I would change `Object[]` to `def` in this case.
{code:groovy}
        Object[] array = properties.getProperty(VALUES_PROPERTY_NAME) as PortableProperties[]
        if (array)
        {
            values = array.collect { PortableProperties pp -> ValueComparison.fromPortableProperties(pp)
}
        }
{code}


And code like this will use `Multimap<A, B>` instead of `LinkedHashMultimap` for the
type of `facets`.  So I would add typecasts to `entry.key` and `entry.value.values()` since
I am no longer putting to a raw-typed multimap.
{code:groovy}
        Multimap<A, B> facets = LinkedHashMultimap.create()
        for (entry in appliedAttributes)
        {
            if (entry.key instanceof A)
            {
                facets.putAll(entry.key, entry.value.values())
            }
        }
{code}

Since these are breaking changes to compilation/type-checking, I would suggest changes to
Groovy 3 only.

> each parameter type not correctly inferenced
> --------------------------------------------
>
>                 Key: GROOVY-9058
>                 URL: https://issues.apache.org/jira/browse/GROOVY-9058
>             Project: Groovy
>          Issue Type: Bug
>          Components: Static compilation
>    Affects Versions: 2.5.6
>            Reporter: Mauro Molinari
>            Priority: Major
>
> Consider this Java class:
> {code:java}
> package test51;
> import java.util.List;
> public class Foo {
>     public List<Object[]> bar() { return null; }
> }{code}
>  and this Groovy class:
> {code:java}
> package test51
> import groovy.transform.CompileStatic
> @CompileStatic
> class Test51 {
>     protected void foo() {
>         List<Object[]> foo = new Foo().bar()
>         foo.each { row ->
>             def o = row[0]
>         }
>     }
>     
>     List bar() {
>     }
> }{code}
> This produces a compiler error because {{row}} is resolved as {{Object}} rather than
{{Object[]}}.
> A workaround is to declare {{row}} as {{Object[] row}} in the closure parameter list.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message