[S] Change in dart/sdk[main]: [DAS] Fixes rename local variable for function expression cases
0 views
Skip to first unread message
Felipe Morschel (Gerrit)
Sep 5, 2025, 12:36:56 PMSep 5
to dart-analys...@google.com, rev...@dartlang.org
Felipe Morschel has uploaded the change for review![]( "Open in Gerrit")
Commit message
[DAS] Fixes rename local variable for function expression cases
Change-Id: Ib3adade497ce928f0327912c3574b67da4d4be42
Change information
Files:
- M pkg/analysis_server/test/services/refactoring/legacy/rename_local_test.dart
- M pkg/analyzer/lib/src/dart/analysis/search.dart
- M pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
Change size: S
Delta: 3 files changed, 25 insertions(+), 1 deletion(-)
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Morschel (Gerrit)
Sep 5, 2025, 12:40:43 PMSep 5
to Samuel Rawlins, Konstantin Shcheglov, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Konstantin Shcheglov and Samuel Rawlins
Felipe Morschel added 2 comments![]( "Open in Gerrit")
File pkg/analyzer/lib/src/dart/analysis/search.dart
Line 417, Patchset 1 (Latest):
return _searchReferences_Local(element, (n) => n is Block, searchedFiles);Felipe Morschel . unresolved
Maybe we are missing it here too?
Line 427, Patchset 1 (Latest):
n is Block ||Felipe Morschel . unresolved
And here? Does anyone know a test case for these?
Related details
Attention is currently required from:
- Konstantin Shcheglov
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Sep 5, 2025, 12:47:23 PMSep 5
to Felipe Morschel, Konstantin Shcheglov, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Felipe Morschel and Konstantin Shcheglov
Samuel Rawlins voted and added 2 comments![]( "Open in Gerrit")
Votes added by Samuel Rawlins
| Code-Review | +1 |
2 comments
File pkg/analyzer/lib/src/dart/analysis/search.dart
Line 417, Patchset 1 (Latest):
return _searchReferences_Local(element, (n) => n is Block, searchedFiles);Felipe Morschel . unresolved
Maybe we are missing it here too?
Samuel Rawlins
Hmm is this something we could do with a self-executing closure?
```
int foo() => (){
int bar() => 7;
return b^ar();
}();
```
Line 427, Patchset 1 (Latest):
n is Block ||Felipe Morschel . unresolved
And here? Does anyone know a test case for these?
Attention is currently required from:
- Felipe Morschel
- Konstantin Shcheglov
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Morschel (Gerrit)
Sep 5, 2025, 1:12:39 PMSep 5
to Samuel Rawlins, Konstantin Shcheglov, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Konstantin Shcheglov
Felipe Morschel added 2 comments![]( "Open in Gerrit")
File pkg/analyzer/lib/src/dart/analysis/search.dart
Line 417, Patchset 1 (Latest):
return _searchReferences_Local(element, (n) => n is Block, searchedFiles);Felipe Morschel . resolved
Samuel RawlinsMaybe we are missing it here too?
Hmm is this something we could do with a self-executing closure?
```
int foo() => (){
int bar() => 7;
return b^ar();
}();
```
Felipe Morschel
Well, I i'm guessing we can't do it today. We can't have something like:
```dart
int foo() => (other() {return 0;})();
// ^ named_function_expression
```
And the case you provided contains a block around it (tested manually and it works). So I'm closing both comments since I don't think we can have a similar case for other members except pattern variables.
Line 427, Patchset 1 (Latest):
n is Block ||Felipe Morschel . resolved
Samuel RawlinsAnd here? Does anyone know a test case for these?
Same self-executing closure trick?
Attention is currently required from:
- Konstantin Shcheglov
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Konstantin Shcheglov (Gerrit)
Sep 5, 2025, 2:57:21 PMSep 5
to Felipe Morschel, Commit Queue, Samuel Rawlins, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Felipe Morschel
Konstantin Shcheglov added 3 comments![]( "Open in Gerrit")
File pkg/analysis_server/test/services/refactoring/legacy/rename_local_test.dart
Line 273, Patchset 1 (Latest):
Future<void> test_createChange_forElement_functionExpression() async {Konstantin Shcheglov . unresolved
`expressionFunctionBody`
Because `functionExpression` is something else.
We already have `test_createChange_patternVariable_ifCase_patternField_implicitName`. And it looks to me that we test something very related. So, it would be better to put them close by.
File pkg/analyzer/lib/src/dart/analysis/search.dart
Line 985, Patchset 1 (Latest):
node is ExpressionFunctionBody,Konstantin Shcheglov . unresolved
Please review `test_searchReferences_VariablePatternElement_*` tests and add necessary test(s).
File pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
Line 546, Patchset 1 (Latest):
Iterable<MapEntry<K, V>> get entries;Konstantin Shcheglov . unresolved
Move it to other getters.
Related details
Attention is currently required from:
- Felipe Morschel
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Morschel (Gerrit)
Sep 16, 2025, 9:36:51 AMSep 16
to Commit Queue, Samuel Rawlins, Konstantin Shcheglov, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Konstantin Shcheglov and Samuel Rawlins
Felipe Morschel added 3 comments![]( "Open in Gerrit")
File pkg/analysis_server/test/services/refactoring/legacy/rename_local_test.dart
Line 273, Patchset 1 (Latest):
Future<void> test_createChange_forElement_functionExpression() async {Konstantin Shcheglov . resolved
`expressionFunctionBody`
Because `functionExpression` is something else.
We already have `test_createChange_patternVariable_ifCase_patternField_implicitName`. And it looks to me that we test something very related. So, it would be better to put them close by.
Felipe Morschel
Oh yes, thanks!
File pkg/analyzer/lib/src/dart/analysis/search.dart
Line 985, Patchset 1 (Latest):
node is ExpressionFunctionBody,Konstantin Shcheglov . resolved
Please review `test_searchReferences_VariablePatternElement_*` tests and add necessary test(s).
Felipe Morschel
Done
File pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
Line 546, Patchset 1 (Latest):
Iterable<MapEntry<K, V>> get entries;Konstantin Shcheglov . resolved
Move it to other getters.
Felipe Morschel
Done. May I ask why do we use this instead of the actual SDK for these tests specifically?
Related details
Attention is currently required from:
- Konstantin Shcheglov
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Sep 18, 2025, 11:41:29 AMSep 18
to Felipe Morschel, Commit Queue, Konstantin Shcheglov, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Felipe Morschel and Konstantin Shcheglov
Samuel Rawlins voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Felipe Morschel
- Konstantin Shcheglov
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Morschel (Gerrit)
Oct 29, 2025, 7:43:18 AM (3 days ago) Oct 29
to Paul Berry, Konstantin Shcheglov, Samuel Rawlins, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Paul Berry
Felipe Morschel voted and added 1 comment![]( "Open in Gerrit")
Votes added by Felipe Morschel
| Auto-Submit | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Felipe Morschel . resolved
Since @sche...@google.com is busy with `Fine`, I'm moving him to CC and replacing the reviewer with @paul...@google.com. If you disagree with this, please feel free to assign anyone you prefer.
Related details
Attention is currently required from:
- Paul Berry
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Berry (Gerrit)
Oct 29, 2025, 4:05:51 PM (2 days ago) Oct 29
to Felipe Morschel, Konstantin Shcheglov, Samuel Rawlins, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Felipe Morschel
Paul Berry voted and added 1 comment![]( "Open in Gerrit")
Votes added by Paul Berry
| Code-Review | +1 |
| Commit-Queue | +2 |
1 comment
Patchset-level comments
Attention is currently required from:
- Felipe Morschel
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Morschel (Gerrit)
Oct 29, 2025, 4:07:58 PM (2 days ago) Oct 29
to Paul Berry, Konstantin Shcheglov, Samuel Rawlins, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Paul Berry
Felipe Morschel voted and added 1 comment![]( "Open in Gerrit")
Votes added by Felipe Morschel
| Auto-Submit | +1 |
Paul Berry . resolved
lgtm, thank you!
Felipe Morschel
I've rebased using the Gerrit CLI to avoid losing the reviews. Can you try and restart that failing bot please? Thanks!
Related details
Attention is currently required from:
- Paul Berry
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Morschel (Gerrit)
Oct 29, 2025, 4:08:20 PM (2 days ago) Oct 29
to Paul Berry, Konstantin Shcheglov, Samuel Rawlins, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Paul Berry
Felipe Morschel added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Paul Berry . resolved
Felipe Morschellgtm, thank you!
I've rebased using the Gerrit CLI to avoid losing the reviews. Can you try and restart that failing bot please? Thanks!
Felipe Morschel
Gerrit UI**
Related details
Attention is currently required from:
- Paul Berry
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Morschel (Gerrit)
Oct 30, 2025, 7:49:50 AM (2 days ago) Oct 30
to Paul Berry, Konstantin Shcheglov, Samuel Rawlins, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Paul Berry and Samuel Rawlins
Felipe Morschel added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 3 (Latest):
Felipe Morschel . unresolved
Can either of you land this, please? Thanks a lot! I've rebased after the g3 failure.
Related details
Attention is currently required from:
- Paul Berry
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samuel Rawlins (Gerrit)
Oct 30, 2025, 11:05:12 AM (2 days ago) Oct 30
to Felipe Morschel, Paul Berry, Konstantin Shcheglov, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Felipe Morschel and Paul Berry
Samuel Rawlins voted Commit-Queue+2![]( "Open in Gerrit")
Attention is currently required from:
- Felipe Morschel
- Paul Berry
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit Queue (Gerrit)
Oct 30, 2025, 11:05:28 AM (2 days ago) Oct 30
to Felipe Morschel, Samuel Rawlins, Paul Berry, Konstantin Shcheglov, dart-analys...@google.com, rev...@dartlang.org
Commit Queue submitted the change![]( "Open in Gerrit")
Unreviewed changes
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
[DAS] Fixes rename local variable for function expression cases
Change-Id: Ib3adade497ce928f0327912c3574b67da4d4be42
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448642
Reviewed-by: Samuel Rawlins <sraw...@google.com>
Commit-Queue: Samuel Rawlins <sraw...@google.com>
Reviewed-by: Paul Berry <paul...@google.com>
Auto-Submit: Felipe Morschel <g...@fmorschel.dev>
Files:
- M pkg/analysis_server/test/services/refactoring/legacy/rename_local_test.dart
- M pkg/analyzer/lib/src/dart/analysis/search.dart
- M pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
- M pkg/analyzer/test/src/dart/analysis/search_test.dart
Change size: S
Delta: 4 files changed, 39 insertions(+), 1 deletion(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Samuel Rawlins, +1 by Paul Berry
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages