From 096b0e827d7282f7e8c131c80e3c8eab45fd2d41 Mon Sep 17 00:00:00 2001 From: cupofme Date: Thu, 16 Jul 2026 10:16:27 +0200 Subject: [PATCH 1/2] Add `avoid_context_read_in_build` lint --- packages/leancode_lint/CHANGELOG.md | 5 + packages/leancode_lint/README.md | 42 ++++ packages/leancode_lint/lib/plugin.dart | 6 + .../lints/avoid_context_read_in_build.dart | 230 ++++++++++++++++++ .../test/mock_libraries/flutter_bloc.dart | 8 + .../avoid_context_read_in_build_test.dart | 194 +++++++++++++++ 6 files changed, 485 insertions(+) create mode 100644 packages/leancode_lint/lib/src/lints/avoid_context_read_in_build.dart create mode 100644 packages/leancode_lint/test/test_cases/avoid_context_read_in_build_test.dart diff --git a/packages/leancode_lint/CHANGELOG.md b/packages/leancode_lint/CHANGELOG.md index 8398ea77..0ed8f2b4 100644 --- a/packages/leancode_lint/CHANGELOG.md +++ b/packages/leancode_lint/CHANGELOG.md @@ -1,3 +1,8 @@ +# Unreleased + +- Add new custom lints: + - [`avoid_context_read_in_build`](https://github.com/leancodepl/flutter_corelibrary/tree/master/packages/leancode_lint#avoid_context_read_in_build) + # 24.0.0 - Add new custom lints: diff --git a/packages/leancode_lint/README.md b/packages/leancode_lint/README.md index fa473322..87811c54 100644 --- a/packages/leancode_lint/README.md +++ b/packages/leancode_lint/README.md @@ -256,6 +256,48 @@ None. +
+avoid_context_read_in_build + +### `avoid_context_read_in_build` + +**AVOID** reading reactive data with `context.read` inside a `build` method. + +`read` grabs a value once and never re-subscribes, so using its result to render +leaves the UI stale when the value changes — `watch` (or a `BlocBuilder` / +`BlocSelector`) is what you want. + +The lint is intentionally narrow. It does **not** flag the many legitimate uses +of `context.read` in `build`: calling a method, adding a bloc event, or grabbing +a bloc/service reference. It only flags reads whose value is consumed as data — +a getter/property read (e.g. `.state`), or a plain non-bloc value used directly. +Reads inside deferred interaction callbacks (`onTap`, `onPressed`) are exempt; +reads inside builder closures that run during `build` are checked. + +**BAD:** + +```dart +Widget build(BuildContext context) { + final count = context.read().state; + return Text('$count'); +} +``` + +**GOOD:** + +```dart +Widget build(BuildContext context) { + final count = context.watch().state; + return Text('$count'); +} +``` + +#### Configuration + +None. + +
+
bloc_related_class_naming diff --git a/packages/leancode_lint/lib/plugin.dart b/packages/leancode_lint/lib/plugin.dart index 4d754127..8eec4b36 100644 --- a/packages/leancode_lint/lib/plugin.dart +++ b/packages/leancode_lint/lib/plugin.dart @@ -7,6 +7,7 @@ import 'package:leancode_lint/src/assists/convert_record_into_nominal_type.dart' import 'package:leancode_lint/src/lints/add_cubit_suffix_for_cubits.dart'; import 'package:leancode_lint/src/lints/avoid_catch_error.dart'; import 'package:leancode_lint/src/lints/avoid_conditional_hooks.dart'; +import 'package:leancode_lint/src/lints/avoid_context_read_in_build.dart'; import 'package:leancode_lint/src/lints/avoid_single_child_in_multi_child_widget.dart'; import 'package:leancode_lint/src/lints/bloc_related_class_naming.dart'; import 'package:leancode_lint/src/lints/bloc_subclasses_naming.dart'; @@ -74,6 +75,11 @@ final class LeanCodeLintPlugin extends Plugin { NeverDiscardBuildContext.code, RenameDiscardedBuildContextFix.new, ) + ..registerWarningRule(AvoidContextReadInBuild()) + ..registerFixForRule( + AvoidContextReadInBuild.code, + ReplaceContextReadWithWatchFix.new, + ) // TODO: disabled by default until stabilized. Add documentation. ..registerLintRule(ConstructorParametersAndFieldsShouldHaveTheSameOrder()) ..registerWarningRule(AvoidSingleChildInMultiChildWidgets()) diff --git a/packages/leancode_lint/lib/src/lints/avoid_context_read_in_build.dart b/packages/leancode_lint/lib/src/lints/avoid_context_read_in_build.dart new file mode 100644 index 00000000..4251822a --- /dev/null +++ b/packages/leancode_lint/lib/src/lints/avoid_context_read_in_build.dart @@ -0,0 +1,230 @@ +import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; +import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:analyzer_plugin/utilities/range_factory.dart'; +import 'package:leancode_lint/src/helpers.dart'; +import 'package:leancode_lint/src/type_checker.dart'; + +/// Warns when `context.read` is used to consume reactive data during `build`. +/// +/// `read` grabs a value once and never re-subscribes, so using its result to +/// render leaves the UI stale when the value changes — `watch` (or a +/// `BlocBuilder`/`BlocSelector`) is what's actually wanted. +/// +/// The rule is deliberately narrow: it does not flag the many legitimate uses +/// of `context.read` in `build` — calling methods, adding bloc events, or +/// grabbing a bloc/service reference. Only reads whose value is consumed as +/// data (a getter/property read, or a plain non-bloc value used directly) are +/// reported. Reads inside deferred interaction callbacks (`onTap`, `onPressed`) +/// are exempt; reads inside builder closures that run during build are checked. +class AvoidContextReadInBuild extends AnalysisRule { + AvoidContextReadInBuild() + : super(name: code.lowerCaseName, description: code.problemMessage); + + static const code = LintCode( + 'avoid_context_read_in_build', + "Avoid reading reactive data with 'context.read' inside 'build' method.", + correctionMessage: + "Use 'context.watch' (or BlocBuilder/BlocSelector) so the widget rebuilds when the value changes.", + severity: .WARNING, + ); + + @override + LintCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + registry.addMethodInvocation(this, _Visitor(this)); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor(this.rule); + + final AnalysisRule rule; + + static const _buildContextChecker = TypeChecker.fromName( + 'BuildContext', + packageName: 'flutter', + ); + + static const _blocChecker = TypeChecker.any([ + .fromName('BlocBase', packageName: 'bloc'), + .fromName('Cubit', packageName: 'bloc'), + .fromName('Bloc', packageName: 'bloc'), + ]); + + @override + void visitMethodInvocation(MethodInvocation node) { + if (node.methodName.name != 'read') { + return; + } + final targetType = node.realTarget?.staticType; + if (targetType == null || + !_buildContextChecker.isAssignableFromType(targetType)) { + return; + } + + // The read itself must execute during build. + if (!_runsDuringBuild(node)) { + return; + } + + final parent = node.parent; + if (parent is VariableDeclaration && identical(parent.initializer, node)) { + _checkTracedVariable(node, parent); + return; + } + + if (_isReactiveDataUse(node, node.staticType)) { + rule.reportAtNode(node.methodName); + } + } + + /// Follows a local variable initialized directly from the read and reports + /// once if any of its references (that run during build) consume reactive + /// data. + void _checkTracedVariable(MethodInvocation node, VariableDeclaration decl) { + final element = decl.declaredFragment?.element; + final buildMethod = node.thisOrAncestorOfType(); + if (element == null || buildMethod == null) { + return; + } + + final references = _ReferenceGatherer.gather(buildMethod.body, element); + for (final reference in references) { + if (_runsDuringBuild(reference) && + _isReactiveDataUse(reference, reference.staticType)) { + rule.reportAtNode(node.methodName); + return; + } + } + } + + /// Whether [occurrence] (the read expression or a reference to a traced + /// variable) is consumed as reactive data. + bool _isReactiveDataUse(Expression occurrence, DartType? type) { + final parent = occurrence.parent; + + // Method-call/cascade receiver: `x.doThing()`, `x.add(e)` — a side effect, + // not a data read. + if (parent is MethodInvocation && + identical(parent.realTarget, occurrence)) { + return false; + } + if (parent is CascadeExpression && identical(parent.target, occurrence)) { + return false; + } + + // Member access: a getter/field read (`x.state`, `x.value`) consumes data, + // but a method tear-off (`x.increment`) is just a reference to call later. + if (parent is PropertyAccess && identical(parent.realTarget, occurrence)) { + return parent.propertyName.element is! MethodElement; + } + if (parent is PrefixedIdentifier && identical(parent.prefix, occurrence)) { + return parent.identifier.element is! MethodElement; + } + + // Used directly as a plain value (argument, interpolation, return, ...): + // flag only when it is not a bloc/cubit object reference. + return type != null && !_blocChecker.isAssignableFromType(type); + } + + /// Whether [node] executes during build: it is inside a widget's `build` + /// method, and every closure between [node] and that method declares a + /// `BuildContext` parameter (i.e. is a builder that runs during build, not a + /// deferred interaction callback). + bool _runsDuringBuild(AstNode node) { + for ( + AstNode? current = node.parent; + current != null; + current = current.parent + ) { + if (current is FunctionExpression && + !_declaresBuildContextParameter(current)) { + return false; + } + if (current is MethodDeclaration) { + if (current.name.lexeme != 'build') { + return false; + } + final classDeclaration = current + .thisOrAncestorOfType(); + return classDeclaration != null && isWidgetClass(classDeclaration); + } + } + return false; + } + + bool _declaresBuildContextParameter(FunctionExpression function) { + final parameters = function.parameters?.parameters; + if (parameters == null) { + return false; + } + for (final parameter in parameters) { + final type = parameter.declaredFragment?.element.type; + if (type != null && _buildContextChecker.isAssignableFromType(type)) { + return true; + } + } + return false; + } +} + +/// Gathers every simple identifier within a subtree that resolves to a given +/// element. +class _ReferenceGatherer extends RecursiveAstVisitor { + _ReferenceGatherer(this._element); + + final Element _element; + final List _references = []; + + static List gather(AstNode root, Element element) { + final gatherer = _ReferenceGatherer(element); + root.accept(gatherer); + return gatherer._references; + } + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + if (identical(node.element, _element)) { + _references.add(node); + } + super.visitSimpleIdentifier(node); + } +} + +class ReplaceContextReadWithWatchFix extends ResolvedCorrectionProducer { + ReplaceContextReadWithWatchFix({required super.context}); + + @override + FixKind get fixKind => const .new( + 'leancode_lint.fix.replaceContextReadWithWatch', + DartFixKindPriority.standard, + "Replace with 'context.watch'", + ); + + @override + CorrectionApplicability get applicability => .automatically; + + @override + Future compute(ChangeBuilder builder) async { + await builder.addDartFileEdit( + file, + (builder) => + builder.addSimpleReplacement(range.diagnostic(diagnostic!), 'watch'), + ); + } +} diff --git a/packages/leancode_lint/test/mock_libraries/flutter_bloc.dart b/packages/leancode_lint/test/mock_libraries/flutter_bloc.dart index d4bd4314..980ffaeb 100644 --- a/packages/leancode_lint/test/mock_libraries/flutter_bloc.dart +++ b/packages/leancode_lint/test/mock_libraries/flutter_bloc.dart @@ -4,7 +4,15 @@ mixin MockFlutterBloc on AnalysisRuleTest { @override void setUp() { newPackage('flutter_bloc').addFile('lib/flutter_bloc.dart', ''' +import 'package:flutter/material.dart'; + export 'package:bloc/bloc.dart'; + +extension BlocContextExtention on BuildContext { + T read() => throw UnimplementedError(); + + T watch() => throw UnimplementedError(); +} '''); super.setUp(); } diff --git a/packages/leancode_lint/test/test_cases/avoid_context_read_in_build_test.dart b/packages/leancode_lint/test/test_cases/avoid_context_read_in_build_test.dart new file mode 100644 index 00000000..70f90927 --- /dev/null +++ b/packages/leancode_lint/test/test_cases/avoid_context_read_in_build_test.dart @@ -0,0 +1,194 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:leancode_lint/src/lints/avoid_context_read_in_build.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../assert_ranges.dart'; +import '../mock_libraries.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidContextReadInBuildTest); + }); +} + +/// Wraps [buildBody] (the contents of a widget's `build` method) in a source +/// file with the helpers the test cases reference. +String _widget(String buildBody) => + ''' +import 'package:flutter/material.dart'; +import 'package:flutter_bloc/flutter_bloc.dart'; + +class MyCubit extends Cubit { + MyCubit() : super(0); + void doThing() {} +} + +class Consumer extends StatelessWidget { + const Consumer({super.key, this.value}); + final Object? value; + @override + Widget build(BuildContext context) => const SizedBox(); +} + +class Button extends StatelessWidget { + const Button({super.key, this.onTap}); + final void Function()? onTap; + @override + Widget build(BuildContext context) => const SizedBox(); +} + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { +$buildBody + } +} +'''; + +@reflectiveTest +class AvoidContextReadInBuildTest extends AnalysisRuleTest + with MockFlutter, MockBloc, MockFlutterBloc { + @override + void setUp() { + rule = AvoidContextReadInBuild(); + + super.setUp(); + } + + Future test_stateGetter_intoVariable_flagged() async { + await assertDiagnosticsInRanges( + _widget(''' + final s = context.[!read!]().state; + return Consumer(value: s);'''), + ); + } + + Future test_stateGetter_inline_flagged() async { + await assertDiagnosticsInRanges( + _widget(''' + return Consumer(value: context.[!read!]().state);'''), + ); + } + + Future test_plainValue_flagged() async { + await assertDiagnosticsInRanges( + _widget(''' + return Consumer(value: context.[!read!]());'''), + ); + } + + Future test_tracedPlainValue_flagged() async { + await assertDiagnosticsInRanges( + _widget(''' + final v = context.[!read!](); + return Consumer(value: v);'''), + ); + } + + Future test_tracedStateGetter_flagged() async { + await assertDiagnosticsInRanges( + _widget(''' + final c = context.[!read!](); + return Consumer(value: c.state);'''), + ); + } + + Future test_insideBuilder_flagged() async { + await assertDiagnosticsInRanges( + _widget(''' + return Builder( + builder: (context) => Consumer(value: context.[!read!]().state), + );'''), + ); + } + + Future test_methodReceiver_ok() async { + await assertNoDiagnostics( + _widget(''' + context.read().doThing(); + return const SizedBox();'''), + ); + } + + Future test_tracedMethodReceiver_ok() async { + await assertNoDiagnostics( + _widget(''' + final c = context.read(); + c.doThing(); + return const SizedBox();'''), + ); + } + + Future test_blocObjectReference_ok() async { + await assertNoDiagnostics( + _widget(''' + return Consumer(value: context.read());'''), + ); + } + + Future test_tracedBlocObjectReference_ok() async { + await assertNoDiagnostics( + _widget(''' + final c = context.read(); + return Consumer(value: c);'''), + ); + } + + Future test_deferredCallback_ok() async { + await assertNoDiagnostics( + _widget(''' + return Button(onTap: () => context.read().state);'''), + ); + } + + Future test_deferredCallbackInsideBuilder_ok() async { + await assertNoDiagnostics( + _widget(''' + return Builder( + builder: (context) => Button(onTap: () => context.read().state), + );'''), + ); + } + + Future test_methodTearOff_ok() async { + await assertNoDiagnostics( + _widget(''' + return Button(onTap: context.read().doThing);'''), + ); + } + + Future test_tracedMethodTearOff_ok() async { + await assertNoDiagnostics( + _widget(''' + final c = context.read(); + return Button(onTap: c.doThing);'''), + ); + } + + Future test_watch_ok() async { + await assertNoDiagnostics( + _widget(''' + return Consumer(value: context.watch().state);'''), + ); + } + + Future test_readOutsideWidget_ok() async { + await assertNoDiagnostics(''' +import 'package:flutter/material.dart'; +import 'package:flutter_bloc/flutter_bloc.dart'; + +class MyCubit extends Cubit { + MyCubit() : super(0); +} + +class NotAWidget { + NotAWidget(this.context); + final BuildContext context; + + int build() => context.read().state; +} +'''); + } +} From 9027a6f3b63552e0a612577a751dd9e82ce7b5b9 Mon Sep 17 00:00:00 2001 From: cupofme Date: Mon, 27 Jul 2026 13:24:17 +0200 Subject: [PATCH 2/2] Flag all read usages in build method --- packages/leancode_lint/README.md | 32 +++-- .../lints/avoid_context_read_in_build.dart | 111 ++---------------- .../avoid_context_read_in_build_test.dart | 68 ++++------- 3 files changed, 56 insertions(+), 155 deletions(-) diff --git a/packages/leancode_lint/README.md b/packages/leancode_lint/README.md index aae3492f..5d919f32 100644 --- a/packages/leancode_lint/README.md +++ b/packages/leancode_lint/README.md @@ -261,18 +261,19 @@ None. ### `avoid_context_read_in_build` -**AVOID** reading reactive data with `context.read` inside a `build` method. +**AVOID** using `context.read` inside a `build` method. `read` grabs a value once and never re-subscribes, so using its result to render leaves the UI stale when the value changes — `watch` (or a `BlocBuilder` / `BlocSelector`) is what you want. -The lint is intentionally narrow. It does **not** flag the many legitimate uses -of `context.read` in `build`: calling a method, adding a bloc event, or grabbing -a bloc/service reference. It only flags reads whose value is consumed as data — -a getter/property read (e.g. `.state`), or a plain non-bloc value used directly. -Reads inside deferred interaction callbacks (`onTap`, `onPressed`) are exempt; -reads inside builder closures that run during `build` are checked. +Every `read` that executes during `build` is flagged, whatever it is used for: +reading a value, calling a method, or grabbing a bloc/service reference. All +three run on every rebuild, so none of them belong in `build`. Either consume +the value with `watch` / `BlocBuilder` / `BlocSelector`, or move the read into a +callback. Reads inside deferred interaction callbacks (`onTap`, `onPressed`) are +exempt — that's where `read` is meant to be used; reads inside builder closures +that run during `build` are checked. **BAD:** @@ -283,6 +284,14 @@ Widget build(BuildContext context) { } ``` +```dart +Widget build(BuildContext context) { + // Fires on every rebuild. + context.read().increment(); + return const SizedBox(); +} +``` + **GOOD:** ```dart @@ -292,6 +301,15 @@ Widget build(BuildContext context) { } ``` +```dart +Widget build(BuildContext context) { + return ElevatedButton( + onPressed: () => context.read().increment(), + child: const Text('+'), + ); +} +``` + #### Configuration None. diff --git a/packages/leancode_lint/lib/src/lints/avoid_context_read_in_build.dart b/packages/leancode_lint/lib/src/lints/avoid_context_read_in_build.dart index 4251822a..fd785d5b 100644 --- a/packages/leancode_lint/lib/src/lints/avoid_context_read_in_build.dart +++ b/packages/leancode_lint/lib/src/lints/avoid_context_read_in_build.dart @@ -5,8 +5,6 @@ import 'package:analyzer/analysis_rule/rule_context.dart'; import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; -import 'package:analyzer/dart/element/element.dart'; -import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/error/error.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; @@ -14,27 +12,27 @@ import 'package:analyzer_plugin/utilities/range_factory.dart'; import 'package:leancode_lint/src/helpers.dart'; import 'package:leancode_lint/src/type_checker.dart'; -/// Warns when `context.read` is used to consume reactive data during `build`. +/// Warns when `context.read` is called during `build`. /// /// `read` grabs a value once and never re-subscribes, so using its result to /// render leaves the UI stale when the value changes — `watch` (or a /// `BlocBuilder`/`BlocSelector`) is what's actually wanted. /// -/// The rule is deliberately narrow: it does not flag the many legitimate uses -/// of `context.read` in `build` — calling methods, adding bloc events, or -/// grabbing a bloc/service reference. Only reads whose value is consumed as -/// data (a getter/property read, or a plain non-bloc value used directly) are -/// reported. Reads inside deferred interaction callbacks (`onTap`, `onPressed`) -/// are exempt; reads inside builder closures that run during build are checked. +/// Every `read` that executes during build is reported, whatever it is used +/// for: reading a value, calling a method, or grabbing a bloc/service +/// reference. All three run on every rebuild, so none of them belong in +/// `build`. Reads inside deferred interaction callbacks (`onTap`, `onPressed`) +/// are exempt — that is where `read` is meant to be used; reads inside builder +/// closures that run during build are checked. class AvoidContextReadInBuild extends AnalysisRule { AvoidContextReadInBuild() : super(name: code.lowerCaseName, description: code.problemMessage); static const code = LintCode( 'avoid_context_read_in_build', - "Avoid reading reactive data with 'context.read' inside 'build' method.", + "Avoid using 'context.read' inside 'build' method.", correctionMessage: - "Use 'context.watch' (or BlocBuilder/BlocSelector) so the widget rebuilds when the value changes.", + "Use 'context.watch' (or BlocBuilder/BlocSelector) to consume the value, or move the read into a callback.", severity: .WARNING, ); @@ -60,12 +58,6 @@ class _Visitor extends SimpleAstVisitor { packageName: 'flutter', ); - static const _blocChecker = TypeChecker.any([ - .fromName('BlocBase', packageName: 'bloc'), - .fromName('Cubit', packageName: 'bloc'), - .fromName('Bloc', packageName: 'bloc'), - ]); - @override void visitMethodInvocation(MethodInvocation node) { if (node.methodName.name != 'read') { @@ -77,69 +69,11 @@ class _Visitor extends SimpleAstVisitor { return; } - // The read itself must execute during build. if (!_runsDuringBuild(node)) { return; } - final parent = node.parent; - if (parent is VariableDeclaration && identical(parent.initializer, node)) { - _checkTracedVariable(node, parent); - return; - } - - if (_isReactiveDataUse(node, node.staticType)) { - rule.reportAtNode(node.methodName); - } - } - - /// Follows a local variable initialized directly from the read and reports - /// once if any of its references (that run during build) consume reactive - /// data. - void _checkTracedVariable(MethodInvocation node, VariableDeclaration decl) { - final element = decl.declaredFragment?.element; - final buildMethod = node.thisOrAncestorOfType(); - if (element == null || buildMethod == null) { - return; - } - - final references = _ReferenceGatherer.gather(buildMethod.body, element); - for (final reference in references) { - if (_runsDuringBuild(reference) && - _isReactiveDataUse(reference, reference.staticType)) { - rule.reportAtNode(node.methodName); - return; - } - } - } - - /// Whether [occurrence] (the read expression or a reference to a traced - /// variable) is consumed as reactive data. - bool _isReactiveDataUse(Expression occurrence, DartType? type) { - final parent = occurrence.parent; - - // Method-call/cascade receiver: `x.doThing()`, `x.add(e)` — a side effect, - // not a data read. - if (parent is MethodInvocation && - identical(parent.realTarget, occurrence)) { - return false; - } - if (parent is CascadeExpression && identical(parent.target, occurrence)) { - return false; - } - - // Member access: a getter/field read (`x.state`, `x.value`) consumes data, - // but a method tear-off (`x.increment`) is just a reference to call later. - if (parent is PropertyAccess && identical(parent.realTarget, occurrence)) { - return parent.propertyName.element is! MethodElement; - } - if (parent is PrefixedIdentifier && identical(parent.prefix, occurrence)) { - return parent.identifier.element is! MethodElement; - } - - // Used directly as a plain value (argument, interpolation, return, ...): - // flag only when it is not a bloc/cubit object reference. - return type != null && !_blocChecker.isAssignableFromType(type); + rule.reportAtNode(node.methodName); } /// Whether [node] executes during build: it is inside a widget's `build` @@ -183,29 +117,6 @@ class _Visitor extends SimpleAstVisitor { } } -/// Gathers every simple identifier within a subtree that resolves to a given -/// element. -class _ReferenceGatherer extends RecursiveAstVisitor { - _ReferenceGatherer(this._element); - - final Element _element; - final List _references = []; - - static List gather(AstNode root, Element element) { - final gatherer = _ReferenceGatherer(element); - root.accept(gatherer); - return gatherer._references; - } - - @override - void visitSimpleIdentifier(SimpleIdentifier node) { - if (identical(node.element, _element)) { - _references.add(node); - } - super.visitSimpleIdentifier(node); - } -} - class ReplaceContextReadWithWatchFix extends ResolvedCorrectionProducer { ReplaceContextReadWithWatchFix({required super.context}); @@ -217,7 +128,7 @@ class ReplaceContextReadWithWatchFix extends ResolvedCorrectionProducer { ); @override - CorrectionApplicability get applicability => .automatically; + CorrectionApplicability get applicability => .singleLocation; @override Future compute(ChangeBuilder builder) async { diff --git a/packages/leancode_lint/test/test_cases/avoid_context_read_in_build_test.dart b/packages/leancode_lint/test/test_cases/avoid_context_read_in_build_test.dart index 70f90927..4b208c89 100644 --- a/packages/leancode_lint/test/test_cases/avoid_context_read_in_build_test.dart +++ b/packages/leancode_lint/test/test_cases/avoid_context_read_in_build_test.dart @@ -23,6 +23,8 @@ class MyCubit extends Cubit { void doThing() {} } +class MyService {} + class Consumer extends StatelessWidget { const Consumer({super.key, this.value}); final Object? value; @@ -79,22 +81,6 @@ class AvoidContextReadInBuildTest extends AnalysisRuleTest ); } - Future test_tracedPlainValue_flagged() async { - await assertDiagnosticsInRanges( - _widget(''' - final v = context.[!read!](); - return Consumer(value: v);'''), - ); - } - - Future test_tracedStateGetter_flagged() async { - await assertDiagnosticsInRanges( - _widget(''' - final c = context.[!read!](); - return Consumer(value: c.state);'''), - ); - } - Future test_insideBuilder_flagged() async { await assertDiagnosticsInRanges( _widget(''' @@ -104,42 +90,42 @@ class AvoidContextReadInBuildTest extends AnalysisRuleTest ); } - Future test_methodReceiver_ok() async { - await assertNoDiagnostics( + Future test_methodReceiver_flagged() async { + await assertDiagnosticsInRanges( _widget(''' - context.read().doThing(); + context.[!read!]().doThing(); return const SizedBox();'''), ); } - Future test_tracedMethodReceiver_ok() async { - await assertNoDiagnostics( + Future test_blocObjectReference_flagged() async { + await assertDiagnosticsInRanges( _widget(''' - final c = context.read(); - c.doThing(); - return const SizedBox();'''), + return Consumer(value: context.[!read!]());'''), ); } - Future test_blocObjectReference_ok() async { - await assertNoDiagnostics( + /// The tear-off evaluates the read during build, unlike + /// [test_deferredCallback_ok] which defers it until the tap. + Future test_methodTearOff_flagged() async { + await assertDiagnosticsInRanges( _widget(''' - return Consumer(value: context.read());'''), + return Button(onTap: context.[!read!]().doThing);'''), ); } - Future test_tracedBlocObjectReference_ok() async { - await assertNoDiagnostics( + Future test_serviceReference_flagged() async { + await assertDiagnosticsInRanges( _widget(''' - final c = context.read(); - return Consumer(value: c);'''), + final s = context.[!read!](); + return Consumer(value: s);'''), ); } Future test_deferredCallback_ok() async { await assertNoDiagnostics( _widget(''' - return Button(onTap: () => context.read().state);'''), + return Button(onTap: () => context.read().doThing());'''), ); } @@ -147,26 +133,12 @@ class AvoidContextReadInBuildTest extends AnalysisRuleTest await assertNoDiagnostics( _widget(''' return Builder( - builder: (context) => Button(onTap: () => context.read().state), + builder: (context) => + Button(onTap: () => context.read().doThing()), );'''), ); } - Future test_methodTearOff_ok() async { - await assertNoDiagnostics( - _widget(''' - return Button(onTap: context.read().doThing);'''), - ); - } - - Future test_tracedMethodTearOff_ok() async { - await assertNoDiagnostics( - _widget(''' - final c = context.read(); - return Button(onTap: c.doThing);'''), - ); - } - Future test_watch_ok() async { await assertNoDiagnostics( _widget('''