Skip to content

Commit

Permalink
[NTI] Test both options for type checking ES6 natively in NTI tests.
Browse files Browse the repository at this point in the history
This requires a few fixes to LateEs6ToEs3Converter, and some extra test infrastructure to expect different warnings from the different cases.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=175558023
  • Loading branch information
shicks authored and Tyler Breisacher committed Nov 13, 2017
1 parent 4e25826 commit f032e1f
Show file tree
Hide file tree
Showing 9 changed files with 177 additions and 57 deletions.
11 changes: 8 additions & 3 deletions src/com/google/javascript/jscomp/LateEs6ToEs3Converter.java
Expand Up @@ -150,11 +150,16 @@ private void visitForOf(NodeTraversal t, Node node, Node parent) {
Node iterable = node.removeFirstChild(); Node iterable = node.removeFirstChild();
Node body = node.removeFirstChild(); Node body = node.removeFirstChild();


ObjectTypeI iterableType = null;
TypeI typeParam = unknownType; TypeI typeParam = unknownType;
if (addTypes) { if (addTypes) {
iterableType = iterable.getTypeI().autobox().toMaybeObjectType(); // TODO(sdh): This is going to be null if the iterable is nullable or unknown. We might want
typeParam = iterableType.getTemplateTypes().get(0); // to consider some way of unifying rather than simply looking at the nominal type.
ObjectTypeI iterableType = iterable.getTypeI().autobox().toMaybeObjectType();
if (iterableType != null) {
TypeIRegistry registry = compiler.getTypeIRegistry();
TypeI iterableBaseType = registry.getNativeType(JSTypeNative.ITERABLE_TYPE);
typeParam = iterableType.getInstantiatedTypeArgument(iterableBaseType);
}
} }
TypeI iteratorType = createGenericType(JSTypeNative.ITERATOR_TYPE, typeParam); TypeI iteratorType = createGenericType(JSTypeNative.ITERATOR_TYPE, typeParam);
TypeI iIterableResultType = createGenericType(JSTypeNative.I_ITERABLE_RESULT_TYPE, typeParam); TypeI iIterableResultType = createGenericType(JSTypeNative.I_ITERABLE_RESULT_TYPE, typeParam);
Expand Down
26 changes: 3 additions & 23 deletions src/com/google/javascript/jscomp/newtypes/JSType.java
Expand Up @@ -708,30 +708,10 @@ public final JSType substituteGenericsWithUnknown() {
return substituteGenerics(this.commonTypes.MAP_TO_UNKNOWN); return substituteGenerics(this.commonTypes.MAP_TO_UNKNOWN);
} }


