Skip to content

Commit cc61a43

Browse files
scheglovCommit Queue
authored andcommitted
Elements. Infer type arguments for redirected generic factory constructors.
Fix element model building to infer type arguments for redirected factory constructors of generic classes (both named and unnamed). This brings element construction in line with AST resolution so redirected targets carry the correct type substitutions instead of defaulting to uninstantiated or incorrect types. What changed: - Perform generic inference when resolving the redirect target type in `NamedTypeResolver`, using the enclosing class as context. - Replace the brittle `redirectedConstructor_namedType` tracking with structural detection of redirecting constructor contexts. - Ensure `ResolutionVisitor` provides the enclosing class to the type resolver and visit the redirection directly. - Adjust `AstResolver` ordering so enclosing declaration info is set before resolving constructor children. - Bump `DATA_VERSION` to invalidate stale serialized data. Why: - Previously, only AST resolution performed inference. Element model building skipped it, producing incorrect constructor redirections in summaries and downstream analyses. Aligning both paths fixes those inconsistencies. Impact: - Correct substitutions are recorded for redirected constructors in summaries; no public API changes expected. - Adds targeted tests to prevent regressions (no behavioral changes beyond the intended inference fix). Fixes: flutter/flutter#174503 Bug: flutter/flutter#174503 Change-Id: If231de40de9af15ec14dc26cda28bdc269d9896e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447603 Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Paul Berry <[email protected]>
1 parent c886915 commit cc61a43

File tree

5 files changed

+213
-23
lines changed

5 files changed

+213
-23
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ testFineAfterLibraryAnalyzerHook;
107107
// TODO(scheglov): Clean up the list of implicitly analyzed files.
108108
class AnalysisDriver {
109109
/// The version of data format, should be incremented on every format change.
110-
static const int DATA_VERSION = 526;
110+
static const int DATA_VERSION = 527;
111111

112112
/// The number of exception contexts allowed to write. Once this field is
113113
/// zero, we stop writing any new exception contexts in this process.

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,6 @@ class NamedTypeResolver with ScopeHelpers {
4848
/// If not `null`, a direct child the [WithClause] in the [enclosingClass].
4949
NamedType? withClause_namedType;
5050

51-
/// If not `null`, the [NamedType] of the redirected constructor being
52-
/// resolved, in the [enclosingClass].
53-
NamedType? redirectedConstructor_namedType;
54-
5551
/// If [resolve] finds out that the given [NamedType] with a
5652
/// [PrefixedIdentifier] name is actually the name of a class and the name of
5753
/// the constructor, it rewrites the [ConstructorName] to correctly represent
@@ -266,7 +262,7 @@ class NamedTypeResolver with ScopeHelpers {
266262
}
267263
}
268264

269-
if (identical(node, redirectedConstructor_namedType)) {
265+
if (_ErrorHelper._isRedirectingConstructor(node)) {
270266
return _inferRedirectedConstructor(
271267
element,
272268
dataForTesting: dataForTesting,
@@ -374,9 +370,6 @@ class NamedTypeResolver with ScopeHelpers {
374370
typeArguments: null,
375371
question: null,
376372
)..element = importPrefixElement;
377-
if (identical(node, redirectedConstructor_namedType)) {
378-
redirectedConstructor_namedType = namedType;
379-
}
380373

381374
constructorName.type = namedType;
382375
constructorName.period = importPrefix.period;

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

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,13 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
177177

178178
TypeImpl get _dynamicType => _typeProvider.dynamicType;
179179

180+
/// Set information about enclosing declarations.
181+
void prepareEnclosingDeclarations({
182+
InterfaceElementImpl? enclosingClassElement,
183+
}) {
184+
_namedTypeResolver.enclosingClass = enclosingClassElement;
185+
}
186+
180187
@override
181188
void visitAnnotation(covariant AnnotationImpl node) {
182189
if (_elementWalker == null) {
@@ -371,7 +378,7 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
371378
});
372379
_defineFormalParameters(element.formalParameters);
373380

374-
_resolveRedirectedConstructor(node);
381+
node.redirectedConstructor?.accept(this);
375382
node.initializers.accept(this);
376383
node.body.accept(this);
377384
});
@@ -1692,18 +1699,6 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
16921699
);
16931700
}
16941701

