Skip to content

Commit c25790e

Browse files
Add @protected to public State method overrides (flutter#157313)
I don't want to see these methods in my autofill suggestions. <br> ![navigator dispose](https://github.com/user-attachments/assets/5d2dc37c-abe9-44f4-ad16-4396a3aeeaf7)
1 parent ff47d63 commit c25790e

26 files changed

+223
-5
lines changed

dev/bots/analyze.dart

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import 'custom_rules/analyze.dart';
2222
import 'custom_rules/avoid_future_catcherror.dart';
2323
import 'custom_rules/no_double_clamp.dart';
2424
import 'custom_rules/no_stop_watches.dart';
25+
import 'custom_rules/protect_public_state_subtypes.dart';
2526
import 'custom_rules/render_box_intrinsics.dart';
2627
import 'run_command.dart';
2728
import 'utils.dart';
@@ -180,7 +181,12 @@ Future<void> run(List<String> arguments) async {
180181
// Only run the private lints when the code is free of type errors. The
181182
// lints are easier to write when they can assume, for example, there is no
182183
// inheritance cycles.
183-
final List<AnalyzeRule> rules = <AnalyzeRule>[noDoubleClamp, noStopwatches, renderBoxIntrinsicCalculation];
184+
final List<AnalyzeRule> rules = <AnalyzeRule>[
185+
noDoubleClamp,
186+
noStopwatches,
187+
renderBoxIntrinsicCalculation,
188+
protectPublicStateSubtypes,
189+
];
184190
final String ruleNames = rules.map((AnalyzeRule rule) => '\n * $rule').join();
185191
printProgress('Analyzing code in the framework with the following rules:$ruleNames');
186192
await analyzeWithRules(flutterRoot, rules,
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// TODO(nate-thegrate): remove this file if @protected changes, or add a test if it doesn't.
6+
// https://github.com/dart-lang/sdk/issues/57094
7+
8+
import 'package:analyzer/dart/analysis/results.dart';
9+
import 'package:analyzer/dart/ast/ast.dart';
10+
import 'package:analyzer/dart/ast/visitor.dart';
11+
import 'package:analyzer/dart/element/element.dart';
12+
import 'package:analyzer/dart/element/type.dart';
13+
14+
import '../utils.dart';
15+
import 'analyze.dart';
16+
17+
final AnalyzeRule protectPublicStateSubtypes = _ProtectPublicStateSubtypes();
18+
19+
class _ProtectPublicStateSubtypes implements AnalyzeRule {
20+
final Map<ResolvedUnitResult, List<MethodDeclaration>> _errors = <ResolvedUnitResult, List<MethodDeclaration>>{};
21+
22+
@override
23+
void applyTo(ResolvedUnitResult unit) {
24+
final _StateSubclassVisitor visitor = _StateSubclassVisitor();
25+
unit.unit.visitChildren(visitor);
26+
final List<MethodDeclaration> unprotected = visitor.unprotectedMethods;
27+
if (unprotected.isNotEmpty) {
28+
_errors.putIfAbsent(unit, () => <MethodDeclaration>[]).addAll(unprotected);
29+
}
30+
}
31+
32+
@override
33+
void reportViolations(String workingDirectory) {
34+
if (_errors.isEmpty) {
35+
return;
36+
}
37+
38+
foundError(
39+
<String>[
40+
for (final MapEntry<ResolvedUnitResult, List<MethodDeclaration>> entry in _errors.entries)
41+
for (final MethodDeclaration method in entry.value)
42+
'${locationInFile(entry.key, method, workingDirectory)}: $method - missing "@protected" annotation.',
43+
'\nPublic State subtypes should add @protected when overriding methods,',
44+
'to avoid exposing internal logic to developers.',
45+
],
46+
);
47+
}
48+
49+
@override
50+
String toString() => 'Add "@protected" to public State subtypes';
51+
}
52+
53+
class _StateSubclassVisitor extends SimpleAstVisitor<void> {
54+
final List<MethodDeclaration> unprotectedMethods = <MethodDeclaration>[];
55+
56+
/// Holds the `State` class [DartType].
57+
static DartType? stateType;
58+
59+
static bool isPublicStateSubtype(InterfaceElement element) {
60+
if (!element.isPublic) {
61+
return false;
62+
}
63+
if (stateType != null) {
64+
return element.allSupertypes.contains(stateType);
65+
}
66+
for (final InterfaceType superType in element.allSupertypes) {
67+
if (superType.element.name == 'State') {
68+
stateType = superType;
69+
return true;
70+
}
71+
}
72+
return false;
73+
}
74+
75+
@override
76+
void visitClassDeclaration(ClassDeclaration node) {
77+
if (isPublicStateSubtype(node.declaredElement!)) {
78+
node.visitChildren(this);
79+
}
80+
}
81+
82+
/// Checks whether overridden `State` methods have the `@protected` annotation,
83+
/// and adds the declaration to [unprotectedMethods] if not.
84+
@override
85+
void visitMethodDeclaration(MethodDeclaration node) {
86+
switch (node.name.lexeme) {
87+
case 'initState':
88+
case 'didUpdateWidget':
89+
case 'didChangeDependencies':
90+
case 'reassemble':
91+
case 'deactivate':
92+
case 'activate':
93+
case 'dispose':
94+
case 'build':
95+
case 'debugFillProperties':
96+
if (!node.declaredElement!.hasProtected) {
97+
unprotectedMethods.add(node);
98+
}
99+
}
100+
}
101+
}

packages/flutter/lib/src/material/drawer.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,7 @@ class DrawerController extends StatefulWidget {
468468
///
469469
/// Typically used by a [Scaffold] to [open] and [close] the drawer.
470470
class DrawerControllerState extends State<DrawerController> with SingleTickerProviderStateMixin {
471+
@protected
471472
@override
472473
void initState() {
473474
super.initState();
@@ -481,6 +482,7 @@ class DrawerControllerState extends State<DrawerController> with SingleTickerPro
481482
..addStatusListener(_animationStatusChanged);
482483
}
483484

485+
@protected
484486
@override
485487
void dispose() {
486488
_historyEntry?.remove();
@@ -489,12 +491,14 @@ class DrawerControllerState extends State<DrawerController> with SingleTickerPro
489491
super.dispose();
490492
}
491493

494+
@protected
492495
@override
493496
void didChangeDependencies() {
494497
super.didChangeDependencies();
495498
_scrimColorTween = _buildScrimColorTween();
496499
}
497500

501+
@protected
498502
@override
499503
void didUpdateWidget(DrawerController oldWidget) {
500504
super.didUpdateWidget(oldWidget);
@@ -751,6 +755,7 @@ class DrawerControllerState extends State<DrawerController> with SingleTickerPro
751755
}
752756
}
753757

758+
@protected
754759
@override
755760
Widget build(BuildContext context) {
756761
assert(debugCheckHasMaterialLocalizations(context));

packages/flutter/lib/src/material/paginated_data_table.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
331331
int _selectedRowCount = 0;
332332
final Map<int, DataRow?> _rows = <int, DataRow?>{};
333333

334+
@protected
334335
@override
335336
void initState() {
336337
super.initState();
@@ -339,6 +340,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
339340
_handleDataSourceChanged();
340341
}
341342

343+
@protected
342344
@override
343345
void didUpdateWidget(PaginatedDataTable oldWidget) {
344346
super.didUpdateWidget(oldWidget);
@@ -349,6 +351,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
349351
}
350352
}
351353

354+
@protected
352355
@override
353356
void reassemble() {
354357
super.reassemble();
@@ -366,6 +369,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
366369
_updateCaches();
367370
}
368371

372+
@protected
369373
@override
370374
void dispose() {
371375
widget.source.removeListener(_handleDataSourceChanged);
@@ -468,6 +472,7 @@ class PaginatedDataTableState extends State<PaginatedDataTable> {
468472

469473
final GlobalKey _tableKey = GlobalKey();
470474

475+
@protected
471476
@override
472477
Widget build(BuildContext context) {
473478
// TODO(ianh): This whole build function doesn't handle RTL yet.

packages/flutter/lib/src/material/popup_menu.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
356356
widget.onTap?.call();
357357
}
358358

359+
@protected
359360
@override
360361
Widget build(BuildContext context) {
361362
final ThemeData theme = Theme.of(context);
@@ -1544,6 +1545,7 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
15441545
};
15451546
}
15461547

1548+
@protected
15471549
@override
15481550
Widget build(BuildContext context) {
15491551
final IconThemeData iconTheme = IconTheme.of(context);

packages/flutter/lib/src/material/refresh_indicator.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ class RefreshIndicatorState extends State<RefreshIndicator>
352352
end: 0.0,
353353
);
354354

355+
@protected
355356
@override
356357
void initState() {
357358
super.initState();
@@ -365,12 +366,14 @@ class RefreshIndicatorState extends State<RefreshIndicator>
365366
_scaleFactor = _scaleController.drive(_oneToZeroTween);
366367
}
367368

369+
@protected
368370
@override
369371
void didChangeDependencies() {
370372
_setupColorTween();
371373
super.didChangeDependencies();
372374
}
373375

376+
@protected
374377
@override
375378
void didUpdateWidget(covariant RefreshIndicator oldWidget) {
376379
super.didUpdateWidget(oldWidget);
@@ -379,6 +382,7 @@ class RefreshIndicatorState extends State<RefreshIndicator>
379382
}
380383
}
381384

385+
@protected
382386
@override
383387
void dispose() {
384388
_positionController.dispose();
@@ -636,6 +640,7 @@ class RefreshIndicatorState extends State<RefreshIndicator>
636640
return _pendingRefreshFuture;
637641
}
638642

643+
@protected
639644
@override
640645
Widget build(BuildContext context) {
641646
assert(debugCheckHasMaterialLocalizations(context));

packages/flutter/lib/src/material/scaffold.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ class ScaffoldMessengerState extends State<ScaffoldMessenger> with TickerProvide
197197
Timer? _snackBarTimer;
198198
bool? _accessibleNavigation;
199199

200+
@protected
200201
@override
201202
void didChangeDependencies() {
202203
final bool accessibleNavigation = MediaQuery.accessibleNavigationOf(context);
@@ -604,6 +605,7 @@ class ScaffoldMessengerState extends State<ScaffoldMessenger> with TickerProvide
604605
hideCurrentMaterialBanner();
605606
}
606607

608+
@protected
607609
@override
608610
Widget build(BuildContext context) {
609611
assert(debugCheckHasMediaQuery(context));
@@ -632,6 +634,7 @@ class ScaffoldMessengerState extends State<ScaffoldMessenger> with TickerProvide
632634
);
633635
}
634636

637+
@protected
635638
@override
636639
void dispose() {
637640
_materialBannerController?.dispose();
@@ -2116,6 +2119,7 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
21162119
@override
21172120
String? get restorationId => widget.restorationId;
21182121

2122+
@protected
21192123
@override
21202124
void restoreState(RestorationBucket? oldBucket, bool initialRestore) {
21212125
registerForRestoration(_drawerOpened, 'drawer_open');
@@ -2676,6 +2680,7 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
26762680
return widget.resizeToAvoidBottomInset ?? true;
26772681
}
26782682

2683+
@protected
26792684
@override
26802685
void initState() {
26812686
super.initState();
@@ -2695,6 +2700,7 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
26952700
);
26962701
}
26972702

2703+
@protected
26982704
@override
26992705
void didUpdateWidget(Scaffold oldWidget) {
27002706
super.didUpdateWidget(oldWidget);
@@ -2732,6 +2738,7 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
27322738
}
27332739
}
27342740

2741+
@protected
27352742
@override
27362743
void didChangeDependencies() {
27372744
// Using maybeOf is valid here since both the Scaffold and ScaffoldMessenger
@@ -2750,6 +2757,7 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
27502757
super.didChangeDependencies();
27512758
}
27522759

2760+
@protected
27532761
@override
27542762
void dispose() {
27552763
_geometryNotifier.dispose();
@@ -2864,6 +2872,7 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
28642872
});
28652873
}
28662874

2875+
@protected
28672876
@override
28682877
Widget build(BuildContext context) {
28692878
assert(debugCheckHasMediaQuery(context));

packages/flutter/lib/src/material/segmented_button.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ class SegmentedButtonState<T> extends State<SegmentedButton<T>> {
426426
@visibleForTesting
427427
final Map<ButtonSegment<T>, MaterialStatesController> statesControllers = <ButtonSegment<T>, MaterialStatesController>{};
428428

429+
@protected
429430
@override
430431
void didUpdateWidget(covariant SegmentedButton<T> oldWidget) {
431432
super.didUpdateWidget(oldWidget);
@@ -464,6 +465,7 @@ class SegmentedButtonState<T> extends State<SegmentedButton<T>> {
464465
}
465466
}
466467

468+
@protected
467469
@override
468470
Widget build(BuildContext context) {
469471
final SegmentedButtonThemeData theme = SegmentedButtonTheme.of(context);
@@ -605,6 +607,7 @@ class SegmentedButtonState<T> extends State<SegmentedButton<T>> {
605607
);
606608
}
607609

610+
@protected
608611
@override
609612
void dispose() {
610613
for (final MaterialStatesController controller in statesControllers.values) {

packages/flutter/lib/src/material/selection_area.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ class SelectionAreaState extends State<SelectionArea> {
113113
/// The [State] of the [SelectableRegion] for which this [SelectionArea] wraps.
114114
SelectableRegionState get selectableRegion => _selectableRegionKey.currentState!;
115115

116+
@protected
116117
@override
117118
Widget build(BuildContext context) {
118119
assert(debugCheckHasMaterialLocalizations(context));

packages/flutter/lib/src/material/tooltip.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,7 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
706706
return true;
707707
}
708708

709+
@protected
709710
@override
710711
void initState() {
711712
super.initState();
@@ -715,6 +716,7 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
715716
GestureBinding.instance.pointerRouter.addGlobalRoute(_handleGlobalPointerEvent);
716717
}
717718

719+
@protected
718720
@override
719721
void didChangeDependencies() {
720722
super.didChangeDependencies();
@@ -797,6 +799,7 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
797799
: SelectionContainer.disabled(child: overlayChild);
798800
}
799801

802+
@protected
800803
@override
801804
void dispose() {
802805
GestureBinding.instance.pointerRouter.removeGlobalRoute(_handleGlobalPointerEvent);
@@ -815,6 +818,7 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
815818
super.dispose();
816819
}
817820

821+
@protected
818822
@override
819823
Widget build(BuildContext context) {
820824
// If message is empty then no need to create a tooltip overlay to show

0 commit comments

Comments
 (0)