/** @Override
* Given an generic supertype of this, public JSType getInstantiatedTypeArgument(TypeI supertype) {
* returns the type argument as instantiated by this type.
*
* Parameter supertype has to be a type with a single type parameter.
*
* For example,<pre> {@code
* (Iterable<Foo>).getInstantiatedTypeArgument(Iterable<?>) returns Foo,
* and
* /** {@literal @}template A * /
* class Foo {}
* /**
* * {@literal @}template B
* * {@literal @}extends {Foo<Array<B>>}
* * /
* class Bar {}
* (Bar<string>).getInstantiatedTypeArguments(Bar<?>) returns string
* (Bar<string>).getInstantiatedTypeArguments(Foo<?>) returns Array<string>
* }</pre>
* This is used, for example, in type-checking for-of and yield.
*/
public JSType getInstantiatedTypeArgument(JSType supertype) {
RawNominalType rawType = RawNominalType rawType =
supertype.getNominalTypeIfSingletonObj().getRawNominalType(); ((JSType) supertype).getNominalTypeIfSingletonObj().getRawNominalType();
List<String> typeParameters = rawType.getTypeParameters(); List<String> typeParameters = rawType.getTypeParameters();
checkState(typeParameters.size() == 1); checkState(typeParameters.size() == 1);


Expand Down
2 changes: 2 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/JSTypes.java
Expand Up @@ -515,6 +515,8 @@ public JSType getNativeType(JSTypeNative typeId) {
return getIIterableResultInstance(UNKNOWN); return getIIterableResultInstance(UNKNOWN);
case I_TEMPLATE_ARRAY_TYPE: case I_TEMPLATE_ARRAY_TYPE:
return getITemplateArrayType(); return getITemplateArrayType();
case ITERABLE_TYPE:
return getIterableInstance(UNKNOWN);
case ITERATOR_TYPE: case ITERATOR_TYPE:
return getIteratorInstance(UNKNOWN); return getIteratorInstance(UNKNOWN);
case GENERATOR_TYPE: case GENERATOR_TYPE:
Expand Down
23 changes: 23 additions & 0 deletions src/com/google/javascript/rhino/ObjectTypeI.java
Expand Up @@ -172,6 +172,29 @@ public interface ObjectTypeI extends TypeI {
*/ */
TypeI getEnumeratedTypeOfEnumObject(); TypeI getEnumeratedTypeOfEnumObject();


/**
* Given an generic supertype of this,
* returns the type argument as instantiated by this type.
*
* Parameter supertype has to be a type with a single type parameter.
*
* For example,<pre> {@code
* (Iterable<Foo>).getInstantiatedTypeArgument(Iterable<?>) returns Foo,
* and
* /** {@literal @}template A * /
* class Foo {}
* /**
* * {@literal @}template B
* * {@literal @}extends {Foo<Array<B>>}
* * /
* class Bar {}
* (Bar<string>).getInstantiatedTypeArguments(Bar<?>) returns string
* (Bar<string>).getInstantiatedTypeArguments(Foo<?>) returns Array<string>
* }</pre>
* This is used, for example, in type-checking for-of and yield.
*/
TypeI getInstantiatedTypeArgument(TypeI supertype);

/** /**
* Returns a set of properties defined or inferred on this type or any of its supertypes. * Returns a set of properties defined or inferred on this type or any of its supertypes.
*/ */
Expand Down
1 change: 1 addition & 0 deletions src/com/google/javascript/rhino/jstype/JSTypeNative.java
Expand Up @@ -98,6 +98,7 @@ public enum JSTypeNative {
GENERATOR_TYPE, GENERATOR_TYPE,


I_ITERABLE_RESULT_TYPE, I_ITERABLE_RESULT_TYPE,
ITERABLE_TYPE,
ITERATOR_TYPE, ITERATOR_TYPE,


I_TEMPLATE_ARRAY_TYPE, I_TEMPLATE_ARRAY_TYPE,
Expand Down
5 changes: 5 additions & 0 deletions src/com/google/javascript/rhino/jstype/ObjectType.java
Expand Up @@ -843,4 +843,9 @@ public ObjectTypeI withoutStrayProperties() {
// so we don't need to change anything. // so we don't need to change anything.
return this; return this;
} }

@Override
public TypeI getInstantiatedTypeArgument(TypeI supertype) {
throw new UnsupportedOperationException();
}
} }
6 changes: 3 additions & 3 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
Expand Up @@ -7039,16 +7039,16 @@ public void testBoxedUnification() {
" * @param {V} val", " * @param {V} val",
" * @template K, V", " * @template K, V",
" */", " */",
"function Map(key, val) {};", "function MyMap(key, val) {};",
"/**", "/**",
" * @param {!Map<K, (V | !Box<V>)>} inMap", " * @param {!MyMap<K, (V | !Box<V>)>} inMap",
" * @constructor", " * @constructor",
" * @template K, V", " * @template K, V",
" */", " */",
"function WrappedMap(inMap){};", "function WrappedMap(inMap){};",
"/** @return {(boolean |!Box<boolean>)} */", "/** @return {(boolean |!Box<boolean>)} */",
"function getUnion(/** ? */ u) { return u; }", "function getUnion(/** ? */ u) { return u; }",
"var inMap = new Map('asdf', getUnion(123));", "var inMap = new MyMap('asdf', getUnion(123));",
"/** @param {!WrappedMap<string, boolean>} x */", "/** @param {!WrappedMap<string, boolean>} x */",
"function getWrappedMap(x) {}", "function getWrappedMap(x) {}",
"getWrappedMap(new WrappedMap(inMap));")); "getWrappedMap(new WrappedMap(inMap));"));
Expand Down
115 changes: 105 additions & 10 deletions test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java
Expand Up @@ -26,6 +26,7 @@
import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature; import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.function.Function;


/** /**
* Use by the classes that test {@link NewTypeInference}. * Use by the classes that test {@link NewTypeInference}.
Expand Down Expand Up @@ -96,7 +97,7 @@ boolean checkTranspiled() {


@SuppressWarnings("hiding") @SuppressWarnings("hiding")
protected static final String DEFAULT_EXTERNS = protected static final String DEFAULT_EXTERNS =
CompilerTypeTestCase.DEFAULT_EXTERNS + LINE_JOINER.join( CompilerTestCase.DEFAULT_EXTERNS + LINE_JOINER.join(
"/** @const {undefined} */", "/** @const {undefined} */",
"var undefined;", "var undefined;",
"/**", "/**",
Expand Down Expand Up @@ -173,6 +174,20 @@ boolean checkTranspiled() {
" * @return {string}", " * @return {string}",
" */", " */",
"String.raw = function(template, var_args) {};", "String.raw = function(template, var_args) {};",
"/**",
" * @constructor",
" * @implements {Iterable<!Array<KEY|VALUE>>}",
" * @param {Iterable<!Array<KEY|VALUE>>=} opt_arg",
" * @template KEY, VALUE",
" */",
"function Map(opt_arg) {}",
"/**",
" * @constructor",
" * @implements {Iterable<VALUE>}",
" * @param {Iterable<VALUE>=} opt_arg",
" * @template VALUE",
" */",
"function Set(opt_arg) {}",
"/** @return {?} */", "/** @return {?} */",
"function any() {}"); "function any() {}");


