Skip to content

Commit

Permalink
[NTI] Change PureFunctionIdentifier to use TypeI.
Browse files Browse the repository at this point in the history
Also, add some missing definitions to test externs and move some externs around.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=144882842
  • Loading branch information
dimvar authored and blickly committed Jan 19, 2017
1 parent 3612621 commit de36c96
Show file tree
Hide file tree
Showing 5 changed files with 374 additions and 301 deletions.
23 changes: 11 additions & 12 deletions src/com/google/javascript/jscomp/PureFunctionIdentifier.java
Expand Up @@ -34,11 +34,11 @@
import com.google.javascript.jscomp.graph.FixedPointGraphTraversal;
import com.google.javascript.jscomp.graph.FixedPointGraphTraversal.EdgeCallback;
import com.google.javascript.jscomp.graph.LinkedDirectedGraph;
import com.google.javascript.rhino.FunctionTypeI;
import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;
import com.google.javascript.rhino.jstype.FunctionType;
import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.TypeI;
import com.google.javascript.rhino.jstype.JSTypeNative;
import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -903,11 +903,11 @@ private void updateSideEffectsFromExtern(Node externFunction, AbstractCompiler c

JSDocInfo info = NodeUtil.getBestJSDocInfo(externFunction);
// Handle externs.
JSType jstype = externFunction.getJSType();
FunctionType functionType = JSType.toMaybeFunctionType(jstype);
TypeI typei = externFunction.getTypeI();
FunctionTypeI functionType = typei == null ? null : typei.toMaybeFunctionType();
if (functionType != null) {
JSType jstypeReturn = functionType.getReturnType();
if (!PureFunctionIdentifier.isLocalValueType(jstypeReturn, compiler)) {
TypeI retType = functionType.getReturnType();
if (!PureFunctionIdentifier.isLocalValueType(retType, compiler)) {
setTaintsReturn();
}
}
Expand Down Expand Up @@ -937,14 +937,13 @@ private void updateSideEffectsFromExtern(Node externFunction, AbstractCompiler c
*
* @return Whether the jstype is something known to be a local value.
*/
private static boolean isLocalValueType(JSType jstype, AbstractCompiler compiler) {
Preconditions.checkNotNull(jstype);
JSType subtype =
jstype.getGreatestSubtype(
(JSType) compiler.getTypeIRegistry().getNativeType(JSTypeNative.OBJECT_TYPE));
private static boolean isLocalValueType(TypeI typei, AbstractCompiler compiler) {
Preconditions.checkNotNull(typei);
TypeI nativeObj = compiler.getTypeIRegistry().getNativeType(JSTypeNative.OBJECT_TYPE);
TypeI subtype = typei.meetWith(nativeObj);
// If the type includes anything related to a object type, don't assume
// anything about the locality of the value.
return subtype.isNoType();
return subtype.isBottom();
}

/**
Expand Down
21 changes: 21 additions & 0 deletions test/com/google/javascript/jscomp/CompilerTestCase.java
Expand Up @@ -217,6 +217,14 @@ public abstract class CompilerTestCase extends TestCase {
"function String(arg) {}",
"/** @param {number} sliceArg */",
"String.prototype.slice = function(sliceArg) {};",
"/**",
" * @this {?String|string}",
" * @param {?} regex",
" * @param {?} str",
" * @param {string=} opt_flags",
" * @return {string}",
" */",
"String.prototype.replace = function(regex, str, opt_flags) {};",
"/** @type {number} */ String.prototype.length;",
"/**",
" * @template T",
Expand Down Expand Up @@ -246,6 +254,19 @@ public abstract class CompilerTestCase extends TestCase {
"Arguments.prototype.length;",
"/**",
" * @constructor",
" * @param {*=} opt_pattern",
" * @param {*=} opt_flags",
" * @return {!RegExp}",
" * @nosideeffects",
" */",
"function RegExp(opt_pattern, opt_flags) {}",
"/**",
" * @param {*} str The string to search.",
" * @return {?Array<string>}",
" */",
"RegExp.prototype.exec = function(str) {};",
"/**",
" * @constructor",
" */",
// TODO(bradfordcsmith): Copy fields for this from es5.js this when we have test cases
// that depend on them.
Expand Down
55 changes: 27 additions & 28 deletions test/com/google/javascript/jscomp/CompilerTypeTestCase.java
Expand Up @@ -30,38 +30,37 @@
*/
abstract class CompilerTypeTestCase extends BaseJSTypeTestCase {

static final String CLOSURE_DEFS =
"var goog = {};" +
"goog.inherits = function(x, y) {};" +
"/** @type {!Function} */ goog.abstractMethod = function() {};" +
"goog.isArray = function(x) {};" +
"goog.isDef = function(x) {};" +
"goog.isFunction = function(x) {};" +
"goog.isNull = function(x) {};" +
"goog.isString = function(x) {};" +
"goog.isObject = function(x) {};" +
"goog.isDefAndNotNull = function(x) {};" +
"goog.array = {};" +
static final String CLOSURE_DEFS = LINE_JOINER.join(
"/** @const */ var goog = {};",
"goog.inherits = function(x, y) {};",
"/** @type {!Function} */ goog.abstractMethod = function() {};",
"goog.isArray = function(x) {};",
"goog.isDef = function(x) {};",
"goog.isFunction = function(x) {};",
"goog.isNull = function(x) {};",
"goog.isString = function(x) {};",
"goog.isObject = function(x) {};",
"goog.isDefAndNotNull = function(x) {};",
"/** @const */ goog.array = {};",
// simplified ArrayLike definition
"/**\n" +
" * @typedef {Array|{length: number}}\n" +
" */\n" +
"goog.array.ArrayLike;" +
"/**\n" +
" * @param {Array.<T>|{length:number}} arr\n" +
" * @param {function(this:S, T, number, goog.array.ArrayLike):boolean} f\n" +
" * @param {S=} opt_obj\n" +
" * @return {!Array.<T>}\n" +
" * @template T,S\n" +
" */" +
"/**",
" * @typedef {Array|{length: number}}",
" */",
"goog.array.ArrayLike;",
"/**",
" * @param {Array.<T>|{length:number}} arr",
" * @param {function(this:S, T, number, goog.array.ArrayLike):boolean} f",
" * @param {S=} opt_obj",
" * @return {!Array.<T>}",
" * @template T,S",
" */",
// return empty array to satisfy return type
"goog.array.filter = function(arr, f, opt_obj){ return []; };" +
"goog.asserts = {};" +
"/** @return {*} */ goog.asserts.assert = function(x) { return x; };";
"goog.array.filter = function(arr, f, opt_obj){ return []; };",
"goog.asserts = {};",
"/** @return {*} */ goog.asserts.assert = function(x) { return x; };");

/** A default set of externs for testing. */
static final String DEFAULT_EXTERNS =
CompilerTestCase.DEFAULT_EXTERNS;
static final String DEFAULT_EXTERNS = CompilerTestCase.DEFAULT_EXTERNS;

protected Compiler compiler;

Expand Down
Expand Up @@ -145,13 +145,6 @@ public abstract class NewTypeInferenceTestBase extends CompilerTypeTestCase {
"function Error(opt_message, opt_file, opt_line) {}",
"/** @type {string} */",
"Error.prototype.stack;",
"/**",
" * @constructor",
" * @param {?=} opt_pattern",
" * @param {?=} opt_flags",
" * @return {!RegExp}",
" */",
"function RegExp(opt_pattern, opt_flags) {}",
"/** @constructor */",
"function Window() {}",
"/** @type {boolean} */",
Expand Down

0 comments on commit de36c96

Please sign in to comment.