closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

Deterministic test fails when using classes/function prototypes, but not otherwise

Open supahero1 opened this issue 3 years ago • 1 comments

The contents of test.js:

let a = null;

function b() {
        if(a != null) {
                console.log("not null");
        }
        a = 0;
}

b();
document.getElementById("id").onclick = b;

Compiled with closure-compiler -O ADVANCED --js test.js --js_output_file test.min.js

Output: none. No warnings, no errors. Compiled successfully.

Now, the new contents of test.js:

class A {
        constructor() {
                this.a = null;
        }
        b() {
                if(this.a != null) {
                        console.log("not null");
                }
                this.a = 0;
        }
}

const a = new A();
a.b();
document.getElementById("id").onclick = a.b.bind(a);

The same compilation step as before.

Output:

test.js:6:5: WARNING - [JSC_DETERMINISTIC_TEST] condition always evaluates to false
left : null
right: null
  6| 		if(this.a != null) {
     		   ^^^^^^^^^^^^^^

0 error(s), 1 warning(s), 100.0% typed

I suspect that's not intended. The condition does not always evaluate to false if the user clicks the "id" element.

$> closure-compiler --version
Closure Compiler (http://github.com/google/closure-compiler)
Version: v20220601

To provide a fuller bug submission, I've also tested the following 2 scripts, both of which work as expected (maybe will prove to be helpful):

let a = {
        a: null
};
function b() {
        if(this.a != null) {
                console.log("not null");
        }
        this.a = 0;
}
b.call(a);
document.getElementById("id").onclick = b.bind(a);

and

function a() {
        this.a = null;
}
a.prototype.b = function() {
        if(this.a != null) {
                console.log("not null");
        }
        this.a = 0;
};

const A = new a();
A.b();
document.getElementById("id").onclick = A.b.bind(A);

However, the script above throws the following warnings:

test.js:2:1: WARNING - [JSC_USED_GLOBAL_THIS] dangerous use of the global this object
  2| 	this.a = null;
     	^^^^

test.js:11:10: WARNING - [JSC_NOT_A_CONSTRUCTOR] cannot instantiate non-constructor
  11| const A = new a();
                ^^^^^^^

0 error(s), 2 warning(s), 57.1% typed

So I figured I will add /** @constructor */ to help it in some way, and there goes the class symptoms again, with the deterministic test failing to figure it out:

/** @constructor */
function a() {
        this.a = null;
}
a.prototype.b = function() {
        if(this.a != null) {
                console.log("not null");
        }
        this.a = 0;
};

const A = new a();
A.b();
document.getElementById("id").onclick = A.b.bind(A);
test.js:6:4: WARNING - [JSC_DETERMINISTIC_TEST] condition always evaluates to false
left : null
right: null
  6| 	if(this.a != null) {
     	   ^^^^^^^^^^^^^^

0 error(s), 1 warning(s), 100.0% typed

supahero1 avatar Aug 10 '22 16:08 supahero1

Interesting, thanks for reporting. A workaround is to explicitly type this.a in the constructor - this should tell Closure Compiler that this.a is potentially non-null:

class A {
        constructor() {
                /** @type {?number} */
                this.a = null;

lauraharker avatar Aug 10 '22 20:08 lauraharker