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

Bug 495751

Summary:

[quickfix] Provide quickfix to rename for loop var

Product:

[ECD] Orion

Reporter:

Eric Moffatt <emoffatt>

Component:

JS Tools

Assignee:

Olivier Thomann <Olivier_Thomann>

Status:

RESOLVED FIXED

QA Contact:

Severity:

enhancement

Priority:

CC:

curtis.windatt.public, Michael_Rennie, Olivier_Thomann

Version:

12.0

Target Milestone:

13.0

Hardware:

OS:

Windows 7

Whiteboard:

Attachments:

Description	Flags
Proposed patch including regression tests	none

Description Eric Moffatt

2016-06-08 16:01:44 EDT

if you have multiple loops (for example) each of which is like this:

for (var i = 0; i < 4; i++)

you will get a warning that 'i' is already defined. Seems like a nice place to put a 'Rename in scope' QF...

Comment 1 Michael Rennie

2016-06-09 11:29:33 EDT

We do not provide the rename in scope fix because the loop is not the scope for the var - so if you invoked the rename in scope fix it would rename 'i' in all of your loops found in the same scope.

We would need to craft another kind of rename that works on body / blocks / loops.

We could also have an additional fix to remove the decl - in case you were wanting to just reuse the same var in each of the loops.

Comment 2 Eric Moffatt

2016-06-10 10:31:14 EDT

Got it. Having a QF to remove the 'var' might be ok but in practical terms I'd likely never use it (just in case I remove the loop with the original declaration.

Low pri I guess (well after being able to select colors...).

Comment 3 Olivier Thomann

2016-07-06 11:57:25 EDT

Michael, so this is about adding a quickfix that removes the 'var' keyword?

Comment 4 Michael Rennie

2016-07-06 12:29:30 EDT

(In reply to Olivier Thomann from comment #3)
> Michael, so this is about adding a quickfix that removes the 'var' keyword?

That is one possible fix, the other would be to compute the usage of 'i' within the following block (not its containing scope), and do a rename on it.

for example:

var i = "hello";

for(var i = 0; i < 10; i++) { //<- these three would be renamed
   var foo = bar[i];          //<- this one would be renamed
}

console.log(i);

But to support the above case, we would have to add our own support to compute the usage within a block rather than a scope.

Comment 5 Olivier Thomann

2016-07-08 11:27:11 EDT

When we report the annotation for "no-redeclare" we have the whole list of references for 'i' and we have the position of the redeclare (i in the for loop).

One way to get it to work would be to "record" all positions of references of 'i' that are within the bounds of the parent node (for statement). This would need to be recorded against the annotation as it is needed to know what 'i' should be renamed. From the quickfix, we don't have the rule context anymore that let us get the scopes. So from the quickfix, we would simply change all recorded positions with a new name.

Comment 6 Olivier Thomann

2016-07-11 13:44:27 EDT

Created attachment 263022 [details]
Proposed patch including regression tests

Comment 7 Olivier Thomann

2016-07-12 14:43:48 EDT

Delivered.

Comment 8 Olivier Thomann

2016-07-18 09:34:21 EDT

*** Bug 486123 has been marked as a duplicate of this bug. ***