Community
Participate
Working Groups
The "add return type" always adds Object or void as return types. It could certainely be made smarter about the possible return type. E.g. correctThis() { return 1; // or return "asdf"; } when I press Ctrl-1 on "correctThis", it offers to change the return type to "Object". This could be improved.
I've tried to fix this by going through the AST and finding ReturnStatements, but I always get a NULL type binding. I've traced the NULL value to this line in DefaultBindingResolver.resolveExpressionType: if (this.checkModificationCount && this.modificationCount != expression.getAST ().modificationCount()) { return null; } What is this? Why can I do to get a type binding for my return statement? Using ASTResolving.getTypeBinding gives the same result.
I think I tried that too (see the code for addMissingReturnTypeProposals). Type bindings are only guaranteed to be there is the code is errorfree. Sometimes, errors are gracefully ignored, but, as it seems not in this case. I think I filed a bug report to the compiler team (jcore) to ask if they could ignore a missing return type. I have to check that.
However, looking at the code you tracked down to be the reason for the NULL value seems strange. We don't modify the AST, or did you?
Nope, as far as I can tell, the AST isn't modified. The only thing that looks "unusual" about this, is how the AST is visited to find the return statements. That and the fact that the method doesn't have a return type obviously. Maybe that's enough to throw off the type binding resolver, but I don't get the checkModifications stuff. Could you ask someone from jdt.core? I'll try to investigate some more on my side.
Moving to jcore for comments.
The reason for this check "this.checkModificationCount && this.modificationCount != expression.getAST().modificationCount())" is that for now (we might try to change this) we consider that as soon as a tree is modified its binding are irrevelant. Take an example: You have the resulting tree for this code: public class A { int foo() { return 2; } } Now you change the type of the method declaration to be Object. If we allow you to get the bindings, you will be a binding with a return type set to int and not Object, because the bindings are linked to the original source. Then is this binding useful for you? What information can you consider as reliable in your method binding? It is not trivial to retrieve bindings when the tree has changed. We have a open discussion in bug 23162. Feel free to add yourself to CC or add your own comments.
I think there is a problem in the quick fix somewhere. I took this example: public class A { correctThis() { return 1; } } In my test I have: ASTNode node = getASTNode((CompilationUnit) result, 0, 0); assertNotNull(node); assertTrue("Not a method declaration", node.getNodeType() == ASTNode.METHOD_DECLARATION); MethodDeclaration methodDeclaration = (MethodDeclaration) node; Block block = methodDeclaration.getBody(); List statements = block.statements(); assertEquals("wrong size", 1, statements.size()); Statement statement = (Statement) statements.get(0); assertTrue("Not a return statement", statement.getNodeType() == ASTNode.RETURN_STATEMENT); ReturnStatement returnStatement = (ReturnStatement) statement; Expression expression = returnStatement.getExpression(); assertNotNull("there is no expression", expression); ITypeBinding typeBinding = expression.resolveTypeBinding(); assertNotNull("No typebinding", typeBinding); assertEquals("wrong name", "int", typeBinding.getName()); So the return statement inside the method has been resolved and its binding is a primitive type binding. I don't see a problem on this side. I will investigate the call of this code.
The problem seems to be that between the creation of the tree and its iteration additional nodes have been created. This is not supposed to happen. This is why null is returned as the binding for the return statement.
Ok, I found the problem. The method getReturnType() always returns a type which is not null even if there is no valid return type for this method. During the conversion I don't set the return type if there is none, but the getter will create a default return type which is 'void'. Therefore the modification count changed and we consider that bindings are irrelevant. There is a pending discussion on this topics in bug 23162. I changed the code in the conversion to always set the return type of a method declaration even if it doesn't exist in the source and now the quick fix replies 'int' or 'String' for a proposed return type in both test cases. My concern is that the default return type has no position and therefore should not appear in the AST tree. So we can either spec this in the doc or return null when there is no return type. Returning null seems to be the right solution except that this is a fairly big API break. I will discuss this issue with Jim and I will post a note asap.
The "right" way to do it is that default values should not see as a modification of the tree. They are internal implementation detail. I will fix 23492 asap and this one will be fixed as a consequence.
Fixed with the change for bug 23492.
Released in 2.1 stream.
Verified.
Regression test added (test0401)
