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

Fixed MethodCallExprContext generic parameter issue. [NullType must not fail matchTypeParameters] #2939

Merged
merged 6 commits into from
Nov 29, 2020

Conversation

zcbbpo
Copy link

@zcbbpo zcbbpo commented Nov 23, 2020

Hi ,

It's seem paser generic parameter will throws java.lang.UnsupportedOperationException: com.github.javaparser.symbolsolver.model.typesystem.NullType

code:

public class ISSUES_Generic_Parameter {

    public static Object test() {
        return Foo.error("code", null, "arg1");
    }

    public static class Foo<T> {
        public T data;
        public static <T> Foo<T> error(String resultCode, T data, Object... params) {
            return null;
        }
    }

}

I will modified com.github.javaparser.symbolsolver.javaparsermodel.contexts.MethodCallExprContext#matchTypeParameters

if (type.isNull()) {
    type = MyObjectProvider.INSTANCE.object();
}

And add com.github.javaparser.symbolsolver.resolution.javaparser.contexts.MethodCallExprContextResolutionTest#testGenericParameter for test.

Please check it.

@jlerbsc
Copy link
Collaborator

jlerbsc commented Nov 25, 2020

I don't understand what this PR is trying to solve. The test case below does not throw an exception. So what are you doing to meet your problem?

void test() {
    String code = 
            "public class ISSUES_Generic_Parameter {\n" + 
            "\n" + 
            "    public static Object test() {\n" + 
            "        return Foo.error(\"code\", null, \"arg1\");\n" + 
            "    }\n" + 
            "\n" + 
            "    public static class Foo<T> {\n" + 
            "        public T data;\n" + 
            "        public static <T> Foo<T> error(String resultCode, T data, Object... params) {\n" + 
            "            return null;\n" + 
            "        }\n" + 
            "    }\n" + 
            "\n" + 
            "}";
    
    ParserConfiguration config = new ParserConfiguration();
    config.setSymbolResolver(new JavaSymbolSolver(new ReflectionTypeSolver(false)));
    StaticJavaParser.setConfiguration(config);

    CompilationUnit cu = StaticJavaParser.parse(code);
    Set<MethodCallExpr> methodCallExpr = new HashSet<>(cu.findAll(MethodCallExpr.class));
    for (MethodCallExpr expr : methodCallExpr) {
        ResolvedMethodDeclaration rd = expr.resolve();
        System.out.println("\t Solved : " + rd.getQualifiedSignature());
    }
}

result is

Solved : ISSUES_Generic_Parameter.Foo.error(java.lang.String, T, java.lang.Object...)

@zcbbpo
Copy link
Author

zcbbpo commented Nov 26, 2020

Hi @jlerbsc
Please try this:

@Test
    public void test() throws Exception {
        String code =
                "public class ISSUES_Generic_Parameter {\n" +
                        "\n" +
                        "    public static Object test() {\n" +
                        "        return Foo.error(\"code\", null, \"arg1\");\n" +
                        "    }\n" +
                        "\n" +
                        "    public static class Foo<T> {\n" +
                        "        public T data;\n" +
                        "        public static <T> Foo<T> error(String resultCode, T data, Object... params) {\n" +
                        "            return null;\n" +
                        "        }\n" +
                        "    }\n" +
                        "\n" +
                        "}";

        ParserConfiguration config = new ParserConfiguration();
        config.setSymbolResolver(new JavaSymbolSolver(new ReflectionTypeSolver(false)));
        StaticJavaParser.setConfiguration(config);

        CompilationUnit cu = StaticJavaParser.parse(code);
        cu.findAll(ReturnStmt.class).forEach(returnStmt -> {
            returnStmt.getExpression().ifPresent(expression -> {
                ResolvedType resolvedType = expression.calculateResolvedType();
                System.out.println(expression);
                System.out.println(resolvedType);
            });
        });

    }

@jlerbsc
Copy link
Collaborator

jlerbsc commented Nov 26, 2020

Hi @MysterAitch I think it's a correct patch in my opinion it can be merged. In this case null type could be assigned to Object type. JLS 5.2 assignment contexts says "a value of the null type (the null reference is the only such value) may be assigned to any reference type, resulting in a null reference of that type."
Perhaps the PR can be modified to remove the changes that are not needed and the test case can be better integrated with existing tests or it would be necessary to create a specific one for this issue.

@MysterAitch MysterAitch added this to the next release milestone Nov 29, 2020
@MysterAitch
Copy link
Member

@zcbbpo -- thank you for the PR!

Hi @MysterAitch I think it's a correct patch in my opinion it can be merged.

I agree! :)

In this case null type could be assigned to Object type. JLS 5.2 assignment contexts says "a value of the null type (the null reference is the only such value) may be assigned to any reference type, resulting in a null reference of that type."

Good spot! 👍

I've just added a comment to directly cross-reference to the JLS.

Perhaps the PR can be modified to remove the changes that are not needed

@jlerbsc what did you have in mind? I've no objections to merging 👍

@jlerbsc
Copy link
Collaborator

jlerbsc commented Nov 29, 2020

@jlerbsc what did you have in mind?

Nothing too bad there was just a commit that did not do what we expected from the patch. commit 3c5ba3c on MethodCallExprContext which is corrected in commit b4e3731

@MysterAitch
Copy link
Member

Perhaps [...] the test case can be better integrated with existing tests or it would be necessary to create a specific one for this issue.

I'll merge this now and it can be improved upon separately

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

Successfully merging this pull request may close these issues.

None yet

3 participants