Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 326901 - Parameter types not populated in function expression call
Summary: Parameter types not populated in function expression call
Status: RESOLVED FIXED
Alias: None
Product: JSDT
Classification: WebTools
Component: General (show other bugs)
Version: 3.2.3   Edit
Hardware: PC Mac OS X - Carbon (unsup.)
: P3 normal (vote)
Target Milestone: 3.2.3   Edit
Assignee: Nitin Dahyabhai CLA
QA Contact: Nitin Dahyabhai CLA
URL:
Whiteboard:
Keywords:
: 325904 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-10-04 05:39 EDT by Jacek Pospychala CLA
Modified: 2010-11-19 13:52 EST (History)
2 users (show)

See Also:
cmjaun: review+
thatnitind: review+


Attachments
patch (1.67 KB, patch)
2010-10-05 09:10 EDT, Jacek Pospychala CLA
cmjaun: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jacek Pospychala CLA 2010-10-04 05:39:11 EDT
I have vague feeling that this was already reported, but can't find it. Basically in following sample:

function(a1) {
 // operations on a1
}(window)

and

(function(a2){
  // operations on a2
})(window)


Variables a1 and a2 should inherit their types from window, thus making content-assist for a1 and a2 inside functions bodies work the same as for window variable.
Comment 1 Jacek Pospychala CLA 2010-10-04 07:41:32 EDT
I'm trying following to conect the types of function call arguments with nested function declaration.
Unfortunately getInferredType() always returns null, not sure why :-|

protected boolean handleFunctionCall(IFunctionCall messageSend, LocalDeclaration assignmentExpression) {
		IExpression receiver = messageSend.getReceiver();
		IExpression[] functionCallArgs = messageSend.getArguments();
		if (functionCallArgs != null && receiver instanceof FunctionExpression) {
			FunctionExpression function = (FunctionExpression) receiver;
			Argument[] args = function.methodDeclaration.arguments;
			
			for (int i = 0; i < functionCallArgs.length; i++) {
				if (args[i].getInferredType() == null) {
					InferredType newType = getInferredType((Expression)functionCallArgs[i]);
					args[i].setInferredType(newType);
				}
			}
		}
		
		return true;
	}
Comment 2 Jacek Pospychala CLA 2010-10-05 09:10:27 EDT
Created attachment 180236 [details]
patch

Patch that I find working, does the following:

- In MessageSend, check if receiver is FunctionExpression.
- resolve MessagageSend arguments types
- in FunctionExpression set arguments types to those resolved in MessageSend

I'm not an expert in types binding, so here are my concerns:
1. why do this in MessageSend.resolveType, and not in FunctionExpression.resolveType?
A: MessageSend is basically every function call, so this type resolution will be ran much more often than if it'd be in FunctionExpression. On the other hand, when in FunctionExpression there seems no way to figure out whether we're inside MessageSend, or not.
2. why add this at the beginning of resolveType(), and not after resolving function expression (aka receiver) type and messageSend arguments?
A: imho correct order should be to resolve MessageSend arguments, then set FunctionExpression arguments, and then resolve FunctionExpression.  The current code first resolves FunctionExpression, then MS arguments and in some cases uses resolved FE. FE.resolveType() already takes into account types of arguments, so FE arguments must be before FE.resolveType(). And to resolve arguments, MS arguments must be resolved first.

I used following examples to test it:
var i = "txt"; // i is String 

(function test1(a) {
	// a is String
})(i);

function test2(b) {
	// b is any
}(i); // here test2 is MethodDeclaration and i is SingleNameReference, parentheres seem disappeared in AST.

function test3(c) {
	// c is any
}
test3(i);

(
/**
 * @param a {Boolean}
 */
function test4(d) {
	// d is String, jsdoc is overriden
})(i);

(function test5(e, f, g) {
	// e is String, f is any, g is any 
})(i);

(function test6(h, i, j) {
	// h, i, j are any, local i hides global i
})();

(function test7() {
	// no errors 
})(i);
Comment 3 Jacek Pospychala CLA 2010-10-05 09:12:45 EDT
Guys, could you take a look and advise, if this is the "right way"?
Thanks
Comment 4 Nitin Dahyabhai CLA 2010-11-15 20:15:55 EST
Approved, although I'm still trying to figure out how to properly unit test it.
Comment 5 Nitin Dahyabhai CLA 2010-11-17 17:20:52 EST
Committed to HEAD and 3.2.3, thanks, Jacek!
Comment 6 Chris Jaun CLA 2010-11-19 13:52:26 EST
*** Bug 325904 has been marked as a duplicate of this bug. ***