Expand Down Expand Up @@ -209,7 +224,7 @@ protected FeatureSet featureSet() {
}; };
} }


private final void parseAndTypeCheck(String externs, String js) { private void parseAndTypeCheck(String externs, String js) {
initializeNewCompiler(compilerOptions); initializeNewCompiler(compilerOptions);
compiler.init( compiler.init(
ImmutableList.of(SourceFile.fromCode("[externs]", externs)), ImmutableList.of(SourceFile.fromCode("[externs]", externs)),
Expand Down Expand Up @@ -239,25 +254,43 @@ private final void parseAndTypeCheck(String externs, String js) {
TranspilationPasses.addEs2017Passes(passes); TranspilationPasses.addEs2017Passes(passes);
TranspilationPasses.addEs2016Passes(passes); TranspilationPasses.addEs2016Passes(passes);
TranspilationPasses.addEs6EarlyPasses(passes); TranspilationPasses.addEs6EarlyPasses(passes);
TranspilationPasses.addEs6LatePasses(passes); TranspilationPasses.addEs6PassesBeforeNTI(passes);
TranspilationPasses.addRewritePolyfillPass(passes); if (!compilerOptions.getTypeCheckEs6Natively()) {
TranspilationPasses.addEs6PassesAfterNTI(passes);
TranspilationPasses.addRewritePolyfillPass(passes);
}
} }
passes.add(makePassFactory("GlobalTypeInfo", new GlobalTypeInfoCollector(compiler))); passes.add(makePassFactory("GlobalTypeInfo", new GlobalTypeInfoCollector(compiler)));
passes.add(makePassFactory("NewTypeInference", new NewTypeInference(compiler))); passes.add(makePassFactory("NewTypeInference", new NewTypeInference(compiler)));
if (compilerOptions.needsTranspilationFrom(FeatureSet.ES6)
&& compilerOptions.getTypeCheckEs6Natively()) {
TranspilationPasses.addEs6PassesAfterNTI(passes);
TranspilationPasses.addRewritePolyfillPass(passes);
}


PhaseOptimizer phaseopt = new PhaseOptimizer(compiler, null); PhaseOptimizer phaseopt = new PhaseOptimizer(compiler, null);
phaseopt.consume(passes); phaseopt.consume(passes);
phaseopt.process(compiler.getExternsRoot(), compiler.getJsRoot()); phaseopt.process(compiler.getExternsRoot(), compiler.getJsRoot());
} }


protected final void typeCheck(String js, DiagnosticType... warningKinds) { // Note: firstWarningKind is necessary to disambiguate the zero-diagnostic case.
protected final void typeCheck(
String js, DiagnosticType firstWarningKind, DiagnosticType... warningKinds) {
typeCheck(js, Diagnostic.wrap(firstWarningKind, warningKinds));
}

protected final void typeCheck(String js, Diagnostic... diagnostics) {
if (this.mode.checkNative()) { if (this.mode.checkNative()) {
compilerOptions.setLanguageOut(LanguageMode.ECMASCRIPT_2015); compilerOptions.setLanguageOut(LanguageMode.ECMASCRIPT_2015);
typeCheck(DEFAULT_EXTERNS, js, warningKinds); typeCheck(DEFAULT_EXTERNS, js, Diagnostic.unwrap(diagnostics, compilerOptions));
} }
if (this.mode.checkTranspiled()) { if (this.mode.checkTranspiled()) {
compilerOptions.setLanguageOut(LanguageMode.ECMASCRIPT5); compilerOptions.setLanguageOut(LanguageMode.ECMASCRIPT5);
typeCheck(DEFAULT_EXTERNS, js, warningKinds); // TODO(sdh): stop allowing this option to be false, then we can eliminate this extra check.
compilerOptions.setTypeCheckEs6Natively(false);
typeCheck(DEFAULT_EXTERNS, js, Diagnostic.unwrap(diagnostics, compilerOptions));
compilerOptions.setTypeCheckEs6Natively(true);
typeCheck(DEFAULT_EXTERNS, js, Diagnostic.unwrap(diagnostics, compilerOptions));
} }
} }


Expand All @@ -269,12 +302,15 @@ protected final void typeCheckCustomExterns(
} }
if (this.mode.checkTranspiled()) { if (this.mode.checkTranspiled()) {
compilerOptions.setLanguageOut(LanguageMode.ECMASCRIPT5); compilerOptions.setLanguageOut(LanguageMode.ECMASCRIPT5);
// TODO(sdh): stop allowing this option to be false, then we can eliminate this extra check.
compilerOptions.setTypeCheckEs6Natively(false);
typeCheck(externs, js, warningKinds);
compilerOptions.setTypeCheckEs6Natively(true);
typeCheck(externs, js, warningKinds); typeCheck(externs, js, warningKinds);
} }
} }


