| Summary: | [occurrences] Occurrences not marked for named function expression in property | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Michael Rennie <Michael_Rennie> |
| Component: | JS Tools | Assignee: | 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: | |||
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. 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
(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(); } }); }); (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. 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. 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.
(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 |
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.