housecleaning: remove nonnullableTypes

the non-nullable by default DEP seems a better way to go here

R=vsm@google.com

Review URL: https://codereview.chromium.org/1396993002 .
This commit is contained in:
John Messerly 2015-10-09 13:14:17 -07:00
parent e00744f753
commit 16c2c4681b
8 changed files with 18 additions and 249 deletions

View file

@ -626,13 +626,7 @@ class CodeChecker extends RecursiveAstVisitor {
var parameterType = rules.elementType(parameter.element);
assert(parameterType != null);
var defaultValue = node.defaultValue;
if (defaultValue == null) {
if (rules.maybeNonNullableType(parameterType)) {
var staticInfo = new InvalidVariableDeclaration(
rules, node.identifier, parameterType);
_recordMessage(staticInfo);
}
} else {
if (defaultValue != null) {
checkAssignment(defaultValue, parameterType);
}
@ -683,19 +677,6 @@ class CodeChecker extends RecursiveAstVisitor {
var initializer = variable.initializer;
if (initializer != null) {
checkAssignment(initializer, dartType);
} else if (rules.maybeNonNullableType(dartType)) {
var element = variable.element;
if (element is FieldElement && !element.isStatic) {
// Initialized - possibly implicitly - during construction.
// Handle this via a runtime check during code generation.
// TODO(vsm): Detect statically whether this can fail and
// report a static error (must fail) or warning (can fail).
} else {
var staticInfo =
new InvalidVariableDeclaration(rules, variable, dartType);
_recordMessage(staticInfo);
}
}
}
}
@ -856,11 +837,14 @@ class CodeChecker extends RecursiveAstVisitor {
case TokenType.STAR_EQ:
case TokenType.TILDE_SLASH_EQ:
case TokenType.PERCENT_EQ:
if (t1 == rules.provider.intType && t2 == rules.provider.intType) return t1;
if (t1 == rules.provider.doubleType && t2 == rules.provider.doubleType) return t1;
if (t1 == rules.provider.intType &&
t2 == rules.provider.intType) return t1;
if (t1 == rules.provider.doubleType &&
t2 == rules.provider.doubleType) return t1;
// This particular combo is not spelled out in the spec, but all
// implementations and analyzer seem to follow this.
if (t1 == rules.provider.doubleType && t2 == rules.provider.intType) return t1;
if (t1 == rules.provider.doubleType &&
t2 == rules.provider.intType) return t1;
}
return normalReturnType;
}

View file

