Bug 166203 - Refactoring doesn't work if initiated on function invocation statement
Refactoring doesn't work if initiated on function invocation statement
Status: VERIFIED FIXED
Product: javascript
Classification: Unclassified
Component: Refactoring
6.x
All All
: P2 (vote)
: 7.3
Assigned To: Petr Pisl
issues@javaee
: 7.0_WAIVER_APPROVED, 7.1_WAIVER_APPROVED, 7.2_WAIVER_APPROVED
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-28 17:12 UTC by Martin Schovanek
Modified: 2013-01-17 13:04 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Schovanek 2009-05-28 17:12:53 UTC
[Nb 6.7 RC1]

Rename does not work when is invoked at function invocation statement.

to reproduce:
-------------
1) have a .js like (| represents the cursor):
  function hello() {}
  he|llo();
2) invoke Rename [Ctrl + R]
3) choose Preview

ERROR: Rename found 0 occurrences.
Comment 1 Vitezslav Stejskal 2009-05-28 17:21:11 UTC
This is my comment from issue #166032 relevant to this problem:

I looked in the JsRenameHandler and it treats the function references as variables, which is of course wrong.

This seems to be poorly implemented anyway, just add to your example 'var hello;' and see what happens. The function
reference hello(); is colored as a variable (green in the default scheme). When the rename refactoring is initiated
either on var hello; it will rename just the variable declaration. If it's initiated on the function declaration it will
rename both the function declaration and the function call. If it's initiated on the function call, it will again rename
only the variable declaration. This all was tested in Nb6.5 and I assume 6.7 behaves the same.
Comment 2 Marek Fukala 2009-05-29 08:53:56 UTC
This is not stopper for 6.7. The problem is here from the very beginning as Vita found out.
Comment 3 Marian Mirilovic 2009-05-29 09:36:59 UTC
Vita/Marek ... thanks for evaluation, I removed this issue from list of potential FSC stoppers.
Comment 4 Petr Jiricka 2009-09-10 08:14:48 UTC
Petre P, can you please look into this? Thanks.
Comment 5 Petr Pisl 2009-09-10 14:30:46 UTC
I can confirm what Vita wrote in May that function reference is treated as a variable. 

The true is that the function name in JavaScript is a variable. So the function declaration 

function hello() {
   return "hello from function";
}

end 

var hello = function () {
   return "hello from function";
}

is almost identical. In both cases you can execute the function hello(). In both cases you can also print tha variable
hello, which contains the function declaration. The text in such case has only difference that the fist function
declaration contains the name of the function, the second one not. 

After this you can assign to the variable static text and from this time it's not function anymore until you assign into
the variable new function. Example:

function hello () {                 // Is exactly same like var hello = function hello() {...
    return "Hello from function";   // or you can also write var hello =function () {...
}

                                    //  output: 
alert(hello);                       //  function hello() {
                                    //      return "Hello from function";
                                    //  }

alert(hello());                     //  Hello from function

hello = "Hello from field";

alert (hello);                      //  Hello from field

alert(hello());                     // Runtime error. The hello is not a function


The mark occurences find in this example shows all occurences of hello variable, but it's done in the way, tha it marks
all nodes, which have text eguel to "hello". Yes the refactoring functionality can do the same, but this approach
doesn't take care about scopes. So if there will be function declaration

function hello (hello) {
    return hello;   
}


and you place cursor on a hello variable outside the function, then all hello are marked in the script inluding hello
parameter of the function, which is also wrong. The hello parameter is a variable in different scope and shouldn't be
marked. 

So if we will do the fix in the way how the mark occurences done, then the ide can offer many unwanted things. So this
is not also solution. I'm not sure how to fix it now, but I'm afraid that there has to be rewritten the variable visitor
and add some functionality for scopes.
Comment 6 Petr Pisl 2010-03-24 09:45:32 UTC
I don't think that this is possible to fix in 6.9 time scope. The release was too short and if there will not be someone, who works on javascript fulltime, then this kind of bug is not possible to fix, because the AST has to be rewritten.
Comment 7 Petr Jiricka 2010-04-19 13:11:20 UTC
Waiver approved for NB 6.9.
Comment 8 Marian Mirilovic 2011-03-15 15:23:10 UTC
Waiver request for >48 hours -> switch to waiver 
http://wiki.netbeans.org/WaiverProcess
Comment 9 Petr Jiricka 2011-05-04 09:05:16 UTC
Also requesting waiver for 7.0.1.
Comment 10 Petr Jiricka 2011-05-06 14:50:41 UTC
Approved.
Comment 11 Petr Pisl 2011-10-03 13:30:28 UTC
We are preparing rewriting the java script support. It's basically not possible to fix in the current infrastructure, but I don't want to close this issue to keep it for the next version of JS support. So downgrading to P3.
Comment 12 Petr Pisl 2012-05-10 12:29:24 UTC
Works correctly in the new JS support, that should be available publicly in NB 7.2.next
Comment 13 Petr Pisl 2012-09-12 14:10:06 UTC
The original report works in the new JS editor that is available as a part of NetBeans 7.3


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo