Skip to content

Commit

Permalink
Add NullSafetyUnderstandingFlag, update APIs to use it.
Browse files Browse the repository at this point in the history
DartType.== implementations ignore nullability if the flag is not true.

TypeSystem APIs eliminate nullability from types before doing anything.

Internally analyzer continues using actual nullability for types.

Package nnbd_migration opted into NullSafetyUnderstandingFlag.

Bug: #40500
Change-Id: Ifeea28c01adf1dc59ed2da675b4a62c6334d529a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134865
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
  • Loading branch information
scheglov authored and [email protected] committed Feb 7, 2020
1 parent 8d9cbaf commit 6f6797e
Show file tree
Hide file tree
Showing 38 changed files with 398 additions and 232 deletions.
8 changes: 8 additions & 0 deletions pkg/analyzer/lib/dart/element/element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1374,6 +1374,14 @@ abstract class LibraryElement implements Element {
/// Return the class defined in this library that has the given [name], or
/// `null` if this library does not define a class with the given name.
ClassElement getType(String className);

/// If a legacy library, return the legacy view on the [element].
/// Otherwise, return the original element.
T toLegacyElementIfOptOut<T extends Element>(T element);

/// If a legacy library, return the legacy version of the [type].
/// Otherwise, return the original type.
DartType toLegacyTypeIfOptOut(DartType type);
}

/// An element that can be (but is not required to be) defined within a method
Expand Down
44 changes: 44 additions & 0 deletions pkg/analyzer/lib/dart/element/null_safety_understanding_flag.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';

import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';

/// Every [DartType] has the nullability suffix, elements declared in legacy
/// libraries have types with star suffixes, and types in migrated libraries
/// have none, question mark, or star suffixes. Analyzer itself can handle
/// mixtures of nullabilities with legacy and migrated libraries.
///
/// However analyzer clients saw only star suffixes so far. Exposing other
/// nullabilities is a breaking change, because types types with different
/// nullabilities are not equal, `null` cannot be used where a non-nullable
/// type is expected, etc. When accessing elements and types that come from
/// migrated libraries, while analyzing a legacy library, nullabilities must
/// be erased, using [LibraryElement.toLegacyElementIfOptOut] and
/// [LibraryElement.toLegacyTypeIfOptOut]. The client must explicitly do
/// this, and explicitly specify that it knows how to handle nullabilities
/// by setting this flag to `true`.
///
/// When this flag is `false` (by default), all types will return their
/// nullability as star. So, type equality and subtype checks will work
/// as they worked before some libraries migrated. Note, that during
/// analysis (building element models, and resolving ASTs), analyzer will use
/// actual nullabilities, according to the language specification, so report
/// all corresponding errors, and perform necessary type operations. It is
/// only when the client later views on the types, they will look as legacy.
class NullSafetyUnderstandingFlag {
static final _zoneKey = Object();

static bool get isEnabled {
return Zone.current[_zoneKey] ?? false;
}

/// Code that understands nullability should be run using this method,
/// otherwise all type operations will treat all nullabilities as star.
static R enableNullSafetyTypes<R>(R body()) {
return runZoned<R>(body, zoneValues: {_zoneKey: true});
}
}
49 changes: 34 additions & 15 deletions pkg/analyzer/lib/src/dart/analysis/driver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/session.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart' show LibraryElement;
import 'package:analyzer/dart/element/null_safety_understanding_flag.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/error/listener.dart';
import 'package:analyzer/exception/exception.dart';
Expand Down Expand Up @@ -1216,6 +1217,21 @@ class AnalysisDriver implements AnalysisDriverGeneric {
{bool withUnit = false,
bool asIsIfPartWithoutLibrary = false,
bool skipIfSameSignature = false}) {
return NullSafetyUnderstandingFlag.enableNullSafetyTypes(() {
return _computeAnalysisResult2(
path,
withUnit: withUnit,
asIsIfPartWithoutLibrary: asIsIfPartWithoutLibrary,
skipIfSameSignature: skipIfSameSignature,
);
});
}

/// Unwrapped implementation of [_computeAnalysisResult].
AnalysisResult _computeAnalysisResult2(String path,
{bool withUnit = false,
bool asIsIfPartWithoutLibrary = false,
bool skipIfSameSignature = false}) {
FileState file = _fsState.getFileForPath(path);

// Prepare the library - the file itself, or the known library.
Expand Down Expand Up @@ -1476,21 +1492,24 @@ class AnalysisDriver implements AnalysisDriverGeneric {
}
}

