Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/leancode_lint/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)

# 25.0.0

- Add new custom lint [`prefer_abstract_final_class`](https://github.com/leancodepl/flutter_corelibrary/tree/master/packages/leancode_lint#prefer_abstract_final_class)
Expand Down
60 changes: 60 additions & 0 deletions packages/leancode_lint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,66 @@ None.

</details>

<details>
<summary><code>avoid_context_read_in_build</code></summary>

### `avoid_context_read_in_build`

**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.

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:**

```dart
Widget build(BuildContext context) {
final count = context.read<CounterCubit>().state;
return Text('$count');
}
```

```dart
Widget build(BuildContext context) {
// Fires on every rebuild.
context.read<CounterCubit>().increment();
return const SizedBox();
}
```

**GOOD:**

```dart
Widget build(BuildContext context) {
final count = context.watch<CounterCubit>().state;
return Text('$count');
}
```

```dart
Widget build(BuildContext context) {
return ElevatedButton(
onPressed: () => context.read<CounterCubit>().increment(),
child: const Text('+'),
);
}
```

#### Configuration

None.

</details>

<details>
<summary><code>bloc_related_class_naming</code></summary>

Expand Down
6 changes: 6 additions & 0 deletions packages/leancode_lint/lib/plugin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -75,6 +76,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())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
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/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 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.
Comment on lines +18 to +19

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

select() is also valid (just like BlocSelector). We probably can't suggest it in the fix (because it would require rewriting more code), but we should mention it in docs and messages

///
/// 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 using 'context.read' inside 'build' method.",
correctionMessage:
"Use 'context.watch' (or BlocBuilder/BlocSelector) to consume the value, or move the read into a callback.",
severity: .WARNING,
);

@override
LintCode get diagnosticCode => code;

@override
void registerNodeProcessors(
RuleVisitorRegistry registry,
RuleContext context,
) {
registry.addMethodInvocation(this, _Visitor(this));
}
}

class _Visitor extends SimpleAstVisitor<void> {
_Visitor(this.rule);

final AnalysisRule rule;

static const _buildContextChecker = TypeChecker.fromName(
'BuildContext',
packageName: 'flutter',
);

@override
void visitMethodInvocation(MethodInvocation node) {
if (node.methodName.name != 'read') {
return;
}
final targetType = node.realTarget?.staticType;
if (targetType == null ||
!_buildContextChecker.isAssignableFromType(targetType)) {
return;
}

if (!_runsDuringBuild(node)) {
return;
}

rule.reportAtNode(node.methodName);
}

/// 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<ClassDeclaration>();
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;
}
}

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 => .singleLocation;

@override
Future<void> compute(ChangeBuilder builder) async {
await builder.addDartFileEdit(
file,
(builder) =>
builder.addSimpleReplacement(range.diagnostic(diagnostic!), 'watch'),
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>() => throw UnimplementedError();

T watch<T>() => throw UnimplementedError();
}
''');
super.setUp();
}
Expand Down
Loading
Loading