@ -32,9 +32,6 @@ abstract class TypeRules {
{bool fuzzyArrows: true, bool ignoreReturn: false}) =>
isSubTypeOf(f1, f2);
bool isNonNullableType(DartType t) => false;
bool maybeNonNullableType(DartType t) => false;
StaticInfo checkAssignment(Expression expr, DartType t);
DartType getStaticType(Expression expr) => expr.staticType;
@ -126,31 +123,9 @@ abstract class TypeRules {
class RestrictedRules extends TypeRules {
final StrongModeOptions options;
final List<DartType> _nonnullableTypes;
DownwardsInference inferrer;
DartType _typeFromName(String name) {
switch (name) {
case 'int':
return provider.intType;
case 'double':
return provider.doubleType;
case 'num':
return provider.numType;
case 'bool':
return provider.boolType;
case 'String':
return provider.stringType;
default:
throw new UnsupportedError('Unsupported non-nullable type $name');
}
}
RestrictedRules(TypeProvider provider, {this.options})
: _nonnullableTypes = <DartType>[],
super(provider) {
var types = options.nonnullableTypes;
_nonnullableTypes.addAll(types.map(_typeFromName));
RestrictedRules(TypeProvider provider, {this.options}) : super(provider) {
inferrer = new DownwardsInference(this);
}
@ -162,7 +137,7 @@ class RestrictedRules extends TypeRules {
if (t.isDynamic && dynamicIsBottom) return true;
// TODO(vsm): We need direct support for non-nullability in DartType.
// This should check on "true/nonnullable" Bottom
if (t.isBottom && _nonnullableTypes.isEmpty) return true;
if (t.isBottom) return true;
return false;
}
@ -172,25 +147,6 @@ class RestrictedRules extends TypeRules {
return false;
}
bool isNonNullableType(DartType t) => _nonnullableTypes.contains(t);
bool maybeNonNullableType(DartType t) {
// Return true iff t *may* be a primitive type.
// If t is a generic type parameter, return true if it may be
// instantiated as a primitive.
if (isNonNullableType(t)) {
return true;
} else if (t is TypeParameterType) {
var bound = t.element.bound;
if (bound == null) {
bound = provider.dynamicType;
}
return _nonnullableTypes.any((DartType p) => isSubTypeOf(p, bound));
} else {
return false;
}
}
bool _anyParameterType(FunctionType ft, bool predicate(DartType t)) {
return ft.normalParameterTypes.any(predicate) ||
ft.optionalParameterTypes.any(predicate) ||
@ -343,13 +299,12 @@ class RestrictedRules extends TypeRules {
return false;
}
// The null type is a subtype of any nonnullable type.
// The null type is a subtype of any nullable type, which is all Dart types.
// TODO(vsm): Note, t1.isBottom still allows for null confusingly.
// _isBottom(t1) does not necessarily imply t1.isBottom if there are
// nonnullable types in the system.
if (t1.isBottom) {
// Return false iff t2 *may* be a primitive type.
return !maybeNonNullableType(t2);
return true;
}
// S <: T where S is a type variable

View file

@ -325,18 +325,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
return js.call('dart.asInt(#)', [fromExpr]);
}
if (!rules.isNonNullableType(from) && rules.isNonNullableType(to)) {
// Converting from a nullable number to a non-nullable number
// only requires a null check.
// TODO(jmesserly): a lot of these checks are meaningless, as people use
// `num` to mean "any kind of number" rather than "could be null".
// The core libraries especially suffer from this problem, with many of
// the `num` methods returning `num`.
return js.call('dart.notNull(#)', fromExpr);
} else {
// A no-op in JavaScript.
return fromExpr;
}
// A no-op in JavaScript.
return fromExpr;
}
return js.call('dart.as(#, #)', [fromExpr, _emitTypeName(to)]);
@ -1126,11 +1116,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
if (fieldNode.initializer != null) {
value = _visit(fieldNode.initializer);
} else {
var type = rules.elementType(element);
value = new JS.LiteralNull();
if (rules.maybeNonNullableType(type)) {
value = js.call('dart.as(#, #)', [value, _emitTypeName(type)]);
}
}
fields[element] = value;
});
@ -2175,18 +2161,12 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
bool typeIsPrimitiveInJS(DartType t) =>
_isNumberInJS(t) || t == _types.boolType;
bool typeIsNonNullablePrimitiveInJS(DartType t) =>
typeIsPrimitiveInJS(t) && rules.isNonNullableType(t);
bool binaryOperationIsPrimitive(DartType leftT, DartType rightT) =>
typeIsPrimitiveInJS(leftT) && typeIsPrimitiveInJS(rightT);
bool unaryOperationIsPrimitive(DartType t) => typeIsPrimitiveInJS(t);
bool _isNonNullableExpression(Expression expr) {
// If the type is non-nullable, no further checking needed.
if (rules.isNonNullableType(getStaticType(expr))) return true;
// TODO(vsm): Revisit whether we really need this when we get
// better non-nullability in the type system.
// TODO(jmesserly): we do recursive calls in a few places. This could

View file

@ -222,15 +222,8 @@ abstract class DownCast extends CoercionInfo {
// Handle null call specially.
if (expression is NullLiteral) {
if (rules.isNonNullableType(toT)) {
reason = "null is invalid as a $toT";
return new StaticTypeError(rules, expression, toT, reason: reason);
} else {
// We should only get here if some coercion is required.
assert(rules.maybeNonNullableType(toT));
// TODO(vsm): Create a NullCast for this once we revisit nonnullability.
return new DownCastImplicit(rules, expression, cast);
}
// TODO(vsm): Create a NullCast for this once we revisit nonnullability.
return new DownCastImplicit(rules, expression, cast);
}
// Inference "casts":

View file

@ -108,10 +108,6 @@ class StrongModeOptions {
/// Whether to inject casts between Dart assignable types.
final bool relaxedCasts;
/// A list of non-nullable type names (e.g., 'int')
final List<String> nonnullableTypes;
static const List<String> NONNULLABLE_TYPES = const <String>[];
/// Whether to include hints about dynamic invokes and runtime checks.
// TODO(jmesserly): this option is not used yet by DDC server mode or batch
// compile to JS.
@ -122,16 +118,13 @@ class StrongModeOptions {
this.inferTransitively: inferTransitivelyDefault,
this.onlyInferConstsAndFinalFields: onlyInferConstAndFinalFieldsDefault,
this.inferDownwards: inferDownwardsDefault,
this.relaxedCasts: true,
this.nonnullableTypes: StrongModeOptions.NONNULLABLE_TYPES});
this.relaxedCasts: true});
StrongModeOptions.fromArguments(ArgResults args, {String prefix: ''})
: relaxedCasts = args[prefix + 'relaxed-casts'],
inferDownwards = args[prefix + 'infer-downwards'],
inferTransitively = args[prefix + 'infer-transitively'],
onlyInferConstsAndFinalFields = args[prefix + 'infer-only-finals'],
nonnullableTypes = _optionsToList(args[prefix + 'nonnullable'],
defaultValue: StrongModeOptions.NONNULLABLE_TYPES),
hints = args[prefix + 'hints'];
static ArgParser addArguments(ArgParser parser,
@ -170,19 +163,6 @@ class StrongModeOptions {
return inferTransitively == s.inferTransitively &&
onlyInferConstsAndFinalFields == s.onlyInferConstsAndFinalFields &&
inferDownwards == s.inferDownwards &&
relaxedCasts == s.relaxedCasts &&
nonnullableTypes.length == s.nonnullableTypes.length &&
new Set.from(nonnullableTypes).containsAll(s.nonnullableTypes);
}
}
List<String> _optionsToList(String option,
{List<String> defaultValue: const <String>[]}) {
if (option == null) {
return defaultValue;
} else if (option.isEmpty) {
return <String>[];
} else {
return option.split(',');
relaxedCasts == s.relaxedCasts;
}
}