if (_libraryContext == null) {
_libraryContext = LibraryContext(
session: currentSession,
logger: _logger,
fsState: fsState,
byteStore: _byteStore,
analysisOptions: _analysisOptions,
declaredVariables: declaredVariables,
sourceFactory: _sourceFactory,
externalSummaries: _externalSummaries,
targetLibrary: library,
);
} else {
_libraryContext.load2(library);
}
NullSafetyUnderstandingFlag.enableNullSafetyTypes(() {
if (_libraryContext == null) {
_libraryContext = LibraryContext(
session: currentSession,
logger: _logger,
fsState: fsState,
byteStore: _byteStore,
analysisOptions: _analysisOptions,
declaredVariables: declaredVariables,
sourceFactory: _sourceFactory,
externalSummaries: _externalSummaries,
targetLibrary: library,
);
} else {
_libraryContext.load2(library);
}
});

return _libraryContext;
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/analyzer/lib/src/dart/constant/evaluation.dart
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class ConstantEvaluationEngine {

/// The type system. This is used to guess the types of constants when their
/// exact value is unknown.
final TypeSystem typeSystem;
final TypeSystemImpl typeSystem;

/// The helper for evaluating variables declared on the command line
/// using '-D', and represented as [DeclaredVariables].
Expand Down Expand Up @@ -109,7 +109,7 @@ class ConstantEvaluationEngine {
}

bool get _isNonNullableByDefault {
return (typeSystem as TypeSystemImpl).isNonNullableByDefault;
return typeSystem.isNonNullableByDefault;
}

DartObjectImpl get _nullObject {
Expand Down Expand Up @@ -937,7 +937,7 @@ class ConstantEvaluationEngine {
return true;
}
var objType = obj.type;
return typeSystem.isSubtypeOf(objType, type);
return typeSystem.isSubtypeOf2(objType, type);
}

/// Determine whether the given string is a valid name for a public symbol
Expand Down Expand Up @@ -1039,7 +1039,7 @@ class ConstantVisitor extends UnifyingAstVisitor<DartObjectImpl> {
ExperimentStatus get experimentStatus => evaluationEngine.experimentStatus;

/// Convenience getter to gain access to the [evaluationEngine]'s type system.
TypeSystem get typeSystem => evaluationEngine.typeSystem;
TypeSystemImpl get typeSystem => evaluationEngine.typeSystem;

/// Convenience getter to gain access to the [evaluationEngine]'s type
/// provider.
Expand Down Expand Up @@ -1211,7 +1211,7 @@ class ConstantVisitor extends UnifyingAstVisitor<DartObjectImpl> {
ParameterizedType elseType = elseResult.type;
return DartObjectImpl.validWithUnknownValue(
typeSystem,
typeSystem.leastUpperBound(thenType, elseType) as ParameterizedType,
typeSystem.getLeastUpperBound(thenType, elseType) as ParameterizedType,
);
}

Expand Down Expand Up @@ -1672,7 +1672,7 @@ class ConstantVisitor extends UnifyingAstVisitor<DartObjectImpl> {
// TODO(brianwilkerson) Figure out why the static type is sometimes null.
DartType staticType = condition.staticType;
if (staticType == null ||
typeSystem.isAssignableTo(staticType, _typeProvider.boolType)) {
typeSystem.isAssignableTo2(staticType, _typeProvider.boolType)) {
// If the static type is not assignable, then we will have already
// reported this error.
// TODO(mfairhurst) get the FeatureSet to suppress this for nnbd too.
Expand Down
4 changes: 2 additions & 2 deletions pkg/analyzer/lib/src/dart/constant/value.dart
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ class DartObjectImpl implements DartObject {
if (isNull) {
return this;
}
if (!typeSystem.isSubtypeOf(type, (castType._state as TypeState)._type)) {
if (!typeSystem.isSubtypeOf2(type, (castType._state as TypeState)._type)) {
throw EvaluationException(
CompileTimeErrorCode.CONST_EVAL_THROWS_EXCEPTION);
}
Expand Down Expand Up @@ -478,7 +478,7 @@ class DartObjectImpl implements DartObject {
state = BoolState.FALSE_STATE;
}
} else {
state = BoolState.from(typeSystem.isSubtypeOf(type, typeType));
state = BoolState.from(typeSystem.isSubtypeOf2(type, typeType));
}
return DartObjectImpl(typeSystem, typeSystem.typeProvider.boolType, state);
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/analyzer/lib/src/dart/element/element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import 'package:analyzer/src/dart/constant/compute.dart';
import 'package:analyzer/src/dart/constant/evaluation.dart';
import 'package:analyzer/src/dart/constant/value.dart';
import 'package:analyzer/src/dart/element/display_string_builder.dart';
import 'package:analyzer/src/dart/element/member.dart';
import 'package:analyzer/src/dart/element/nullability_eliminator.dart';
import 'package:analyzer/src/dart/element/type.dart';
import 'package:analyzer/src/dart/element/type_algebra.dart';
import 'package:analyzer/src/dart/element/type_provider.dart';
Expand Down Expand Up @@ -5662,6 +5664,18 @@ class LibraryElementImpl extends ElementImpl implements LibraryElement {
BooleanArray.set(_resolutionCapabilities, capability.index, value);
}

@override
T toLegacyElementIfOptOut<T extends Element>(T element) {
if (isNonNullableByDefault) return element;
return Member.legacy(element);
}

@override
DartType toLegacyTypeIfOptOut(DartType type) {
if (isNonNullableByDefault) return type;
return NullabilityEliminator.perform(typeProvider, type);
}

@override
void visitChildren(ElementVisitor visitor) {
super.visitChildren(visitor);
Expand Down
4 changes: 2 additions & 2 deletions pkg/analyzer/lib/src/dart/element/top_merge.dart
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,8 @@ class TopMergeHelper {
if (R_isCovariant) {
var T1 = T_parameter.type;
var T2 = S_parameter.type;
var T1_isSubtype = typeSystem.isSubtypeOf(T1, T2);
var T2_isSubtype = typeSystem.isSubtypeOf(T2, T1);
var T1_isSubtype = typeSystem.isSubtypeOf2(T1, T2);
var T2_isSubtype = typeSystem.isSubtypeOf2(T2, T1);
if (T1_isSubtype && T2_isSubtype) {
// if `T1 <: T2` and `T2 <: T1`, then the result is
// `NNBD_TOP_MERGE(T1, T2)`, and it is covariant.
Expand Down
57 changes: 37 additions & 20 deletions pkg/analyzer/lib/src/dart/element/type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:collection';

import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/null_safety_understanding_flag.dart';
import 'package:analyzer/dart/element/nullability_suffix.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/dart/element/type_provider.dart';
Expand Down Expand Up @@ -241,27 +242,35 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
List<TypeParameterElement> get typeParameters => const [] /*TODO(paulberry)*/;

@override
bool operator ==(Object object) {
if (object is FunctionTypeImpl) {
if (typeFormals.length != object.typeFormals.length) {
bool operator ==(Object other) {
if (identical(other, this)) {
return true;
}

if (other is FunctionTypeImpl) {
if (NullSafetyUnderstandingFlag.isEnabled) {
if (other.nullabilitySuffix != nullabilitySuffix) {
return false;
}
}

if (other.typeFormals.length != typeFormals.length) {
return false;
}
// `<T>T -> T` should be equal to `<U>U -> U`
// To test this, we instantiate both types with the same (unique) type
// variables, and see if the result is equal.
if (typeFormals.isNotEmpty) {
List<DartType> freshVariables = FunctionTypeImpl.relateTypeFormals(
this, object, (t, s, _, __) => t == s);
this, other, (t, s, _, __) => t == s);
if (freshVariables == null) {
return false;
}
return instantiate(freshVariables) ==
object.instantiate(freshVariables);
return instantiate(freshVariables) == other.instantiate(freshVariables);
}

return returnType == object.returnType &&
_equalParameters(parameters, object.parameters) &&
nullabilitySuffix == object.nullabilitySuffix;
return other.returnType == returnType &&
_equalParameters(other.parameters, parameters);
}
return false;
}
Expand Down Expand Up @@ -930,14 +939,18 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
(element.library.session as AnalysisSessionImpl).inheritanceManager;

@override
bool operator ==(Object object) {
if (identical(object, this)) {
bool operator ==(Object other) {
if (identical(other, this)) {
return true;
}
if (object is InterfaceTypeImpl) {
return element == object.element &&
TypeImpl.equalArrays(typeArguments, object.typeArguments) &&
nullabilitySuffix == object.nullabilitySuffix;
if (other is InterfaceTypeImpl) {
if (NullSafetyUnderstandingFlag.isEnabled) {
if (other.nullabilitySuffix != nullabilitySuffix) {
return false;
}
}
return other.element == element &&
TypeImpl.equalArrays(other.typeArguments, typeArguments);
}
return false;
}
Expand Down Expand Up @@ -1553,15 +1566,15 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
for (DartType type in types) {
// If any existing type in the bucket is more specific than this type,
// then we can ignore this type.
if (bucket.any((DartType t) => typeSystem.isSubtypeOf(t, type))) {
if (bucket.any((DartType t) => typeSystem.isSubtypeOf2(t, type))) {
continue;
}
// Otherwise, we need to add this type to the bucket and remove any types
// that are less specific than it.
bool added = false;
int i = 0;
while (i < bucket.length) {
if (typeSystem.isSubtypeOf(type, bucket[i])) {
if (typeSystem.isSubtypeOf2(type, bucket[i])) {
if (added) {
if (i < bucket.length - 1) {
bucket[i] = bucket.removeLast();
Expand Down Expand Up @@ -2057,9 +2070,13 @@ class TypeParameterTypeImpl extends TypeImpl implements TypeParameterType {
return true;
}

if (other is TypeParameterTypeImpl &&
other.nullabilitySuffix == nullabilitySuffix &&
other.element == element) {
if (other is TypeParameterTypeImpl && other.element == element) {
if (NullSafetyUnderstandingFlag.isEnabled) {
if (other.nullabilitySuffix != nullabilitySuffix) {
return false;
}
}

// If the same declaration, or the same promoted element.
if (identical(other.element, element)) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ class AssignmentExpressionResolver {
_inferenceHelper.recordStaticType(node, type);

var leftWriteType = _getStaticType2(node.leftHandSide);
if (!_typeSystem.isAssignableTo(type, leftWriteType)) {
if (!_typeSystem.isAssignableTo2(type, leftWriteType)) {
_resolver.errorReporter.reportErrorForNode(
StaticTypeWarningCode.INVALID_ASSIGNMENT,
node.rightHandSide,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ class ExtensionMemberResolver {
if (receiverType.isVoid) {
_errorReporter.reportErrorForNode(
StaticWarningCode.USE_OF_VOID_RESULT, receiverExpression);
} else if (!_typeSystem.isAssignableTo(receiverType, node.extendedType)) {
} else if (!_typeSystem.isAssignableTo2(receiverType, node.extendedType)) {
_errorReporter.reportErrorForNode(
CompileTimeErrorCode.EXTENSION_OVERRIDE_ARGUMENT_NOT_ASSIGNABLE,
receiverExpression,
Expand Down Expand Up @@ -212,7 +212,7 @@ class ExtensionMemberResolver {
var boundType = typeParameters[i].bound;
if (boundType != null) {
boundType = substitution.substituteType(boundType);
if (!_typeSystem.isSubtypeOf(argType, boundType)) {
if (!_typeSystem.isSubtypeOf2(argType, boundType)) {
_errorReporter.reportErrorForNode(
CompileTimeErrorCode.TYPE_ARGUMENT_NOT_MATCHING_BOUNDS,
typeArgumentList.arguments[i],
Expand Down Expand Up @@ -429,7 +429,7 @@ class ExtensionMemberResolver {

/// Ask the type system for a subtype check.
bool _isSubtypeOf(DartType type1, DartType type2) =>
_typeSystem.isSubtypeOf(type1, type2);
_typeSystem.isSubtypeOf2(type1, type2);

List<DartType> _listOfDynamic(List<TypeParameterElement> parameters) {
return List<DartType>.filled(parameters.length, _dynamicType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ class TypeSystemTypeOperations

@override
bool isSubtypeOf(DartType leftType, DartType rightType) {
return typeSystem.isSubtypeOf(leftType, rightType);
return typeSystem.isSubtypeOf2(leftType, rightType);
}

@override
Expand Down
Loading

0 comments on commit 6f6797e

Please sign in to comment.