private final void typeCheck( private void typeCheck(String externs, String js, DiagnosticType... warningKinds) {
String externs, String js, DiagnosticType... warningKinds) {
parseAndTypeCheck(externs, js); parseAndTypeCheck(externs, js);
JSError[] warnings = compiler.getWarnings(); JSError[] warnings = compiler.getWarnings();
JSError[] errors = compiler.getErrors(); JSError[] errors = compiler.getErrors();
Expand Down Expand Up @@ -313,11 +349,15 @@ protected final void typeCheckMessageContents(
} }
if (this.mode.checkTranspiled()) { if (this.mode.checkTranspiled()) {
compilerOptions.setLanguageOut(LanguageMode.ECMASCRIPT5); compilerOptions.setLanguageOut(LanguageMode.ECMASCRIPT5);
// TODO(sdh): stop allowing this option to be false, then we can eliminate this extra check.
compilerOptions.setTypeCheckEs6Natively(false);
typeCheckMessageContentsHelper(js, warningKind, warningMsg);
compilerOptions.setTypeCheckEs6Natively(true);
typeCheckMessageContentsHelper(js, warningKind, warningMsg); typeCheckMessageContentsHelper(js, warningKind, warningMsg);
} }
} }


private final void typeCheckMessageContentsHelper( private void typeCheckMessageContentsHelper(
String js, DiagnosticType warningKind, String warningMsg) { String js, DiagnosticType warningKind, String warningMsg) {
parseAndTypeCheck(DEFAULT_EXTERNS, js); parseAndTypeCheck(DEFAULT_EXTERNS, js);
JSError[] warnings = compiler.getWarnings(); JSError[] warnings = compiler.getWarnings();
Expand All @@ -328,4 +368,59 @@ private final void typeCheckMessageContentsHelper(
assertError(warning).hasType(warningKind); assertError(warning).hasType(warningKind);
assertError(warning).hasMessage(warningMsg); assertError(warning).hasMessage(warningMsg);
} }

/** Abstraction over DiagnosticType that allows warnings to be expected conditionally. */
protected abstract static class Diagnostic implements Function<CompilerOptions, DiagnosticType> {

protected static Diagnostic of(final DiagnosticType type) {
return new Diagnostic() {
@Override
public DiagnosticType apply(CompilerOptions unused) {
return type;
}
};
}

protected static Diagnostic[] wrap(DiagnosticType firstType, DiagnosticType[] types) {
Diagnostic[] out = new Diagnostic[types.length + 1];
out[0] = of(firstType);
for (int i = 0; i < types.length; i++) {
out[i + 1] = of(types[i]);
}
return out;
}

protected static DiagnosticType[] unwrap(Diagnostic[] diagnostics, CompilerOptions options) {
List<DiagnosticType> out = new ArrayList<>();
for (Diagnostic diagnostic : diagnostics) {
DiagnosticType unwrapped = diagnostic.apply(options);
if (unwrapped != null) {
out.add(unwrapped);
}
}
return out.toArray(new DiagnosticType[0]);
}
}

/** A warning that is only expected when type checking ES6 natively. */
protected static Diagnostic nativeEs6Only(final DiagnosticType type) {
return new Diagnostic() {
@Override
public DiagnosticType apply(CompilerOptions options) {
return options.needsTranspilationFrom(FeatureSet.ES6) && options.getTypeCheckEs6Natively()
? type : null;
}
};
}

/** A warning that is only expected when not type checking ES6 natively. */
protected static Diagnostic fullyTranspiledOnly(final DiagnosticType type) {
return new Diagnostic() {
@Override
public DiagnosticType apply(CompilerOptions options) {
return options.needsTranspilationFrom(FeatureSet.ES6) && !options.getTypeCheckEs6Natively()
? type : null;
}
};
}
} }

0 comments on commit f032e1f

Please sign in to comment.