Skip to content

Commit 5c42bb3

Browse files
fshcheglovCommit Queue
authored andcommitted
Ensure named parameter order is preserved when creating top level function element augmentations.
Fix a bug that caused incorrect linking of named formal parameters in `bundle_reader`, and add handling for repeated named formal parameters in `element_builder`. Change-Id: Ie761730b6ce1b500b09eb16d653020d13b6329df Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450424 Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
1 parent e55a46d commit 5c42bb3

File tree

3 files changed

+241
-51
lines changed

3 files changed

+241
-51
lines changed

pkg/analyzer/lib/src/dart/element/element.dart

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3549,22 +3549,46 @@ class FormalParameterFragmentImpl extends VariableFragmentImpl
35493549
}) {
35503550
DeferredResolutionReadingMixin.withoutLoadingResolution(() {
35513551
var firstFormalParameters = getFragments(fragments.first);
3552-
for (var i = 0; i < firstFormalParameters.length; i++) {
3553-
// Side effect: set element for the fragment.
3554-
var first = firstFormalParameters[i];
3555-
switch (first) {
3552+
for (var fragment in firstFormalParameters) {
3553+
switch (fragment) {
35563554
case FieldFormalParameterFragmentImpl():
3557-
FieldFormalParameterElementImpl(first);
3555+
FieldFormalParameterElementImpl(fragment);
35583556
case SuperFormalParameterFragmentImpl():
3559-
SuperFormalParameterElementImpl(first);
3557+
SuperFormalParameterElementImpl(fragment);
35603558
default:
3561-
FormalParameterElementImpl(first);
3559+
FormalParameterElementImpl(fragment);
35623560
}
3563-
fragments.reduce((previous, current) {
3564-
getFragments(previous)[i].addFragment(getFragments(current)[i]);
3565-
return current;
3566-
});
35673561
}
3562+
3563+
var length = firstFormalParameters.length;
3564+
fragments.reduce((previous, current) {
3565+
var previousFragments = getFragments(previous);
3566+
var currentFragments = getFragments(current);
3567+
3568+
var index = 0;
3569+
while (index < previousFragments.length) {
3570+
var previousFragment = previousFragments[index];
3571+
if (!previousFragment.isPositional) {
3572+
break;
3573+
}
3574+
previousFragments[index].addFragment(currentFragments[index]);
3575+
index++;
3576+
}
3577+
3578+
var namedMap = <String, List<FormalParameterFragmentImpl>>{};
3579+
for (var i = index; i < length; i++) {
3580+
var f = currentFragments[i];
3581+
(namedMap[f.name!] ??= <FormalParameterFragmentImpl>[]).add(f);
3582+
}
3583+
3584+
for (var i = index; i < length; i++) {
3585+
var previousParameter = previousFragments[i];
3586+
var currentParameter = namedMap[previousParameter.name]!.removeAt(0);
3587+
previousParameter.addFragment(currentParameter);
3588+
}
3589+
3590+
return current;
3591+
});
35683592
});
35693593
}
35703594
}

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

Lines changed: 52 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -624,8 +624,8 @@ class ElementBuilder {
624624
);
625625

626626
fragment.formalParameters = _linkFormalParameters(
627-
lastFragments: lastFragment.formalParameters,
628-
fragments: fragment.formalParameters,
627+
previousFragments: lastFragment.formalParameters,
628+
currentFragments: fragment.formalParameters,
629629
);
630630
return;
631631
}
@@ -822,78 +822,94 @@ class ElementBuilder {
822822
}
823823