View file

@ -207,103 +207,6 @@ void main() {
'''
});
testChecker('Primitives', {
'/main.dart': '''
int /*severe:InvalidVariableDeclaration*/a;
double /*severe:InvalidVariableDeclaration*/b;
num c;
class A {
int a;
double b;
num c;
static int /*severe:InvalidVariableDeclaration*/x;
static double /*severe:InvalidVariableDeclaration*/y;
static num z;
}
void foo(int w, [int x = /*severe:StaticTypeError*/null, int /*severe:InvalidVariableDeclaration*/y, int z = 0]) {
}
void bar(int w, {int x = /*severe:StaticTypeError*/null, int /*severe:InvalidVariableDeclaration*/y, int z: 0}) {
}
void main() {
int /*severe:InvalidVariableDeclaration*/x;
double /*severe:InvalidVariableDeclaration*/y;
num z;
bool b;
// int is non-nullable
x = /*severe:StaticTypeError*/null;
x = 42;
x = /*info:DownCastImplicit*/z;
// double is non-nullable
y = /*severe:StaticTypeError*/null;
y = /*severe:StaticTypeError*/42;
y = 42.0;
y = /*info:DownCastImplicit*/z;
// num is nullable
z = null;
z = x;
z = y;
// bool is nullable
b = null;
b = true;
}
'''
}, nonnullableTypes: <String>[
'int',
'double'
]);
testChecker('Primitives and generics', {
'/main.dart': '''
class A<T> {
// TODO(vsm): This needs a static info indicating a runtime
// check at construction.
T x;
// TODO(vsm): Should this be a different type of DownCast?
T foo() => /*info:DownCastImplicit*/null;
void bar() {
int /*severe:InvalidVariableDeclaration*/x;
num y;
// TODO(vsm): This should be a runtime check:
// Transformed to: T z = cast(null, T)
T /*severe:InvalidVariableDeclaration*/z;
}
void baz(T x, [T /*severe:InvalidVariableDeclaration*/y, T z = /*info:DownCastImplicit*/null]) {
}
}
class B<T extends List> {
T x;
// T cannot be primitive.
T foo() => null;
}
class C<T extends num> {
// TODO(vsm): This needs a static info indicating a runtime
// check at construction.
T x;
// TODO(vsm): Should this be a different type of DownCast?
T foo() => /*info:DownCastImplicit*/null;
}
'''
}, nonnullableTypes: <String>[
'int',
'double'
]);
testChecker('Constructors', {
'/main.dart': '''
const num z = 25;

View file

@ -30,30 +30,6 @@ void main() {
'''
});
testChecker('Error when declared type is `int` and assigned null.', {
'/main.dart': '''
test1() {
int x = 3;
x = /*severe:StaticTypeError*/null;
}
'''
}, nonnullableTypes: <String>[
'int',
'double'
]);
testChecker('Error when inferred type is `int` and assigned null.', {
'/main.dart': '''
test1() {
var x = 3;
x = /*severe:StaticTypeError*/null;
}
'''
}, nonnullableTypes: <String>[
'int',
'double'
]);
testChecker('No error when declared type is `num` and assigned null.', {
'/main.dart': '''
test1() {

View file

@ -77,8 +77,7 @@ void testChecker(String name, Map<String, String> testFiles,
customUrlMappings: const {},
relaxedCasts: true,
inferDownwards: StrongModeOptions.inferDownwardsDefault,
inferTransitively: StrongModeOptions.inferTransitivelyDefault,
nonnullableTypes: StrongModeOptions.NONNULLABLE_TYPES}) {
inferTransitively: StrongModeOptions.inferTransitivelyDefault}) {
test(name, () {
expect(testFiles.containsKey('/main.dart'), isTrue,
reason: '`/main.dart` is missing in testFiles');
@ -102,7 +101,6 @@ void testChecker(String name, Map<String, String> testFiles,
relaxedCasts: relaxedCasts,
inferDownwards: inferDownwards,
inferTransitively: inferTransitively,
nonnullableTypes: nonnullableTypes,
hints: true));
// Run the checker on /main.dart.