Skip to content

Commit

Permalink
[_fe_analyzer_shared] Avoid deconstruction of potentially nullable ty…
Browse files Browse the repository at this point in the history
…pe variables

The exhaustiveness model relies on null being extracted from nullable, for instance modeling `int?` as `int|Null`. This failed on type variable that are potentially nullable but not explicitly nullable. For these `Null` should be extract from their bound and not from themselves.

The logic in the getStaticType method now uses `isNullable` to determine both when to extract and to combine, so to avoid extracting but not combining `Null` for type variables and also not combine but not extract for instance for `void`.

Closes #56998

Change-Id: I47c2d0d6535ca66fe00e6344b11550c4308a7388
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392944
Reviewed-by: Chloe Stefantsova <[email protected]>
Commit-Queue: Johnni Winther <[email protected]>
  • Loading branch information
johnniwinther authored and Commit Queue committed Nov 1, 2024
1 parent 297c90b commit 58c336e
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 17 deletions.
39 changes: 24 additions & 15 deletions pkg/_fe_analyzer_shared/lib/src/exhaustiveness/shared.dart
Original file line number Diff line number Diff line change
Expand Up @@ -185,49 +185,57 @@ class ExhaustivenessCache<
}

StaticType staticType;
Type nonNullable = typeOperations.getNonNullable(type);
if (typeOperations.isBoolType(nonNullable)) {
bool extractNull = typeOperations.isNullable(type);
Type typeWithoutNull = type;
if (extractNull) {
// If [type] is nullable, we model the static type by creating the
// non-nullable equivalent and then add `Null` afterwards.
//
// For instance we model `int?` as `int|Null`.
typeWithoutNull = typeOperations.getNonNullable(type);
}
if (typeOperations.isBoolType(typeWithoutNull)) {
staticType = _boolStaticType;
} else if (typeOperations.isRecordType(nonNullable)) {
staticType = new RecordStaticType(typeOperations, this, nonNullable);
} else if (typeOperations.isRecordType(typeWithoutNull)) {
staticType = new RecordStaticType(typeOperations, this, typeWithoutNull);
} else {
Type? futureOrTypeArgument =
typeOperations.getFutureOrTypeArgument(nonNullable);
typeOperations.getFutureOrTypeArgument(typeWithoutNull);
if (futureOrTypeArgument != null) {
StaticType typeArgument = getStaticType(futureOrTypeArgument);
StaticType futureType = getStaticType(
typeOperations.instantiateFuture(futureOrTypeArgument));
bool isImplicitlyNullable =
typeOperations.isNullable(futureOrTypeArgument);
staticType = new FutureOrStaticType(
typeOperations, this, nonNullable, typeArgument, futureType,
typeOperations, this, typeWithoutNull, typeArgument, futureType,
isImplicitlyNullable: isImplicitlyNullable);
} else {
EnumClass? enumClass = enumOperations.getEnumClass(nonNullable);
EnumClass? enumClass = enumOperations.getEnumClass(typeWithoutNull);
if (enumClass != null) {
staticType = new EnumStaticType(
typeOperations, this, nonNullable, _getEnumInfo(enumClass));
typeOperations, this, typeWithoutNull, _getEnumInfo(enumClass));
} else {
Class? sealedClass =
_sealedClassOperations.getSealedClass(nonNullable);
_sealedClassOperations.getSealedClass(typeWithoutNull);
if (sealedClass != null) {
staticType = new SealedClassStaticType(
typeOperations,
this,
nonNullable,
typeWithoutNull,
this,
_sealedClassOperations,
_getSealedClassInfo(sealedClass));
} else {
Type? listType = typeOperations.getListType(nonNullable);
Type? listType = typeOperations.getListType(typeWithoutNull);
if (listType != null) {
staticType =
new ListTypeStaticType(typeOperations, this, nonNullable);
new ListTypeStaticType(typeOperations, this, typeWithoutNull);
} else {
bool isImplicitlyNullable =
typeOperations.isNullable(nonNullable);
typeOperations.isNullable(typeWithoutNull);
staticType = new TypeBasedStaticType(
typeOperations, this, nonNullable,
typeOperations, this, typeWithoutNull,
isImplicitlyNullable: isImplicitlyNullable);
Type? bound = typeOperations.getTypeVariableBound(type);
if (bound != null) {
Expand All @@ -239,7 +247,8 @@ class ExhaustivenessCache<
}
}
}
if (typeOperations.isNullable(type)) {
if (extractNull) {
// Include the `Null` which extracted from [type] into [typeWithoutNull`.
staticType = staticType.nullable;
}
return staticType;
Expand Down
38 changes: 38 additions & 0 deletions pkg/_fe_analyzer_shared/test/exhaustiveness/data/issue56998.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) 2024, 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.

int nonExhaustive1<T extends int?>(T value) =>
/*
checkingOrder={int?,int,Null},
error=non-exhaustive:null,
subtypes={int,Null},
type=int?
*/switch (value) { int() /*space=int*/ => value };

int nonExhaustive2<T extends int?>(
T? value) => /*
checkingOrder={int?,int,Null},
error=non-exhaustive:null,
subtypes={int,Null},
type=int?
*/
switch (value) {
int() /*space=int*/ => value
};

int exhaustive<T extends int>(T value) => /*type=int*/
switch (value) {
int() /*space=int*/ => value
};

int nonExhaustive3<T extends int>(
T? value) => /*
checkingOrder={int?,int,Null},
error=non-exhaustive:null,
subtypes={int,Null},
type=int?
*/
switch (value) {
int() /*space=int*/ => value
};
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ exhaustiveC(C c) => /*
type=C
*/switch (c) { C(: num _a) /*space=C(_a: num)*/=> 0, }

nonExhaustiveA(C c) => /*analyzer.
nonExhaustiveC(C c) => /*
error=non-exhaustive:C(_a: double()),
fields={_a:num},
type=C
*/switch (c) { C(: int _a) /*analyzer.space=C(_a: int)*/=> 0, }
*/switch (c) { C(: int _a) /*space=C(_a: int)*/=> 0, }

0 comments on commit 58c336e

Please sign in to comment.