824824
List<FormalParameterFragmentImpl> _linkFormalParameters({
825-
required List<FormalParameterFragmentImpl> lastFragments,
826-
required List<FormalParameterFragmentImpl> fragments,
825+
required List<FormalParameterFragmentImpl> previousFragments,
826+
required List<FormalParameterFragmentImpl> currentFragments,
827827
}) {
828828
int getPositionalSize(List<FormalParameterFragmentImpl> fragments) {
829829
return fragments.takeWhile((f) => f.isPositional).length;
830830
}
831831

832832
FormalParameterFragmentImpl createFragment(
833-
FormalParameterFragmentImpl lastParameter,
833+
FormalParameterFragmentImpl previousParameter,
834834
) {
835-
switch (lastParameter) {
835+
switch (previousParameter) {
836836
case FieldFormalParameterFragmentImpl():
837837
return FieldFormalParameterFragmentImpl(
838-
name: lastParameter.name,
838+
name: previousParameter.name,
839839
nameOffset: null,
840-
parameterKind: lastParameter.parameterKind,
840+
parameterKind: previousParameter.parameterKind,
841841
)..isSynthetic = true;
842842
case SuperFormalParameterFragmentImpl():
843843
return SuperFormalParameterFragmentImpl(
844-
name: lastParameter.name,
844+
name: previousParameter.name,
845845
nameOffset: null,
846-
parameterKind: lastParameter.parameterKind,
846+
parameterKind: previousParameter.parameterKind,
847847
)..isSynthetic = true;
848848
default:
849849
return FormalParameterFragmentImpl(
850-
name: lastParameter.name,
850+
name: previousParameter.name,
851851
nameOffset: null,
852-
parameterKind: lastParameter.parameterKind,
852+
parameterKind: previousParameter.parameterKind,
853853
)..isSynthetic = true;
854854
}
855855
}
856856

857-
var positionalSize = getPositionalSize(fragments);
858-
var positional = fragments.sublist(0, positionalSize);
859-
var named = fragments.sublist(positionalSize);
857+
var currentPositionalSize = getPositionalSize(currentFragments);
858+
var resultPositional = currentFragments.sublist(0, currentPositionalSize);
859+
var currentNamed = currentFragments.sublist(currentPositionalSize);
860860

861-
var lastPositionalSize = getPositionalSize(lastFragments);
862-
var lastPositional = lastFragments.sublist(0, lastPositionalSize);
863-
var lastNamed = lastFragments.sublist(lastPositionalSize);
861+
var previousPositionalSize = getPositionalSize(previousFragments);
862+
var previousPositional = previousFragments.sublist(
863+
0,
864+
previousPositionalSize,
865+
);
866+
var previousNamed = previousFragments.sublist(previousPositionalSize);
864867

865868
// Trim extra positional parameters.
866-
if (lastPositional.length < positional.length) {
867-
positional.length = lastPositional.length;
869+
if (previousPositionalSize < currentPositionalSize) {
870+
resultPositional.length = previousPositional.length;
868871
}
869872

870873
// Synthesize missing positional parameters.
871-
if (lastPositional.length > positional.length) {
872-
for (var i = positional.length; i < lastPositional.length; i++) {
873-
var lastParameter = lastPositional[i];
874-
positional.add(createFragment(lastParameter));
874+
if (previousPositional.length > resultPositional.length) {
875+
for (var i = currentPositionalSize; i < previousPositionalSize; i++) {
876+
var previousParameter = previousPositional[i];
877+
resultPositional.add(createFragment(previousParameter));
875878
}
876879
}
877880

878-
for (var i = 0; i < lastPositional.length; i++) {
879-
lastPositional[i].addFragment(positional[i]);
881+
for (var i = 0; i < resultPositional.length; i++) {
882+
previousPositional[i].addFragment(resultPositional[i]);
880883
}
881884

882-
var newNamed = <FormalParameterFragmentImpl>[];
883-
var namedMap = <String, FormalParameterFragmentImpl>{};
884-
for (var f in named) {
885-
namedMap[f.name!] = f;
885+
var resultNamed = <FormalParameterFragmentImpl>[];
886+
var previousNamedMap = <String, List<FormalParameterFragmentImpl>>{};
887+
for (var previousParameter in previousNamed) {
888+
(previousNamedMap[previousParameter.name!] ??=
889+
<FormalParameterFragmentImpl>[])
890+
.add(previousParameter);
886891
}
887892

888-
for (var lastParameter in lastNamed) {
889-
var formalParameter = namedMap[lastParameter.name];
890-
formalParameter ??= createFragment(lastParameter);
893+
// Link common formal parameters between previous and current fragments.
894+
for (var currentParameter in currentNamed) {
895+
var previousParameters = previousNamedMap[currentParameter.name];
896+
if (previousParameters != null && previousParameters.isNotEmpty) {
897+
var previousParameter = previousParameters.removeAt(0);
898+
previousParameter.addFragment(currentParameter);
899+
resultNamed.add(currentParameter);
900+
}
901+
}
891902

892-
lastParameter.addFragment(formalParameter);
893-
newNamed.add(formalParameter);
903+
// Synthesize missing named parameters.
904+
for (var previousParameters in previousNamedMap.values) {
905+
for (var previousParameter in previousParameters) {
906+
var parameter = createFragment(previousParameter);
907+
previousParameter.addFragment(parameter);
908+
resultNamed.add(parameter);
909+
}
894910
}
895911

896-
return [...positional, ...newNamed];
912+
return [...resultPositional, ...resultNamed];
897913
}
898914

899915
void _linkTypeParameters({

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

Lines changed: 154 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2505,6 +2505,156 @@ library
25052505
''');
25062506
}
25072507

2508+
test_formalParameters_requiredNamed_repeated() async {
2509+
var library = await buildLibrary(r'''
2510+
void foo({required int n1, required int n1}) {}
2511+
augment void foo({required int n1, required int n1}) {}
2512+
''');
2513+
2514+
configuration.withExportScope = true;
2515+
checkElementText(library, r'''
2516+
library
2517+
reference: <testLibrary>
2518+
fragments
2519+
#F0 <testLibraryFragment>
2520+
element: <testLibrary>
2521+
functions
2522+
#F1 foo (nameOffset:5) (firstTokenOffset:0) (offset:5)
2523+
element: <testLibrary>::@function::foo
2524+
nextFragment: #F2
2525+
formalParameters
2526+
#F3 n1 (nameOffset:23) (firstTokenOffset:10) (offset:23)
2527+
element: <testLibrary>::@function::foo::@formalParameter::n1
2528+
nextFragment: #F4
2529+
#F5 n1 (nameOffset:40) (firstTokenOffset:27) (offset:40)
2530+
element: <testLibrary>::@function::foo::@formalParameter::n1
2531+
nextFragment: #F6
2532+
#F2 foo (nameOffset:61) (firstTokenOffset:48) (offset:61)
2533+
element: <testLibrary>::@function::foo
2534+
previousFragment: #F1
2535+
formalParameters
2536+
#F4 n1 (nameOffset:79) (firstTokenOffset:66) (offset:79)
2537+
element: <testLibrary>::@function::foo::@formalParameter::n1
2538+
previousFragment: #F3
2539+
#F6 n1 (nameOffset:96) (firstTokenOffset:83) (offset:96)
2540+
element: <testLibrary>::@function::foo::@formalParameter::n1
2541+
previousFragment: #F5
2542+
functions
2543+
foo
2544+
reference: <testLibrary>::@function::foo
2545+
firstFragment: #F1
2546+
formalParameters
2547+
#E0 requiredNamed n1
2548+
firstFragment: #F3
2549+
type: int
2550+
#E1 requiredNamed n1
2551+
firstFragment: #F5
2552+
type: int
2553+
returnType: void
2554+
exportedReferences
2555+
declared <testLibrary>::@function::foo
2556+
exportNamespace
2557+
foo: <testLibrary>::@function::foo
2558+
''');
2559+
}
2560+
2561+
test_formalParameters_requiredNamed_repeated_12() async {
2562+
var library = await buildLibrary(r'''
2563+
void foo({required int n1) {}
2564+
augment void foo({required int n1, required int n1}}) {}
2565+
''');
2566+
2567+
configuration.withExportScope = true;
2568+
checkElementText(library, r'''
2569+
library
2570+
reference: <testLibrary>
2571+
fragments
2572+
#F0 <testLibraryFragment>
2573+
element: <testLibrary>
2574+
functions
2575+
#F1 foo (nameOffset:5) (firstTokenOffset:0) (offset:5)
2576+
element: <testLibrary>::@function::foo
2577+
nextFragment: #F2
2578+
formalParameters
2579+
#F3 n1 (nameOffset:23) (firstTokenOffset:10) (offset:23)
2580+
element: <testLibrary>::@function::foo::@formalParameter::n1
2581+
nextFragment: #F4
2582+
#F2 foo (nameOffset:43) (firstTokenOffset:30) (offset:43)
2583+
element: <testLibrary>::@function::foo
2584+
previousFragment: #F1
2585+
formalParameters
2586+
#F4 n1 (nameOffset:61) (firstTokenOffset:48) (offset:61)
2587+
element: <testLibrary>::@function::foo::@formalParameter::n1
2588+
previousFragment: #F3
2589+
functions
2590+
foo
2591+
reference: <testLibrary>::@function::foo
2592+
firstFragment: #F1
2593+
formalParameters
2594+
#E0 requiredNamed n1
2595+
firstFragment: #F3
2596+
type: int
2597+
returnType: void
2598+
exportedReferences
2599+
declared <testLibrary>::@function::foo
2600+
exportNamespace
2601+
foo: <testLibrary>::@function::foo
2602+
''');
2603+
}
2604+
2605+
test_formalParameters_requiredNamed_repeated_21() async {
2606+
var library = await buildLibrary(r'''
2607+
void foo({required int n1, required int n1}) {}
2608+
augment void foo({required int n1}) {}
2609+
''');
2610+
2611+
configuration.withExportScope = true;
2612+
checkElementText(library, r'''
2613+
library
2614+
reference: <testLibrary>
2615+
fragments
2616+
#F0 <testLibraryFragment>
2617+
element: <testLibrary>
2618+
functions
2619+
#F1 foo (nameOffset:5) (firstTokenOffset:0) (offset:5)
2620+
element: <testLibrary>::@function::foo
2621+
nextFragment: #F2
2622+
formalParameters
2623+
#F3 n1 (nameOffset:23) (firstTokenOffset:10) (offset:23)
2624+
element: <testLibrary>::@function::foo::@formalParameter::n1
2625+
nextFragment: #F4
2626+
#F5 n1 (nameOffset:40) (firstTokenOffset:27) (offset:40)
2627+
element: <testLibrary>::@function::foo::@formalParameter::n1
2628+
nextFragment: #F6
2629+
#F2 foo (nameOffset:61) (firstTokenOffset:48) (offset:61)
2630+
element: <testLibrary>::@function::foo
2631+
previousFragment: #F1
2632+
formalParameters
2633+
#F4 n1 (nameOffset:79) (firstTokenOffset:66) (offset:79)
2634+
element: <testLibrary>::@function::foo::@formalParameter::n1
2635+
previousFragment: #F3
2636+
#F6 n1 (nameOffset:<null>) (firstTokenOffset:<null>) (offset:61)
2637+
element: <testLibrary>::@function::foo::@formalParameter::n1
2638+
previousFragment: #F5
2639+
functions
2640+
foo
2641+
reference: <testLibrary>::@function::foo
2642+
firstFragment: #F1
2643+
formalParameters
2644+
#E0 requiredNamed n1
2645+
firstFragment: #F3
2646+
type: int
2647+
#E1 requiredNamed n1
2648+
firstFragment: #F5
2649+
type: int
2650+
returnType: void
2651+
exportedReferences
2652+
declared <testLibrary>::@function::foo
2653+
exportNamespace
2654+
foo: <testLibrary>::@function::foo
2655+
''');
2656+
}
2657+
25082658
test_formalParameters_requiredNamed_swapped() async {
25092659
var library = await buildLibrary(r'''
25102660
void foo({required int n2, required int n1}) {}
@@ -2533,12 +2683,12 @@ library
25332683
element: <testLibrary>::@function::foo
25342684
previousFragment: #F1
25352685
formalParameters
2536-
#F4 n2 (nameOffset:79) (firstTokenOffset:66) (offset:79)
2537-
element: <testLibrary>::@function::foo::@formalParameter::n2
2538-
previousFragment: #F3
2539-
#F6 n1 (nameOffset:96) (firstTokenOffset:83) (offset:96)
2686+
#F6 n1 (nameOffset:79) (firstTokenOffset:66) (offset:79)
25402687
element: <testLibrary>::@function::foo::@formalParameter::n1
25412688
previousFragment: #F5
2689+
#F4 n2 (nameOffset:96) (firstTokenOffset:83) (offset:96)
2690+
element: <testLibrary>::@function::foo::@formalParameter::n2
2691+
previousFragment: #F3
25422692
functions
25432693
foo
25442694
reference: <testLibrary>::@function::foo

0 commit comments

Comments
 (0)