Bugzilla – Full Text Bug Listing
| Summary: | [occurrences] Too many occurrences are marked when a variable redefined | ||
|---|---|---|---|
| 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, Michael_Rennie |
| Version: | unspecified | Flags: | Michael_Rennie:
review+
|
| Target Milestone: | 5.0 RC2 | ||
| Hardware: | PC | ||
| OS: | Mac OS X | ||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 428133 | ||
(In reply to Michael Rennie from comment #0) > If you place the carat in the param 'foo' (new expression) all occurrences > of foo are marked (WRONG) - also happened before the fix to bug 427836, so > not a regression In checkId we only check for redefines if both the defScope and defNode are set. However, in this case only def.scope is set because the carat is not in the defining node. I don't understand the reason for defNode. Removing defNode entirely fixes the problem. > If you place the carat on the last param 'foo' (function f3) both param > usages are marked (WRONG) When we encounter a redefine, we skip the scope, but if any occurrences were previously marked they stay. A solution here is to change the redefine code to: //trying to re-define, we can break since any matches past here would not be the original definition //redefinition is OK if we are still descending to the offset, but not once we are in position if(node.range[0] >= this.context.start) { return true; } else { // If we are redefining, any occurrences collected are now invalid this.occurrences = []; scope.occurrences = []; } This will clear all previous occurrences, and continue on as though the last defining node is the valid define. I'm not certain if this is the optimal behaviour, Mike and I can debug it together tomorrow. One limitation with this approach is if the carat is in an earlier define, all usage of the word will be marked, but not the redefines, whereas if the carat is in a later define, only usage after the carat will be marked. 1 var foo; 2 var a = foo; 3 var foo; 4 var b = foo; 1 - Occurrences at 1, 2, 4. Redefine at 3 skipped. 2 - Occurrences at 1, 2, 4. Redefine at 3 skipped. 3 - Occurrences at 3, 4. All previous occurrences skipped. 4 - Occurrences at 3, 4. Occurrences previous to redefine skipped. https://git.eclipse.org/r/21964 Mike please review Covers the problems in comment #0 and adds a large number of tests. Looks good, and all tests pass; +1. Merged via gerrit: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=0cf6f5b9bf52beadaa98ca0e8ba63436c2f70d72 |
Consider the following snippet: var foo = 1; function f1() {}; function f2() { var bar = new f1(foo); } function f3(foo, bar, baz) {}; If you place the carat on the first 'foo' it is marked and so is the param in the new expression (GOOD) If you place the carat in the param 'foo' (new expression) all occurrences of foo are marked (WRONG) - also happened before the fix to bug 427836, so not a regression If you place the carat on the last param 'foo' (function f3) both param usages are marked (WRONG)