[L] Change in dart/sdk[main]: [DAS, linter] Fixes `no_leading_underscores` fixes when shadowing
0 views
Skip to first unread message
FMorschel (Gerrit)
Aug 3, 2026, 2:03:56 PM (2 days ago) Aug 3
to dart-analys...@google.com, rev...@dartlang.org
FMorschel has uploaded the change for review![]( "Open in Gerrit")
Commit message
[DAS, linter] Fixes `no_leading_underscores` fixes when shadowing
Change-Id: Id3316d99c157e60d9e5bf3152148523156da23ba
Change information
Files:
- M pkg/linter/lib/src/diagnostic.g.dart
- M pkg/linter/lib/src/rules/no_leading_underscores_for_library_prefixes.dart
- M pkg/linter/lib/src/rules/no_leading_underscores_for_local_identifiers.dart
- A pkg/linter/lib/src/util/shadowing.dart
- M pkg/linter/messages.yaml
- M pkg/linter/test/rule_test_support.dart
- M pkg/linter/test/rules/no_leading_underscores_for_library_prefixes_test.dart
- M pkg/linter/test/rules/no_leading_underscores_for_local_identifiers_test.dart
Change size: L
Delta: 8 files changed, 345 insertions(+), 39 deletions(-)
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. |
FMorschel (Gerrit)
Aug 3, 2026, 2:09:52 PM (2 days ago) Aug 3
to Brian Wilkerson, Samuel Rawlins, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson and Samuel Rawlins
FMorschel voted and added 2 comments![]( "Open in Gerrit")
Votes added by FMorschel
| Auto-Submit | +1 |
2 comments
Patchset-level comments
File-level comment, Patchset 2 (Latest):
FMorschel . resolved
I've sent two separate patches so you can clearly see what I did just for the fix correction-sake and what I did for the lint changes.
File pkg/linter/lib/src/util/shadowing.dart
Line 30, Patchset 2 (Latest):
bool isShadowedAtSomeReference(String newName, Element element, AstNode body) {FMorschel . resolved
I'm not sure this should be here (in this new file, at this location) and in what form (standalone function), so whatever suggestions here around that will be appreciated.
Related details
Attention is currently required from:
- Brian Wilkerson
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Aug 3, 2026, 2:57:58 PM (2 days ago) Aug 3
to FMorschel, Brian Wilkerson, Samuel Rawlins, dart-analys...@google.com, rev...@dartlang.org
Attention needed from FMorschel and Samuel Rawlins
Brian Wilkerson voted and added 2 comments![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
| Commit-Queue | +1 |
2 comments
File pkg/analysis_server/lib/src/services/correction/dart/remove_leading_underscore.dart
Line 90, Patchset 2 (Latest):
references.addAll({Brian Wilkerson . unresolved
Why are we adding the entries in this literal map to `references` rather than just assigning the map to `reference`?
File pkg/linter/lib/src/util/shadowing.dart
Line 30, Patchset 2 (Latest):
bool isShadowedAtSomeReference(String newName, Element element, AstNode body) {FMorschel . resolved
I'm not sure this should be here (in this new file, at this location) and in what form (standalone function), so whatever suggestions here around that will be appreciated.
Brian Wilkerson
I don't have a strong opinion about which file this support should be in.
I'm not a fan of top-level functions (preferring to stick to object-oriented solutions), so I'd probably make these extension methods on `AstNode`:
```dart
extension AstNodeExtention on AstNode {
AstNode get enclosingBody { ... }
bool isShadowedInNode(String newName, Element element) { ... }
}
```That isn't a requirement, and I'm not sure it's the _best_ alternative, just what comes to mind.
Related details
Attention is currently required from:
- FMorschel
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
FMorschel (Gerrit)
Aug 3, 2026, 3:03:35 PM (2 days ago) Aug 3
to dart-...@luci-project-accounts.iam.gserviceaccount.com, Brian Wilkerson, Samuel Rawlins, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson and Samuel Rawlins
FMorschel added 2 comments![]( "Open in Gerrit")
File pkg/analysis_server/lib/src/services/correction/dart/remove_leading_underscore.dart
Line 90, Patchset 2 (Latest):
references.addAll({Brian Wilkerson . unresolved
Why are we adding the entries in this literal map to `references` rather than just assigning the map to `reference`?
FMorschel
Oh, thanks! That was a small refactor. I originally had the for loop around the add. I'll wait for the bots to push a new patch for this.
File pkg/linter/lib/src/util/shadowing.dart
Line 30, Patchset 2 (Latest):
bool isShadowedAtSomeReference(String newName, Element element, AstNode body) {FMorschel . resolved
Brian WilkersonI'm not sure this should be here (in this new file, at this location) and in what form (standalone function), so whatever suggestions here around that will be appreciated.
I don't have a strong opinion about which file this support should be in.
I'm not a fan of top-level functions (preferring to stick to object-oriented solutions), so I'd probably make these extension methods on `AstNode`:
```dart
extension AstNodeExtention on AstNode {
AstNode get enclosingBody { ... }bool isShadowedInNode(String newName, Element element) { ... }
}
```That isn't a requirement, and I'm not sure it's the _best_ alternative, just what comes to mind.
FMorschel
Alright, I'll refactor this too. Probably move to `extensions.dart` (I think we have that here in linter too? I'll find out)
Related details
Attention is currently required from:
- Brian Wilkerson
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
FMorschel (Gerrit)
Aug 3, 2026, 5:16:57 PM (2 days ago) Aug 3
to dart-...@luci-project-accounts.iam.gserviceaccount.com, Brian Wilkerson, Samuel Rawlins, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson and Samuel Rawlins
FMorschel voted and added 2 comments![]( "Open in Gerrit")
Votes added by FMorschel
| Auto-Submit | +1 |
2 comments
File pkg/analysis_server/lib/src/services/correction/dart/remove_leading_underscore.dart
FMorschelWhy are we adding the entries in this literal map to `references` rather than just assigning the map to `reference`?
Oh, thanks! That was a small refactor. I originally had the for loop around the add. I'll wait for the bots to push a new patch for this.
FMorschel
Done
File pkg/analysis_server/test/src/services/correction/fix/remove_leading_underscore_test.dart
Line 66, Patchset 2:
Future<void> test_importPrefix_withPart() async {FMorschel . resolved
This was passing on Windows due to new line normalization. I believe my change makes it work correctly on any OS now.
Related details
Attention is currently required from:
- Brian Wilkerson
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Aug 3, 2026, 6:08:53 PM (2 days ago) Aug 3
to FMorschel, Brian Wilkerson, dart-...@luci-project-accounts.iam.gserviceaccount.com, Samuel Rawlins, dart-analys...@google.com, rev...@dartlang.org
Attention needed from FMorschel and Samuel Rawlins
Brian Wilkerson voted![]( "Open in Gerrit")
Attention is currently required from:
- FMorschel
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
FMorschel (Gerrit)
Aug 3, 2026, 8:06:46 PM (2 days ago) Aug 3
to Brian Wilkerson, dart-...@luci-project-accounts.iam.gserviceaccount.com, Samuel Rawlins, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson and Samuel Rawlins
FMorschel voted and added 1 comment![]( "Open in Gerrit")
Votes added by FMorschel
| Auto-Submit | +1 |
1 comment
File pkg/analysis_server/test/src/services/correction/fix/remove_leading_underscore_test.dart
Line 66, Patchset 2:
Future<void> test_importPrefix_withPart() async {FMorschel . resolved
This was passing on Windows due to new line normalization. I believe my change makes it work correctly on any OS now.
FMorschel
Okay, I completely missed using the correct variable in `assertHasFixForTarget`. Now it really should work. Thanks for the bots!
Related details
Attention is currently required from:
- Brian Wilkerson
- Samuel Rawlins
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Aug 4, 2026, 12:00:41 PM (yesterday) Aug 4
to FMorschel, Keerti Parthasarathy, Brian Wilkerson, dart-...@luci-project-accounts.iam.gserviceaccount.com, dart-analys...@google.com, rev...@dartlang.org
Attention needed from FMorschel and Keerti Parthasarathy
Brian Wilkerson voted![]( "Open in Gerrit")
Attention is currently required from:
- FMorschel
- Keerti Parthasarathy
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keerti Parthasarathy (Gerrit)
12:13 PM (7 hours ago) 12:13 PM
to FMorschel, Brian Wilkerson, dart-...@luci-project-accounts.iam.gserviceaccount.com, dart-analys...@google.com, rev...@dartlang.org
Related details
Attention is currently required from:
- FMorschel
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
dart-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
12:42 PM (6 hours ago) 12:42 PM
to FMorschel, Keerti Parthasarathy, Brian Wilkerson, dart-analys...@google.com, rev...@dartlang.org
dart-...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[DAS, linter] Fixes `no_leading_underscores` fixes when shadowing
Change-Id: Id3316d99c157e60d9e5bf3152148523156da23ba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/530700
Reviewed-by: Brian Wilkerson <brianwi...@google.com>
Commit-Queue: Keerti Parthasarathy <kee...@google.com>
Auto-Submit: FMorschel <g...@fmorschel.dev>
Reviewed-by: Keerti Parthasarathy <kee...@google.com>
Files:
- M pkg/analysis_server/lib/src/services/correction/dart/remove_leading_underscore.dart
- M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
- M pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart
- M pkg/analysis_server/test/src/services/correction/fix/remove_leading_underscore_test.dart
- M pkg/linter/lib/src/diagnostic.g.dart
- M pkg/linter/lib/src/extensions.dart
- M pkg/linter/lib/src/rules/no_leading_underscores_for_library_prefixes.dart
- M pkg/linter/lib/src/rules/no_leading_underscores_for_local_identifiers.dart
- M pkg/linter/messages.yaml
- M pkg/linter/test/rule_test_support.dart
- M pkg/linter/test/rules/no_leading_underscores_for_library_prefixes_test.dart
- M pkg/linter/test/rules/no_leading_underscores_for_local_identifiers_test.dart
Change size: L
Delta: 12 files changed, 497 insertions(+), 122 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Keerti Parthasarathy, +1 by Brian Wilkerson
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages