Skip to content

Commit e6e94f7

Browse files
johnniwintherCommit Queue
authored andcommitted
[flow_analysis] Don't promote on invalid type
Flow analysis didn't handle invalid type so non-null promotion would occur on declaration and initialization on erroneous code, leading to warnings about null-aware that is likely valid. For instance f(Unresolved o) { // Error: Unresolved is unresolved int? i = o.property; i?.isEven; // Warning about unnecessary null-aware access if (i != null) { // Warning about unnecessary null comparison i.isEven; } } To handle this fully we need to track invalid nullability (if that is even feasible) but for now we change the default to avoid non-null promotion in such cases. This *does* change the kind cascading errors/warnings that we produce. For instance f(Unresolved o) { // Error: Unresolved is unresolved int? i = o.nonNullProperty; i.isEven; // Error for access on int? } but since it probably more likely for code to *not* depend on non-null promotion, this should be less noise for the user. Change-Id: Ia2bc3505a43b52e5151b93a7fee24e95246b4bbc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443320 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Paul Berry <[email protected]> Commit-Queue: Johnni Winther <[email protected]>
1 parent e3d4534 commit e6e94f7

File tree

12 files changed

+156
-2
lines changed

12 files changed

+156
-2
lines changed

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4034,6 +4034,11 @@ class PromotionModel<Type extends Object> {
40344034
List<Type>? candidates = null;
40354035

40364036
void handleTypeOfInterest(Type type) {
4037+
// If the written type is invalid, we assume no promotion.
4038+
if (helper.typeOperations.isInvalidType(writtenType)) {
4039+
return;
4040+
}
4041+
40374042
// The written type must be a subtype of the type.
40384043
if (!helper.typeOperations.isSubtypeOf(writtenType, type)) {
40394044
return;

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis_operations.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ abstract interface class FlowAnalysisTypeOperations<Type extends Object> {
8888
/// Returns `true` if [type] is a reference to a type parameter.
8989
bool isTypeParameterType(Type type);
9090

91+
/// Returns `true` if [type] represents the invalid type, i.e. the type of
92+
/// an invalid expression.
93+
bool isInvalidType(Type type);
94+
9195
/// Computes the nullable form of [type], in other words the least upper bound
9296
/// of [type] and `Null`.
9397
///

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12639,6 +12639,28 @@ main() {
1263912639
]);
1264012640
});
1264112641

12642+
test('Invalid type does not promote on assignment', () {
12643+
// Declared type is `num?`; assigning an invalid type does not promote
12644+
// to `num`.
12645+
var x = Var('x');
12646+
h.run([
12647+
declare(x, initializer: expr('num?')),
12648+
checkNotPromoted(x),
12649+
x.write(expr('error')),
12650+
checkNotPromoted(x),
12651+
]);
12652+
});
12653+
12654+
test('Invalid type does not promote on declaration', () {
12655+
// Declared type is `num?`; initializing an invalid type does not
12656+
// promote to `num`.
12657+
var x = Var('x');
12658+
h.run([
12659+
declare(x, type: 'num?', initializer: expr('error')),
12660+
checkNotPromoted(x),
12661+
]);
12662+
});
12663+
1264212664
test('Untested type is not a type of interest', () {
1264312665
// Declared type is `Object`; assigning an `int` does not promote.
1264412666
var x = Var('x');
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
method1(Unresolved? o) {
6+
int? a = o.foo;
7+
a;
8+
}
9+
10+
method2(Unresolved? o) {
11+
int? a;
12+
a;
13+
a = o.foo;
14+
a;
15+
}

pkg/_fe_analyzer_shared/test/mini_ast.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3298,6 +3298,11 @@ class MiniAstOperations
32983298
return type is PrimaryType && type.isInterfaceType;
32993299
}
33003300

3301+
@override
3302+
bool isInvalidType(SharedTypeView type) {
3303+
return type is InvalidType;
3304+
}
3305+
33013306
@override
33023307
bool isKnownType(SharedTypeSchemaView typeSchema) {
33033308
var unwrapped = typeSchema.unwrapTypeSchemaView<Type>();

pkg/_fe_analyzer_shared/test/mini_types.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1285,6 +1285,11 @@ class TypeSystem {
12851285
// types with a single name and no type arguments (this covers both
12861286
// primitive types and type variables).
12871287
switch ((t0, t1)) {
1288+
case (InvalidType(), _):
1289+
case (_, InvalidType()):
1290+
// `InvalidType` is treated as a top and a bottom type, which is
1291+
// consistent with CFE and analyzer implementations.
1292+
return true;
12881293
case (
12891294
PrimaryType(nameInfo: var t0Info, isQuestionType: false, args: []),
12901295
PrimaryType(nameInfo: var t1Info, isQuestionType: false, args: []),
@@ -1316,7 +1321,7 @@ class TypeSystem {
13161321
if (_isTop(t1)) return true;
13171322

13181323
// Left Top: if T0 is dynamic or void then T0 <: T1 if Object? <: T1
1319-
if (t0 is DynamicType || t0 is InvalidType || t0 is VoidType) {
1324+
if (t0 is DynamicType || t0 is VoidType) {
13201325
return isSubtype(_objectQuestionType, t1);
13211326
}
13221327

@@ -1355,7 +1360,6 @@ class TypeSystem {
13551360
// false).
13561361
if (t0 is NullType ||
13571362
t0 is DynamicType ||
1358-
t0 is InvalidType ||
13591363
t0 is VoidType ||
13601364
t0.isQuestionType) {
13611365
return false;

pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,11 @@ class TypeSystemOperations
664664
type.element is! ExtensionTypeElement;
665665
}
666666

667+
@override
668+
bool isInvalidType(SharedTypeView type) {
669+
return type.unwrapTypeView<TypeImpl>() is InvalidType;
670+
}
671+
667672
@override
668673
bool isKnownType(SharedTypeSchemaView typeSchema) {
669674
return UnknownInferredType.isKnown(

pkg/analyzer/test/id_tests/type_promotion_test.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ class _TypePromotionDataComputer extends DataComputer<DartType> {
3939
DataInterpreter<DartType> get dataValidator =>
4040
const _TypePromotionDataInterpreter();
4141

42+
@override
43+
bool get supportsErrors => true;
44+
4245
@override
4346
void computeUnitData(
4447
TestingData testingData,

pkg/analyzer/test/src/diagnostics/invalid_null_aware_operator_test.dart

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,37 @@ f(List<int>? x) {
326326
''');
327327
}
328328

329+
test_invalid_nonNullable() async {
330+
await assertErrorsInCode(
331+
'''
332+
f(Unresolved o) {
333+
int? i = o.nonNull;
334+
i.isEven;
335+
}
336+
''',
337+
[
338+
error(CompileTimeErrorCode.UNDEFINED_CLASS, 2, 10),
339+
error(
340+
CompileTimeErrorCode.UNCHECKED_PROPERTY_ACCESS_OF_NULLABLE_VALUE,
341+
44,
342+
6,
343+
),
344+
],
345+
);
346+
}
347+
348+
test_invalid_nullable() async {
349+
await assertErrorsInCode(
350+
'''
351+
f(Unresolved o) {
352+
int? i = o.nullable;
353+
i?.isEven;
354+
}
355+
''',
356+
[error(CompileTimeErrorCode.UNDEFINED_CLASS, 2, 10)],
357+
);
358+
}
359+
329360
test_method_class() async {
330361
await assertErrorsInCode(
331362
'''

pkg/analyzer/test/src/diagnostics/unnecessary_null_comparison_test.dart

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,32 @@ f() {
8686

8787
@reflectiveTest
8888
class UnnecessaryNullComparisonTrueTest extends PubPackageResolutionTest {
89+
test_equal_invalid_nonNull() async {
90+
await assertErrorsInCode(
91+
'''
92+
f(Unresolved o) {
93+
int? i = o.nonNull;
94+
i == null;
95+
null == i;
96+
}
97+
''',
98+
[error(CompileTimeErrorCode.UNDEFINED_CLASS, 2, 10)],
99+
);
100+
}
101+
102+
test_equal_invalid_nullable() async {
103+
await assertErrorsInCode(
104+
'''
105+
f(Unresolved o) {
106+
int? i = o.nullable;
107+
i == null;
108+
null == i;
109+
}
110+
''',
111+
[error(CompileTimeErrorCode.UNDEFINED_CLASS, 2, 10)],
112+
);
113+
}
114+
89115
test_notEqual_intLiteral() async {
90116
await assertNoErrorsInCode('''
91117
f(int a, int? b) {
@@ -97,6 +123,32 @@ f(int a, int? b) {
97123
''');
98124
}
99125

126+
test_notEqual_invalid_nonNull() async {
127+
await assertErrorsInCode(
128+
'''
129+
f(Unresolved o) {
130+
int? i = o.nonNull;
131+
i != null;
132+
null != i;
133+
}
134+
''',
135+
[error(CompileTimeErrorCode.UNDEFINED_CLASS, 2, 10)],
136+
);
137+
}
138+
139+
test_notEqual_invalid_nullable() async {
140+
await assertErrorsInCode(
141+
'''
142+
f(Unresolved o) {
143+
int? i = o.nullable;
144+
i != null;
145+
null != i;
146+
}
147+
''',
148+
[error(CompileTimeErrorCode.UNDEFINED_CLASS, 2, 10)],
149+
);
150+
}
151+
100152
test_notEqual_notNullable() async {
101153
await assertErrorsInCode(
102154
'''

0 commit comments

Comments
 (0)