Skip to content

Commit dab4eb0

Browse files
MichaelRFairhurstcommit-bot@chromium.org
authored andcommitted
Add a "legacy type asserter," manage broken test cases
Change-Id: Ic046805c3246ad7e5e7c56fb9188d6ebd197e2fa Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105560 Commit-Queue: Mike Fairhurst <mfairhurst@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
1 parent f69d5dc commit dab4eb0

File tree

6 files changed

+390
-2
lines changed

6 files changed

+390
-2
lines changed

pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import 'package:analyzer/src/dart/constant/utilities.dart';
2020
import 'package:analyzer/src/dart/element/element.dart';
2121
import 'package:analyzer/src/dart/element/handle.dart';
2222
import 'package:analyzer/src/dart/element/inheritance_manager2.dart';
23+
import 'package:analyzer/src/dart/resolver/legacy_type_asserter.dart';
2324
import 'package:analyzer/src/error/codes.dart';
2425
import 'package:analyzer/src/error/inheritance_override.dart';
2526
import 'package:analyzer/src/error/pending_error.dart';
@@ -201,6 +202,9 @@ class LibraryAnalyzer {
201202
}
202203
});
203204
}
205+
206+
assert(units.values.every(LegacyTypeAsserter.assertLegacyTypes));
207+
204208
timerLibraryAnalyzerVerify.stop();
205209

206210
// Return full results.
Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
// Copyright (c) 2019, 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+
import 'dart:collection';
6+
7+
import 'package:analyzer/dart/analysis/features.dart';
8+
import 'package:analyzer/dart/ast/ast.dart';
9+
import 'package:analyzer/dart/ast/visitor.dart';
10+
import 'package:analyzer/dart/element/element.dart';
11+
import 'package:analyzer/dart/element/type.dart';
12+
import 'package:analyzer/src/dart/element/type.dart';
13+
14+
/// A visitor to assert that legacy libraries deal with legacy types.
15+
///
16+
/// Intended to be used via the static method
17+
/// [LegacyTypeAsserter.assertLegacyTypes], inside an `assert()` node.
18+
///
19+
/// Has a defense against being accidentally run outside of an assert statement,
20+
/// but that can be overridden if needed.
21+
///
22+
/// Checks that the static type of every node, as well as the elements of many
23+
/// nodes, have legacy types, and asserts that the legacy types are deep legacy
24+
/// types.
25+
class LegacyTypeAsserter extends GeneralizingAstVisitor {
26+
// TODO(mfairhurst): remove custom equality/hashCode once both use nullability
27+
Set<DartType> visitedTypes = LinkedHashSet<DartType>(
28+
equals: (a, b) =>
29+
a == b &&
30+
(a as TypeImpl).nullabilitySuffix ==
31+
(b as TypeImpl).nullabilitySuffix,
32+
hashCode: (a) =>
33+
a.hashCode * 11 + (a as TypeImpl).nullabilitySuffix.hashCode);
34+
35+
LegacyTypeAsserter({bool requireIsDebug = true}) {
36+
if (requireIsDebug) {
37+
bool isDebug = false;
38+
39+
assert(() {
40+
isDebug = true;
41+
return true;
42+
}());
43+
44+
if (!isDebug) {
45+
throw UnsupportedError(
46+
'Legacy type asserter is being run outside of a debug environment');
47+
}
48+
}
49+
}
50+
51+
@override
52+
visitClassDeclaration(ClassDeclaration node) {
53+
_assertLegacyType(node.element?.type);
54+
super.visitClassDeclaration(node);
55+
}
56+
57+
@override
58+
visitClassMember(ClassMember node) {
59+
final element = node.element;
60+
if (element is ExecutableElement) {
61+
_assertLegacyType(element?.type);
62+
}
63+
super.visitClassMember(node);
64+
}
65+
66+
@override
67+
visitCompilationUnit(CompilationUnit node) {
68+
if (!node.featureSet.isEnabled(Feature.non_nullable)) {
69+
super.visitCompilationUnit(node);
70+
}
71+
}
72+
73+
@override
74+
visitDeclaredIdentifier(DeclaredIdentifier node) {
75+
_assertLegacyType(node.element?.type);
76+
super.visitDeclaredIdentifier(node);
77+
}
78+
79+
@override
80+
visitEnumDeclaration(EnumDeclaration node) {
81+
_assertLegacyType(node.element?.type);
82+
super.visitEnumDeclaration(node);
83+
}
84+
85+
@override
86+
visitExpression(Expression e) {
87+
_assertLegacyType(e.staticType);
88+
super.visitExpression(e);
89+
}
90+
91+
@override
92+
visitFormalParameter(FormalParameter node) {
93+
_assertLegacyType(node.element?.type);
94+
super.visitFormalParameter(node);
95+
}
96+
97+
@override
98+
visitFunctionDeclaration(FunctionDeclaration node) {
99+
_assertLegacyType(node.element?.type);
100+
super.visitFunctionDeclaration(node);
101+
}
102+
103+
@override
104+
visitTypeAnnotation(TypeAnnotation node) {
105+
_assertLegacyType(node.type);
106+
super.visitTypeAnnotation(node);
107+
}
108+
109+
@override
110+
visitTypeName(TypeName node) {
111+
_assertLegacyType(node.type);
112+
super.visitTypeName(node);
113+
}
114+
115+
@override
116+
visitVariableDeclaration(VariableDeclaration node) {
117+
_assertLegacyType(node.element?.type);
118+
super.visitVariableDeclaration(node);
119+
}
120+
121+
void _assertLegacyType(DartType type) {
122+
if (type == null) {
123+
return;
124+
}
125+
126+
if (type.isDynamic || type.isVoid) {
127+
return;
128+
}
129+
130+
if (type.isBottom && type.isDartCoreNull) {
131+
// Never?, which is ok.
132+
//
133+
// Note: we could allow Null? and Null, but we really should be able to
134+
// guarantee that we are only working with Null*, so that's what this
135+
// currently does.
136+
return;
137+
}
138+
139+
if (visitedTypes.contains(type)) {
140+
return;
141+
}
142+
143+
visitedTypes.add(type);
144+
145+
if (type is TypeParameterType) {
146+
_assertLegacyType(type.bound);
147+
} else if (type is InterfaceType) {
148+
type.typeArguments.forEach(_assertLegacyType);
149+
type.typeParameters.map((param) => param.type).forEach(_assertLegacyType);
150+
} else if (type is FunctionType) {
151+
_assertLegacyType(type.returnType);
152+
type.parameters.map((param) => param.type).forEach(_assertLegacyType);
153+
type.typeArguments.forEach(_assertLegacyType);
154+
type.typeParameters.map((param) => param.type).forEach(_assertLegacyType);
155+
}
156+
157+
if ((type as TypeImpl).nullabilitySuffix == NullabilitySuffix.star) {
158+
return;
159+
}
160+
161+
throw AssertionError('Expected all legacy types, but got '
162+
'${(type as TypeImpl).toString(withNullability: true)} '
163+
'(${type.runtimeType})');
164+
}
165+
166+
static bool assertLegacyTypes(CompilationUnit compilationUnit) {
167+
new LegacyTypeAsserter().visitCompilationUnit(compilationUnit);
168+
return true;
169+
}
170+
}

