Record typedef not type checked correctly if inside constructor

[GoogleCodeExporter]

We define a record type with *country*, *language*, and *name* keys defined all 
as string:

/**
 * @typedef {{country: string, language: string, name: string}}
 */
tv.i18n.data;

If we access the record type we just defined inside a constructor by setting it 
as an object property, it will not be type checked correctly.

Here's a sample usage:

/**
 * @constructor
 * @param {tv.i18n.data} data
 */
tv.i18n.ContentResource = function(data) {
  // Works normally, all these are defined.
  this.country = data.country;
  this.language = data.language;
  this.name = data.name;

  // Not throwing error?? id and map is not part of the record typedef.
  this.id = data.id;
  this.map = data.map;

  // Throws error, just because property name != name of the property on data.
  // ERROR - Property styleStr never defined on data
  // If you change "this.style" to this.styleStr the error goes away.
  this.style = data.styleStr; 
};

The compiler is not raising error when I am accessing *id* and *name*, both 
undefined keys in the type. It is, however, raising an error when I access 
*styleStr*. The error goes away if I change the statement into "this.styleStr = 
data.styleStr".

I'm compiling with checktypes etc enabled. I've attached a test case, the bug 
is tested in the latest closure compiler jar (April 8th, 2014).

Here is the script I use to compile:

java -jar compiler.jar \
--js testtypedef.js \
--module test:1 \
--module_output_path_prefix ./ \
--create_source_map ./%outname%.map \
--jscomp_error checkTypes \
--jscomp_error checkVars \
--jscomp_error visibility \
--jscomp_error ambiguousFunctionDecl \
--jscomp_error const \
--jscomp_error externsValidation \
--jscomp_error nonStandardJsDocs \
--jscomp_error strictModuleDepCheck \
--jscomp_error undefinedNames \
--jscomp_error uselessCode \
--jscomp_error invalidCasts \
--jscomp_error checkRegExp \
--jscomp_error duplicate \
--jscomp_error internetExplorerChecks \

I am using Ubuntu.

Original issue reported on code.google.com by ri...@traveloka.com on 11 Apr 2014 at 11:50

Attachments:

• [Typedef Bug.tar.gz](https://storage.googleapis.com/google-code-attachments/closure-compiler/issue-1300/comment-0/Typedef Bug.tar.gz)

[GoogleCodeExporter]

+dimvar

In the new type inference system, will "{{country: string, language: string, 
name: string}}" mean "an object with only those three properties, and no 
others" or "an object with those three properties, and it may contain other 
properties too"? Do we have any way to distinguish those two cases?

Original comment by tbreisac...@google.com on 23 Apr 2014 at 6:34

[GoogleCodeExporter]

Per discussion here
https://groups.google.com/forum/#!searchin/closure-compiler-discuss/constructor|
sort:date/closure-compiler-discuss/gs_MM6oahd8/PM90tdk00fYJ
I couldn't reproduce this.

@tyler: both in the current and the new inference, a record type means an 
object with these properties, and possibly more. So, if a function takes a 
record type, you can pass any object that has at least the properties in the 
record.

But, if you access a property on a record type that's not mentioned in the 
type, we warn about inexistent-property. So, the record type should contain the 
minimal set of properties you want to access.

Original comment by dim...@google.com on 23 Apr 2014 at 6:41

[GoogleCodeExporter]

Just read your replies. Will try to reproduce in the closure compiler web app.
I used the newest April compiler.jar version.

Did you try to reproduce via compiler.jar call?

Original comment by ri...@traveloka.com on 24 Apr 2014 at 5:13

[GoogleCodeExporter]

No I didn't. I would prefer to see a simpler repro case if possible.

Original comment by dim...@google.com on 24 Apr 2014 at 5:33

[GoogleCodeExporter]

The bug is reproduce-able via the appspot web app.

Expected result:

- I accessed both data.id and data.map, both properties are not defined in the 
typedef. Closure compiler should throw a warning for them.
- Closure compiler should throw a warning for data.styleStr access, no matter 
what the property name it is assigned to.

Actual result:

- data.styleStr warning is thrown, but data.id and data.map access is not.
- If we change "this.style = data.styleStr" to "this.styleStr = data.styleStr" 
the warning disappears and compilation is successful.

// ==ClosureCompiler==
// @compilation_level ADVANCED_OPTIMIZATIONS
// @warning_level verbose
// @output_file_name default.js
// ==/ClosureCompiler==

goog.provide('tv.i18n.ContentResource');
goog.provide('tv.i18n.data');

/**
 * @typedef {{country: string, language: string, name: string}}
 */
tv.i18n.data;

/**
 * @constructor
 * @param {tv.i18n.data} data
 */
tv.i18n.ContentResource = function(data) {
  // Works normally, all these are defined.
  this.country = data.country;
  this.language = data.language;
  this.name = data.name;

  // Not throwing error?? id and map is not part of the record typedef.
  this.id = data.id;
  this.map = data.map;

  // Throws error, just because property name != name of the property on data.
  // ERROR - Property styleStr never defined on data
  // If you change "this.style" to this.styleStr the error goes away.
  this.style = data.styleStr;
};

Original comment by ri...@traveloka.com on 24 Apr 2014 at 6:23

[GoogleCodeExporter]

Addition:

Similarly, if we change "this.map = data.map" into "this.mapasdf = data.map" 
the compiler throws warning as expected. It seems if the property name is the 
same, somehow typedef isn't checked. 

Original comment by ri...@traveloka.com on 24 Apr 2014 at 6:29

[GoogleCodeExporter]

The case is a bit different with "this.id = data.id":

- this.id = data.id; // no warning
- this.idasdf = data.id // no warning
- this.id = data.ids // warning!
- this.ids = data.ids // no warning

The compiler should throw warning for all of the above.

Original comment by ri...@traveloka.com on 24 Apr 2014 at 6:42

[GoogleCodeExporter]

I minimized the repro to the following.

/**
 * @constructor
 * @param {{a: number}} x
 */
function Bar(x) {
  this.b = x.b; // warns correctly
}

/**
 * @constructor
 * @param {{c: number}} x
 */
function Foo(x) {
  this.c = x.c; // comment out this line & the next warning no longer missed
  this.d = x.d; // misses warning
}

This is clearly a bug, and it has to do with how the compiler is unnecessarily 
permissive when doing the inexistent-property checks. Thanks for the report. We 
will fix this in the new type inference (currently under development).

Original comment by dim...@google.com on 24 Apr 2014 at 6:50

• Changed state: Accepted

[GoogleCodeExporter]

Thank you for the fast response, and for the software :)

Original comment by ri...@traveloka.com on 24 Apr 2014 at 7:00

[GoogleCodeExporter]

Issue tracking has been migrated to github. Please make any further comments on 
this issue through https://github.com/google/closure-compiler/issues

Original comment by blic...@google.com on 1 May 2014 at 6:31

• Added labels: Restrict-AddIssueComment-Nobody

[GoogleCodeExporter]

Original comment by blic...@google.com on 1 May 2014 at 6:34

• Changed state: Duplicate