Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 439641

Summary: [occurrences] Occurrences not marked for named function expression in property
Product: [ECD] Orion Reporter: Michael Rennie <Michael_Rennie>
Component: JS ToolsAssignee: Curtis Windatt <curtis.windatt.public>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: curtis.windatt.public
Version: 6.0   
Target Milestone: 7.0   
Hardware: PC   
OS: Mac OS X   
Whiteboard:

Description Michael Rennie CLA 2014-07-15 13:20:20 EDT
Consider the following snippet:

var foo = {
    func: function func() {
        
    }
};

if you select the 'func' property id, I would expect the name of the function to also be marked, provided it matched the property name.
Comment 1 Curtis Windatt CLA 2014-07-23 12:13:18 EDT
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=19a4e095cc55deb0bf13dc81e57ca52fb0e689ef

Added a simple fix for this.  If we find an object property with a value of a function expression, we check the expression id for an occurrence.

Note this only works when the object property is selected, not the reverse.  Currently object properties are only marked when an object property or this member expression is selected.
Comment 2 Michael Rennie CLA 2014-07-24 10:39:06 EDT
I think we should fix up the usage to be consistent.

Also the following fails:

var foo = {
    f: function f() {
        
    }
    baz: function b() {
        this.f();
    }
};
f<caret here>();

marks the f id in the func expression
Comment 3 Curtis Windatt CLA 2014-09-25 12:55:42 EDT
(In reply to Curtis Windatt from comment #1)
> Note this only works when the object property is selected, not the reverse. 
> Currently object properties are only marked when an object property or this
> member expression is selected.

This logic can be done now that we know what node we have selected. If a named func expr is selected we could then check for object properties with the same name in the same scope.

(In reply to Michael Rennie from comment #2)
> var foo = {
>     f: function f() {
>         
>     }
>     baz: function b() {
>         this.f();
>     }
> };
> f<caret here>();
> 
> marks the f id in the func expression

Are we saying that the object definition effectively means a new scope?  Currently we only add scopes for func decl and func expr.  There is a related test case already, but we don't check the last carat location.

it('test_namedFuncExpr6', function() {
			editorContext.text = "var x = { a: function a() {} }; a();";
			return occurrences.computeOccurrences(editorContext, setContext(11, 11)).then(function(results) {
				try {
					assertOccurrences(results, [{start:10, end:11}, {start:22, end:23}]);
				}
				finally {
					tearDown();
				}
			});
		});
Comment 4 Curtis Windatt CLA 2014-09-25 13:23:29 EDT
(In reply to Curtis Windatt from comment #3)
> Are we saying that the object definition effectively means a new scope? 
> Currently we only add scopes for func decl and func expr.  There is a
> related test case already, but we don't check the last carat location.

Yes and no.  EScope will not treat it as a functional scope.  However, for naming/identifiers, they are treated as a separate scope.
Comment 5 Curtis Windatt CLA 2014-09-25 13:32:36 EDT
Mike suggested a possible workaround.  As we have the node, we can check if the user has selected a named function expression with a parent object property.  If so, we could just run the logic to mark object properties.  We would have to fix up the logic to always select both the object property and the named function expression.  Also it would probably be a good idea to check that the names match.
Comment 6 Curtis Windatt CLA 2014-10-20 14:24:16 EDT
var foo = {
    f: function f() {
        
    }
    baz: function b() {
        this.f();  // Also a problem
    }
};
f();

Note that depending on whether you select before the f, after the f or the whole f on the line marked above, you get different behaviour.  This is because there is a missing parent.
Comment 7 Curtis Windatt CLA 2014-10-20 14:35:25 EDT
(In reply to Michael Rennie from comment #2)
> I think we should fix up the usage to be consistent.
> 
> Also the following fails:
> 
> var foo = {
>     f: function f() {
>         
>     }
>     baz: function b() {
>         this.f();
>     }
> };
> f<caret here>();
> 
> marks the f id in the func expression

This is caused by Bug 447962