pkg/analyzer/lib/src/generated/testing/ast_test_factory.dart

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'package:analyzer/dart/analysis/features.dart';
56
import 'package:analyzer/dart/ast/ast.dart';
67
import 'package:analyzer/dart/ast/standard_ast_factory.dart';
78
import 'package:analyzer/dart/ast/token.dart';
@@ -287,6 +288,22 @@ class AstTestFactory {
287288
endToken: TokenFactory.tokenFromType(TokenType.EOF),
288289
featureSet: null);
289290

291+
static CompilationUnit compilationUnit9(
292+
{String scriptTag,
293+
List<Directive> directives,
294+
List<CompilationUnitMember> declarations,
295+
FeatureSet featureSet}) =>
296+
astFactory.compilationUnit2(
297+
beginToken: TokenFactory.tokenFromType(TokenType.EOF),
298+
scriptTag:
299+
scriptTag == null ? null : AstTestFactory.scriptTag(scriptTag),
300+
directives: directives == null ? new List<Directive>() : directives,
301+
declarations: declarations == null
302+
? new List<CompilationUnitMember>()
303+
: declarations,
304+
endToken: TokenFactory.tokenFromType(TokenType.EOF),
305+
featureSet: featureSet);
306+
290307
static ConditionalExpression conditionalExpression(Expression condition,
291308
Expression thenExpression, Expression elseExpression) =>
292309
astFactory.conditionalExpression(

0 commit comments

Comments
 (0)