1695-
void _resolveRedirectedConstructor(ConstructorDeclaration node) {
1696-
var redirectedConstructor = node.redirectedConstructor;
1697-
if (redirectedConstructor == null) return;
1698-
1699-
var namedType = redirectedConstructor.type;
1700-
_namedTypeResolver.redirectedConstructor_namedType = namedType;
1701-
1702-
redirectedConstructor.accept(this);
1703-
1704-
_namedTypeResolver.redirectedConstructor_namedType = null;
1705-
}
1706-
17071702
/// Resolves the given [namedType], reports errors if the resulting type
17081703
/// is not valid in the context of the [declaration] and [clause].
17091704
void _resolveType({

pkg/analyzer/lib/src/summary2/ast_resolver.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ class AstResolver {
9191
node.redirectedConstructor?.accept(visitor);
9292
}
9393

94+
_prepareEnclosingDeclarations();
9495
visit(_resolutionVisitor);
9596
visit(_scopeResolverVisitor);
9697

97-
_prepareEnclosingDeclarations();
9898
_flowAnalysis.bodyOrInitializer_enter(node, node.parameters, visit: visit);
9999
visit(_resolverVisitor);
100100
_resolverVisitor.checkIdle();
@@ -119,6 +119,10 @@ class AstResolver {
119119
}
120120

121121
void _prepareEnclosingDeclarations() {
122+
_resolutionVisitor.prepareEnclosingDeclarations(
123+
enclosingClassElement: enclosingClassElement,
124+
);
125+
122126
_resolverVisitor.prepareEnclosingDeclarations(
123127
enclosingClassElement: enclosingClassElement,
124128
enclosingExecutableElement: enclosingExecutableElement,

pkg/analyzer/test/src/summary/elements/class_test.dart

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4377,6 +4377,81 @@ library
43774377
''');
43784378
}
43794379

4380+
test_class_constructor_redirected_factory_named_generic_inference() async {
4381+
var library = await buildLibrary('''
4382+
class A<T, U> implements B<T, U> {
4383+
A.named();
4384+
}
4385+
4386+
class B<T2, U2> {
4387+
factory B() = A.named;
4388+
}
4389+
''');
4390+
checkElementText(library, r'''
4391+
library
4392+
reference: <testLibrary>
4393+
fragments
4394+
#F0 <testLibraryFragment>
4395+
element: <testLibrary>
4396+
classes
4397+
#F1 class A (nameOffset:6) (firstTokenOffset:0) (offset:6)
4398+
element: <testLibrary>::@class::A
4399+
typeParameters
4400+
#F2 T (nameOffset:8) (firstTokenOffset:8) (offset:8)
4401+
element: #E0 T
4402+
#F3 U (nameOffset:11) (firstTokenOffset:11) (offset:11)
4403+
element: #E1 U
4404+
constructors
4405+
#F4 named (nameOffset:39) (firstTokenOffset:37) (offset:39)
4406+
element: <testLibrary>::@class::A::@constructor::named
4407+
typeName: A
4408+
typeNameOffset: 37
4409+
periodOffset: 38
4410+
#F5 class B (nameOffset:57) (firstTokenOffset:51) (offset:57)
4411+
element: <testLibrary>::@class::B
4412+
typeParameters
4413+
#F6 T2 (nameOffset:59) (firstTokenOffset:59) (offset:59)
4414+
element: #E2 T2
4415+
#F7 U2 (nameOffset:63) (firstTokenOffset:63) (offset:63)
4416+
element: #E3 U2
4417+
constructors
4418+
#F8 factory new (nameOffset:<null>) (firstTokenOffset:71) (offset:79)
4419+
element: <testLibrary>::@class::B::@constructor::new
4420+
typeName: B
4421+
typeNameOffset: 79
4422+
classes
4423+
class A
4424+
reference: <testLibrary>::@class::A
4425+
firstFragment: #F1
4426+
typeParameters
4427+
#E0 T
4428+
firstFragment: #F2
4429+
#E1 U
4430+
firstFragment: #F3
4431+
interfaces
4432+
B<T, U>
4433+
constructors
4434+
named
4435+
reference: <testLibrary>::@class::A::@constructor::named
4436+
firstFragment: #F4
4437+
class B
4438+
reference: <testLibrary>::@class::B
4439+
firstFragment: #F5
4440+
typeParameters
4441+
#E2 T2
4442+
firstFragment: #F6
4443+
#E3 U2
4444+
firstFragment: #F7
4445+
constructors
4446+
factory new
4447+
reference: <testLibrary>::@class::B::@constructor::new
4448+
firstFragment: #F8
4449+
redirectedConstructor: ConstructorMember
4450+
baseElement: <testLibrary>::@class::A::@constructor::named
4451+
substitution: {T: T2, U: U2}
4452+
''');
4453+
}
4454+
43804455
test_class_constructor_redirected_factory_named_generic_viaTypeAlias() async {
43814456
var library = await buildLibrary('''
43824457
typedef A<T, U> = C<T, U>;
@@ -4939,6 +5014,129 @@ library
49395014
''');
49405015
}
49415016

5017+
test_class_constructor_redirected_factory_unnamed_generic_inference() async {
5018+
var library = await buildLibrary('''
5019+
class A<T, U> implements B<T, U> {
5020+
A();
5021+
}
5022+
5023+
class B<T2, U2> {
5024+
factory B() = A;
5025+
}
5026+
''');
5027+
checkElementText(library, r'''
5028+
library
5029+
reference: <testLibrary>
5030+
fragments
5031+
#F0 <testLibraryFragment>
5032+
element: <testLibrary>
5033+
classes
5034+
#F1 class A (nameOffset:6) (firstTokenOffset:0) (offset:6)
5035+
element: <testLibrary>::@class::A
5036+
typeParameters
5037+
#F2 T (nameOffset:8) (firstTokenOffset:8) (offset:8)
5038+
element: #E0 T
5039+
#F3 U (nameOffset:11) (firstTokenOffset:11) (offset:11)
5040+
element: #E1 U
5041+
constructors
5042+
#F4 new (nameOffset:<null>) (firstTokenOffset:37) (offset:37)
5043+
element: <testLibrary>::@class::A::@constructor::new
5044+
typeName: A
5045+
typeNameOffset: 37
5046+
#F5 class B (nameOffset:51) (firstTokenOffset:45) (offset:51)
5047+
element: <testLibrary>::@class::B
5048+
typeParameters
5049+
#F6 T2 (nameOffset:53) (firstTokenOffset:53) (offset:53)
5050+
element: #E2 T2
5051+
#F7 U2 (nameOffset:57) (firstTokenOffset:57) (offset:57)
5052+
element: #E3 U2
5053+
constructors
5054+
#F8 factory new (nameOffset:<null>) (firstTokenOffset:65) (offset:73)
5055+
element: <testLibrary>::@class::B::@constructor::new
5056+
typeName: B
5057+
typeNameOffset: 73
5058+
classes
5059+
class A
5060+
reference: <testLibrary>::@class::A
5061+
firstFragment: #F1
5062+
typeParameters
5063+
#E0 T
5064+
firstFragment: #F2
5065+
#E1 U
5066+
firstFragment: #F3
5067+
interfaces
5068+
B<T, U>
5069+
constructors
5070+
new
5071+
reference: <testLibrary>::@class::A::@constructor::new
5072+
firstFragment: #F4
5073+
class B
5074+
reference: <testLibrary>::@class::B
5075+
firstFragment: #F5
5076+
typeParameters
5077+
#E2 T2
5078+
firstFragment: #F6
5079+
#E3 U2
5080+
firstFragment: #F7
5081+
constructors
5082+
factory new
5083+
reference: <testLibrary>::@class::B::@constructor::new
5084+
firstFragment: #F8
5085+
redirectedConstructor: ConstructorMember
5086+
baseElement: <testLibrary>::@class::A::@constructor::new
5087+
substitution: {T: T2, U: U2}
5088+
''');
5089+
}
5090+
5091+
test_class_constructor_redirected_factory_unnamed_generic_inference_self() async {
5092+
var library = await buildLibrary('''
5093+
class A<T> {
5094+
A();
5095+
factory A.redirected() = A;
5096+
}
5097+
''');
5098+
checkElementText(library, r'''
5099+
library
5100+
reference: <testLibrary>
5101+
fragments
5102+
#F0 <testLibraryFragment>
5103+
element: <testLibrary>
5104+
classes
5105+
#F1 class A (nameOffset:6) (firstTokenOffset:0) (offset:6)
5106+
element: <testLibrary>::@class::A
5107+
typeParameters
5108+
#F2 T (nameOffset:8) (firstTokenOffset:8) (offset:8)
5109+
element: #E0 T
5110+
constructors
5111+
#F3 new (nameOffset:<null>) (firstTokenOffset:15) (offset:15)
5112+
element: <testLibrary>::@class::A::@constructor::new
5113+
typeName: A
5114+
typeNameOffset: 15
5115+
#F4 factory redirected (nameOffset:32) (firstTokenOffset:22) (offset:32)
5116+
element: <testLibrary>::@class::A::@constructor::redirected
5117+
typeName: A
5118+
typeNameOffset: 30
5119+
periodOffset: 31
5120+
classes
5121+
class A
5122+
reference: <testLibrary>::@class::A
5123+
firstFragment: #F1
5124+
typeParameters
5125+
#E0 T
5126+
firstFragment: #F2
5127+
constructors
5128+
new
5129+
reference: <testLibrary>::@class::A::@constructor::new
5130+
firstFragment: #F3
5131+
factory redirected
5132+
reference: <testLibrary>::@class::A::@constructor::redirected
5133+
firstFragment: #F4
5134+
redirectedConstructor: ConstructorMember
5135+
baseElement: <testLibrary>::@class::A::@constructor::new
5136+
substitution: {T: T}
5137+
''');
5138+
}
5139+
49425140
test_class_constructor_redirected_factory_unnamed_generic_viaTypeAlias() async {
49435141
var library = await buildLibrary('''
49445142
typedef A<T, U> = C<T, U>;

0 commit comments

Comments
 (0)