[M] Change in dart/sdk[main]: analyzer: Report unused fields with origin in a primary constructor
0 views
Skip to first unread message
Brian Wilkerson (Gerrit)
Dec 15, 2025, 5:32:55 PM (6 days ago) Dec 15
to Samuel Rawlins, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Samuel Rawlins
Brian Wilkerson voted and added 3 comments![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
3 comments
Patchset-level comments
File-level comment, Patchset 7 (Latest):
Brian Wilkerson . resolved
I can't think of what purpose an extension type would serve, if its representation field is never referenced.
I can't either, but if the type implements the representation type then members on `this` can be implicitly invoked. Not sure whether that gets counted as a reference by our implementation.
File pkg/analyzer/messages.yaml
Line 30568, Patchset 7 (Latest):
correctionMessage: Try removing the '' keyword to avoid declaring a field, or try using the field, or removing it.Brian Wilkerson . unresolved
Is this intended to have a parameter containing the keyword?
Line 30591, Patchset 7 (Latest):
If the parameter is still necessary, but the associated field is not, tryBrian Wilkerson . unresolved
nit: "try" --> "then"
Also, consider including the example code with the fix applied (for all three fixes).
Related details
Attention is currently required from:
- 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)
Dec 15, 2025, 5:40:55 PM (6 days ago) Dec 15
to Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Samuel Rawlins added 2 comments![]( "Open in Gerrit")
File pkg/analyzer/messages.yaml
Line 30568, Patchset 7:
correctionMessage: Try removing the '' keyword to avoid declaring a field, or try using the field, or removing it.Brian Wilkerson . resolved
Is this intended to have a parameter containing the keyword?
Samuel Rawlins
Haha it was! Thanks for catching; I've added some logic.
Line 30591, Patchset 7:
If the parameter is still necessary, but the associated field is not, tryBrian Wilkerson . resolved
nit: "try" --> "then"
Also, consider including the example code with the fix applied (for all three fixes).
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)
Dec 15, 2025, 5:48:04 PM (6 days ago) Dec 15
to Samuel Rawlins, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Samuel Rawlins
Brian Wilkerson voted and added 1 comment![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
Attention is currently required from:
- Samuel Rawlins
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)
Dec 17, 2025, 2:55:15 PM (4 days ago) Dec 17
to Samuel Rawlins, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Samuel Rawlins
Paul Berry added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 9 (Latest):
Paul Berry . resolved
lgtm. Your call on whether to address my code review comment.
File pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart
Line 716, Patchset 9 (Latest):
// We take a hands-off approach to reporting unused private representationPaul Berry . unresolved
This comment isn't clear to me in isolation. Looking at the commit message, I think what you're saying is that we don't report an extension type representation field as unused because (a) it's not likely to arise in practice, and (b) it's not possible to remove the unused field. Consider saying this more directly.
Related details
Attention is currently required from:
- 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)
Dec 17, 2025, 8:13:23 PM (4 days ago) Dec 17
to Paul Berry, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Paul Berry
Samuel Rawlins added 1 comment![]( "Open in Gerrit")
File pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart
Line 716, Patchset 9:
// We take a hands-off approach to reporting unused private representationPaul Berry . resolved
This comment isn't clear to me in isolation. Looking at the commit message, I think what you're saying is that we don't report an extension type representation field as unused because (a) it's not likely to arise in practice, and (b) it's not possible to remove the unused field. Consider saying this more directly.
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)
Dec 18, 2025, 1:06:38 PM (3 days ago) Dec 18
to Samuel Rawlins, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Samuel Rawlins
Paul Berry voted and added 1 comment![]( "Open in Gerrit")
Votes added by Paul Berry
| Code-Review | +1 |
Paul Berry . resolved
lgtm
Attention is currently required from:
- 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)
Dec 18, 2025, 2:26:08 PM (3 days ago) Dec 18
to Paul Berry, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Samuel Rawlins voted Commit-Queue+2![]( "Open in Gerrit")
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)
Dec 18, 2025, 2:26:24 PM (3 days ago) Dec 18
to Samuel Rawlins, Paul Berry, Brian Wilkerson, dart-analys...@google.com, rev...@dartlang.org
Commit Queue submitted the change![]( "Open in Gerrit")
Change information
Commit message:
analyzer: Report unused fields with origin in a primary constructor
Work towards https://github.com/dart-lang/sdk/issues/62248
In this change, we exclude fields declared in a primary constructor
which is declared in an extension type. Since the representation field
is required, the corrective action is unclear. (I can't think of what
purpose an extension type would serve, if its representation field is
never referenced. But users are creative.)
Change-Id: I9556f5d28a16dc7114b30c77369281774bc7c2fd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/467924
Reviewed-by: Brian Wilkerson <brianwi...@google.com>
Reviewed-by: Paul Berry <paul...@google.com>
Commit-Queue: Samuel Rawlins <sraw...@google.com>
Files:
- M pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
- M pkg/analyzer/lib/src/diagnostic/diagnostic.g.dart
- M pkg/analyzer/lib/src/diagnostic/diagnostic_code_values.g.dart
- M pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart
- M pkg/analyzer/messages.yaml
- M pkg/analyzer/test/src/diagnostics/test_all.dart
- A pkg/analyzer/test/src/diagnostics/unused_field_from_primary_constructor_test.dart
Change size: M
Delta: 7 files changed, 181 insertions(+), 6 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Brian Wilkerson, +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