Skip to content

Commit

Permalink
Perform looser null/undefined typechecking only for files generated f…
Browse files Browse the repository at this point in the history
…rom Java.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=125375011
  • Loading branch information
dimvar authored and blickly committed Jun 20, 2016
1 parent f917eed commit e55ce2a
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 136 deletions.
16 changes: 0 additions & 16 deletions src/com/google/javascript/jscomp/CompilerOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -191,21 +191,6 @@ public boolean shouldGenerateTypedExterns() {
/** Checks types on expressions */
public boolean checkTypes;

// When compiling to JS from other languages (eg Java), we can be lenient
// around null and undefined in types.
// NOTE(dimvar): IN DEVELOPMENT, DO NOT USE!
// This flag is temporary while developing the feature.
// The final version will check this on a per-file basis.
private boolean checkTypesModuloNullUndefined;

void setTypecheckModuloNullUndefined(boolean ignoreNull) {
this.checkTypesModuloNullUndefined = ignoreNull;
}

boolean getTypecheckModuloNullUndefined() {
return this.checkTypesModuloNullUndefined;
}

public CheckLevel reportMissingOverride;

/**
Expand Down Expand Up @@ -1047,7 +1032,6 @@ public CompilerOptions() {
checkSymbols = false;
checkSuspiciousCode = false;
checkTypes = false;
checkTypesModuloNullUndefined = false;
reportMissingOverride = CheckLevel.OFF;
checkGlobalNamesLevel = CheckLevel.OFF;
brokenClosureRequiresLevel = CheckLevel.ERROR;
Expand Down
12 changes: 10 additions & 2 deletions src/com/google/javascript/jscomp/TypeCheck.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.google.common.collect.Iterables;
import com.google.javascript.jscomp.CodingConvention.SubclassRelationship;
import com.google.javascript.jscomp.CodingConvention.SubclassType;
import com.google.javascript.jscomp.TypeValidator.SubtypingMode;
import com.google.javascript.jscomp.type.ReverseAbstractInterpreter;
import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.JSTypeExpression;
Expand Down Expand Up @@ -432,8 +433,15 @@ private void report(NodeTraversal t, Node n, DiagnosticType diagnosticType,
}

@Override
public boolean shouldTraverse(
NodeTraversal t, Node n, Node parent) {
public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
if (n.isScript()) {
String filename = n.getSourceFileName();
if (filename != null && filename.endsWith(".java.js")) {
this.validator.setSubtypingMode(SubtypingMode.IGNORE_NULL_UNDEFINED);
} else {
this.validator.setSubtypingMode(SubtypingMode.NORMAL);
}
}
switch (n.getType()) {
case FUNCTION:
// normal type checking
Expand Down
13 changes: 7 additions & 6 deletions src/com/google/javascript/jscomp/TypeValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,13 @@ class TypeValidator {
private final JSType allValueTypes;
private final JSType nullOrUndefined;

// Feature in development. See CompilerOptions#checkTypesModuloNullUndefined
static enum SubtypingMode {
NORMAL,
IGNORE_NULL_UNDEFINED
}
private final SubtypingMode subtypingMode;
// In TypeCheck, when we are analyzing a file with .java.js suffix, we set
// this field to IGNORE_NULL_UNDEFINED
private SubtypingMode subtypingMode = SubtypingMode.NORMAL;

// TODO(nicksantos): Provide accessors to better filter the list of type
// mismatches. For example, if we pass (Cake|null) where only Cake is
Expand Down Expand Up @@ -164,10 +165,6 @@ static enum SubtypingMode {
STRING_TYPE, NUMBER_TYPE, BOOLEAN_TYPE, NULL_TYPE, VOID_TYPE);
this.nullOrUndefined = typeRegistry.createUnionType(
NULL_TYPE, VOID_TYPE);
this.subtypingMode =
compiler.getOptions().getTypecheckModuloNullUndefined()
? SubtypingMode.IGNORE_NULL_UNDEFINED
: SubtypingMode.NORMAL;
}

/**
Expand Down Expand Up @@ -206,6 +203,10 @@ Iterable<TypeMismatch> getMismatches() {
return mismatches;
}

void setSubtypingMode(SubtypingMode mode) {
this.subtypingMode = mode;
}

/**
* all uses of implicitly implemented structural interfaces,
* captured during type validation and type checking
Expand Down
116 changes: 5 additions & 111 deletions test/com/google/javascript/jscomp/TypeCheckTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16623,120 +16623,20 @@ public void testDuplicateVariableDefinition8_7() {
+ "original definition at [testcode]:5 with type (null|rec<string>)");
}

public void testModuloNullUndef1() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"function f(/** number */ to, /** (number|null) */ from) {",
" to = from;",
"}"));
}

