Bug 198032

This is a bad trap - please fix in 7.01

The fix for this will probably also fix issue 197817.

It's problem in infrastructure and I'm afraid that it will not be easy to fix.

I am requesting the removal of the refactoring feature in the JavaScript editor until this bug is fixed.

This bug is breaking JavaScript code without warning. The JavaScript developer does not have any awareness of breaking their code with refactoring all, and there is no means of discovering the errors. That is a fundamental problem that no IDE should have at any production stage.

It might be difficult to fix this bug. I was hoping for an easy fix, but I did not say that, meaning "please fix it only if it is easy to fix".

> The JavaScript developer does not have any awareness of breaking their code with refactoring all

I believe for other languages, we had a dialog saying "refactoring is approximate, please check the diff". Would it help in this case too?

> I was hoping for an easy fix, but I did not say that

No worries, this will be fixed. Just not for 7.0.1 - there is not enough time in the 7.0.1 schedule for this complex fix. I suggest to leave this bug open with P2 priority and request a waiver for 7.0.1.

IMHO JavaScript refactoring is approximate only where the scope/reachability is not well defined.

However in the private scope, refactoring is 100% defined, and a failure is catastrophic. Please try the test case. If it doesn't work here, then this is just extreme poor quality, and not worth taking the risk to even use it.

So please, please switch refactoring off or fix this bug in this release.

The problem in the infrastructure is that there is no context for variables. This is big problem. There has to be added this information and this is not easy. 

Unfortunately this behavior is here from NB 6.5.

Right, the behavior has been the same for several years. I still think refactoring is useful even in the current limited form - this is the first request to disable it that we got in several years. Going forward, there is a plan to fix it (involving essentially a complete rewrite of the JavaScript editor), so this will be addressed. However, such a change is not doable for 7.0.1, therefore I am requesting a waiver for 7.0.1.

Refactoring is only useful if does not break our source code.

In this case, the refactoring breaks the source code with no means available to detect and repair the damage. Then, a simple search and replace is more useful than refactoring.

The fact that "this is the first request to disable it that we got in several years" is the reflection of the fact above, so not many developers except the reporter use refactoring in a meaningful way. Otherwise they would have found this rather obvious bug.

More generally, not may people cared to use the NetBeans JavaScript editor at all. I don't know exactly why, possibly because of other severe bugs, eg bug 159704 which was raised by the same reporter. Likewise why would one want to rewrite the editor if nobody except the reporter complains?

I appreciate that there have been plans to rewrite the JavaScript editor for years. If the timeline is not very short term, eg in the range of weeks, then IMHO it would be preferable to limit the damage before such a rewrite not just carry on with business as usual ie waiver.

> More generally, not may people cared to use the NetBeans JavaScript editor at all. 

Not true, our statistics show that more than a quarter of all users use the JavaScript editor.

Sorry, any change in this area is too large and risky for NetBeans 7.0.1. I am marking the waiver as approved.

I have returned to this bug. I'm trying to solve it in the old infrastructure, because it looks like the new one, will not be ready in the next release.

At first I have to admit that I'm not big expert in writing JS code. I have change a little the example that you have provided: 

function level0() {
  var var_a = "level0";// invoking refectoring through CTRL+R here
                
  function level1_1(){
    var_a = var_a;
    document.write("level1_1: " + var_a +  "<br/>");
  }
  function level1_2(){
    var var_a;
    document.write("level1_2: " + var_a +  "<br/>");
  }
  function level1_3 (){
    var_a = var_a;
    document.write("level1_3: " + var_a +  "<br/>");
  }
  function level1_4 (){
    this.level2_1 = function(){
      var_a = var_a;
      document.write("level2_1: " + var_a +  "<br/>");
    }
  }
                
  level1_1();
  level1_2();
  level1_3();
  var o = new level1_4();
  o.level2_1();
}
level0();

After executing the code the output is:
level1_1: level0
level1_2: undefined
level1_3: level0
level2_1: level0

If I do the code right, that only var_a shouldn't be refactored in level1_2 method. When you press ctrl+r in var_a declaration only the var_a in level1_1 method are offered to refactor. So from my example there should be also refactored var_a in level1_3 and level2_1 methods, but this doesn't correspond with comment in your example. So at first i would like to clarify it.

Yes Petr, I agree with your last statements.

My choice of naming wasn't perfect, here is another version of the example:

/*
Variable hidden from refactor rename
*/
function level0() {
    var first;// Try rename refactor from here
    function level1_1(){
        first = "a";
    }
    function level1_2(){
        var first = "a";// This hidden declaration causes the damage.
    }
    function level1_3(){
        first = "a";// hidden - cannot be refactored from level0
    }
    function level1_4(){
        this.level2_1 = function(){
            first = "x";// hidden - cannot be refactored from level0
        }
    }
}

As you say, "var first" should only be protected from refactoring in function level1_2() which is the only place where it is declared again on a nested level.

In the new JS support it's going to work. The occurrences are already implemented. The refactoring not yet.

Instant rename also implemented in the new JS support.

This issue can be closed for now. The "standard" refactoring is not implemented yet in the new JS editor, but the instant rename works as expected.

verified

Product Version: NetBeans IDE 7.3 RC1 (Build 201301142100)
Java: 1.7.0_10; Java HotSpot(TM) Client VM 23.6-b04
Runtime: Java(TM) SE Runtime Environment 1.7.0_10-b18
System: Linux version 3.2.0-35-generic-pae running on i386; UTF-8; en_US (nb)