google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.38k stars 1.15k forks source link

JSC_INEXISTENT_PROPERTY_WITH_SUGGESTION is hidden if the inexistent property is set in unrelated code. #4164

Closed bamtang-dev closed 4 months ago

bamtang-dev commented 4 months ago

Hello, not sure if this is expected behavior. Below code is minimal sample tested in ADVANCED mode on: https://closure-compiler.appspot.com/

/**
 * @constructor
 * @struct
 */
function SGlobal() {
    this.className = "SGlobal";
}
/** @type {number}*/
SGlobal.scale = 1;

/**
 * @constructor
 * @struct
 * @param {number} x
 */
function ZoomManager(x) {
    this.m_x = x * SGlobal.scale;
}

/**
 * @param {number} time
 * @param {number} x
 */
ZoomManager.prototype.onDisplace = function(time, x) {
    this.m_x = x + time * SGlobal.scaleT; // <--- THIS IS A TYPO, property was 'scale'
};

ZoomManager.prototype.neverCalled = function() {
    //SGlobal.scaleT = 4;
};

var obj = new ZoomManager(10);
obj.onDisplace(0.5, 1);
var dist = obj.m_x;
console.log(dist);

This code has a typo and the compiler correctly points the error:

_JSC_INEXISTENT_PROPERTY_WITHSUGGESTION: Property scaleT never defined on SGlobal. Did you mean scale? at line 25 character 31

But if you are unlucky like me and have a second typo in other unrelated part of your code the warning goes away and no warning or error is printed. Here I put the second typo in a method of the class that is never called

ZoomManager.prototype.neverCalled = function() {
    SGlobal.scaleT = 4;  // <--- SECOND TYPO, replace this code in above example
};

What is interesting is that the compiler somehow knows that the output is undefined because is producing compiled code with NaN:

'use strict';
var a = new function() {
  this.g = 10;
}();
a.g = NaN;
console.log(a.g);

I was under the impression that the compiler warned me whenever an undeclared property of a @struct was tried to use.

rishipal commented 4 months ago

I suspect this is happening because by doing SGlobal.scaleT = 4; inside ZoomManager.prototype.neverCalled you have created a property named scaleT. Hence it's no longer an inexistent property, i.e. the compiler sees that a property with that name exists somewhere in the code. Hence the inexistent property error is not reported by closure compiler and this seems WAI to me.

Furthermore, if you change the neverCalled method to have a "use" of the inexistent property, then the compiler correctly complains at both use-sites of the inexistent property.

ZoomManager.prototype.neverCalled = function() {
    this.m_x = SGlobal.scaleT;
};

See - http://shortn/_J3xapGS5bl

Will discuss this in the internal bug triage meeting and update again.

rishipal commented 4 months ago

Closing as WAI per my comment above.