public void testModuloNullUndef2() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"function f(/** number */ to, /** (number|undefined) */ from) {",
" to = from;",
"}"));
}

public void testModuloNullUndef3() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {}",
"function f(/** !Foo */ to, /** ?Foo */ from) {",
" to = from;",
"}"));
}

public void testModuloNullUndef4() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {}",
"/** @constructor @extends {Foo} */",
"function Bar() {}",
"function f(/** !Foo */ to, /** ?Bar */ from) {",
" to = from;",
"}"));
}

public void testModuloNullUndef5() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"function f(/** {a: number} */ to, /** {a: (null|number)} */ from) {",
" to = from;",
"}"));
}

public void testModuloNullUndef6() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"function f(/** {a: number} */ to, /** ?{a: (null|number)} */ from) {",
" to = from;",
"}"));
}

public void testModuloNullUndef7() {
testTypesModuloNullUndefined(LINE_JOINER.join(
public void testModuloNullUndefThatWorkedWithoutSpecialSubtypingRules() {
testTypes(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {}",
"function f(/** function():!Foo */ to, /** function():?Foo */ from) {",
" to = from;",
"function f(/** function(?Foo, !Foo) */ x) {",
" return /** @type {function(!Foo, ?Foo)} */ (x);",
"}"));
}

public void testModuloNullUndef8() {
testTypesModuloNullUndefined(LINE_JOINER.join(
testTypes(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {}",
"function f(/** !Array<!Foo> */ to, /** !Array<?Foo> */ from) {",
" to = from;",
"}"));
}

public void testModuloNullUndef9() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"/**",
" * @constructor",
" * @template T",
" */",
"function Foo() {}",
"function f(/** !Foo<number> */ to, /** !Foo<(number|null)> */ from) {",
" to = from;",
"}"));
}

public void testModuloNullUndef10() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"/** @interface */",
"function Foo() {}",
"/** @type {function(?number)} */",
"Foo.prototype.prop;",
"/** @constructor @implements {Foo} */",
"function Bar() {}",
"/** @type {function(number)} */",
"Bar.prototype.prop;"));
}

public void testModuloNullUndef11() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"/** @constructor */",
"function Bar() {}",
"/** @type {!number} */",
"Bar.prototype.prop;",
"function f(/** ?number*/ n) {",
" (new Bar).prop = n;",
"}"));
}

public void testModuloNullUndef12() {
testTypesModuloNullUndefined(LINE_JOINER.join(
"function f(/** number */ n) {}",
"f(/** @type {?number} */ (null));"));
}

public void testModuloNullUndefThatWorkedWithoutSpecialSubtypingRules() {
testTypes(LINE_JOINER.join(
"/** @constructor */",
"function Foo() {}",
"function f(/** function(?Foo, !Foo) */ x) {",
" return /** @type {function(!Foo, ?Foo)} */ (x);",
"}"));

testTypes(LINE_JOINER.join(
"function f(/** ?Object */ x) {",
Expand All @@ -16749,12 +16649,6 @@ public void testModuloNullUndefThatWorkedWithoutSpecialSubtypingRules() {
"}"));
}

private void testTypesModuloNullUndefined(String js) {
compiler.getOptions().setTypecheckModuloNullUndefined(true);
testTypes(js);
compiler.getOptions().setTypecheckModuloNullUndefined(false);
}

private void testTypes(String js) {
testTypes(js, (String) null);
}
Expand Down

0 comments on commit e55ce2a

Please sign in to comment.