Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
7 changes: 6 additions & 1 deletion 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_build_context_in_blocs`](https://github.com/leancodepl/flutter_corelibrary/tree/master/packages/leancode_lint#avoid_build_context_in_blocs)

# 24.0.0

- Add new custom lints:
Expand Down Expand Up @@ -93,7 +98,7 @@
- Remove the following lints which have been removed from Dart:
- [`package_api_docs`](https://dart.dev/tools/linter-rules/package_api_docs)
- [`unsafe_html`](https://dart.dev/tools/linter-rules/unsafe_html)
- Disable the [`require_trailing_commas`](https://dart.dev/tools/linter-rules/require_trailing_commas) lint as it conflicts with Dart 3.7 formatter (https://github.com/dart-lang/sdk/issues/60119).
- Disable the [`require_trailing_commas`](https://dart.dev/tools/linter-rules/require_trailing_commas) lint as it conflicts with Dart 3.7 formatter (<https://github.com/dart-lang/sdk/issues/60119>).

# 15.1.0

Expand Down
50 changes: 50 additions & 0 deletions packages/leancode_lint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,56 @@ None.

</details>

<details>
<summary><code>avoid_build_context_in_blocs</code></summary>

### `avoid_build_context_in_blocs`

**AVOID** letting a `BuildContext` cross into a Bloc/Cubit.

A `BuildContext` couples business logic to the widget tree, which risks stale
contexts and wrong `InheritedWidget` reads and makes the logic hard to test. The
rule flags both passing a `BuildContext` into a Bloc/Cubit (via a method such as
`add`, or a constructor) and declaring one inside a Bloc/Cubit (as a parameter or
field). A value merely derived from a context (e.g. `MediaQuery.sizeOf(context)`)
is allowed.

**BAD:**

```dart
bloc.add(CounterEvent(context));

final event = CounterEvent(context);
bloc.add(event);

class CounterCubit extends Cubit<int> {
CounterCubit() : super(0);

final BuildContext context;

void another(BuildContext context) {}
}
```

**GOOD:**

```dart
bloc.add(CounterEvent());
bloc.add(CounterEvent(MediaQuery.sizeOf(context)));

class CounterCubit extends Cubit<int> {
CounterCubit() : super(0);

void another() {}
}
```

#### Configuration

None.

</details>

<details>
<summary><code>avoid_catch_error</code></summary>

Expand Down
2 changes: 2 additions & 0 deletions packages/leancode_lint/lib/plugin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:leancode_lint/src/assists/convert_iterable_map_to_collection_for
import 'package:leancode_lint/src/assists/convert_positional_to_named_formal.dart';
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_build_context_in_blocs.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_single_child_in_multi_child_widget.dart';
Expand Down Expand Up @@ -63,6 +64,7 @@ final class LeanCodeLintPlugin extends Plugin {
CatchParameterNames(config: config.catchParameterNames),
)
..registerWarningRule(AvoidCatchError())
..registerWarningRule(AvoidBuildContextInBlocs())
..registerWarningRule(AvoidConditionalHooks())
..registerWarningRule(HookWidgetDoesNotUseHooks())
..registerFixForRule(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
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:leancode_lint/src/bloc_utils.dart';
import 'package:leancode_lint/src/type_checker.dart';

/// Warns when a `BuildContext` crosses into a Bloc/Cubit.
///
/// A `BuildContext` couples business logic to the widget tree, which risks
/// stale contexts and wrong `InheritedWidget` reads and makes the logic hard to
/// test. This rule flags a `BuildContext` on both sides of the boundary:
/// passing one into a Bloc/Cubit (via a method, e.g. `bloc.add(...)`, or a
/// constructor) and declaring one inside a Bloc/Cubit (as a parameter or field).
class AvoidBuildContextInBlocs extends AnalysisRule {
AvoidBuildContextInBlocs()
: super(name: code.lowerCaseName, description: code.problemMessage);
Comment thread
cupofme marked this conversation as resolved.

static const code = LintCode(
'avoid_build_context_in_blocs',
"Avoid using 'BuildContext' in a Bloc/Cubit.",
correctionMessage:
"Remove the 'BuildContext' and directly pass only the data the Bloc/Cubit needs instead.",
severity: .WARNING,
);

@override
LintCode get diagnosticCode => code;

@override
void registerNodeProcessors(
RuleVisitorRegistry registry,
RuleContext context,
) {
final visitor = _Visitor(this);
registry
..addMethodInvocation(this, visitor)
..addInstanceCreationExpression(this, visitor)
..addClassDeclaration(this, visitor);
}
}

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

final AnalysisRule rule;

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

// Passing side: `bloc.add(...)` and other method calls on a Bloc/Cubit.
@override
void visitMethodInvocation(MethodInvocation node) {
final targetType = node.realTarget?.staticType;
if (targetType == null || determineBlocType(targetType.element) == null) {
return;
}

_reportContextArguments(node.argumentList);
}

// Passing side: `CounterCubit(context)` / `CounterBloc(context)`.
@override
void visitInstanceCreationExpression(InstanceCreationExpression node) {
if (determineBlocType(node.staticType?.element) == null) {
return;
}

_reportContextArguments(node.argumentList);
}

// Declaration side: `BuildContext` parameters and fields inside a Bloc/Cubit.
@override
void visitClassDeclaration(ClassDeclaration node) {
final element = node.declaredFragment?.element;
if (determineBlocType(element) == null) {
return;
}

final members = switch (node.body) {
BlockClassBody(:final members) => members,
_ => const <ClassMember>[],
};

for (final member in members) {
switch (member) {
case MethodDeclaration(:final parameters?):
_checkParameters(parameters);
case ConstructorDeclaration(:final parameters):
_checkParameters(parameters);
case FieldDeclaration(:final fields):
_checkFields(fields);
case _:
break;
}
}
}

void _checkParameters(FormalParameterList parameters) {
for (final parameter in parameters.parameters) {
final name = parameter.name;
final type = parameter.declaredFragment?.element.type;
if (name != null && type != null && _isBuildContext(type)) {
Comment thread
cupofme marked this conversation as resolved.
Outdated
rule.reportAtToken(name);
}
}
}

void _checkFields(VariableDeclarationList fields) {
for (final variable in fields.variables) {
final type = variable.declaredFragment?.element.type;
if (type != null && _isBuildContext(type)) {
rule.reportAtToken(variable.name);
}
}
}

void _reportContextArguments(ArgumentList argumentList) {
for (final argument in argumentList.arguments) {
final expression = argument.argumentExpression;
if (_carriesContext(expression, {}, 0)) {
rule.reportAtNode(expression);
}
}
}

/// Whether [expression] carries a `BuildContext` into the enclosing call.
///
/// Returns true when the expression is itself a `BuildContext`, when it
/// constructs an object with a `BuildContext` argument (e.g. an event like
/// `CounterEvent(context)`, including nested constructions), or when it is a
/// local variable whose initializer does so. Recursion deliberately does not
/// descend into method/function calls, so a value merely *derived* from a
/// context (e.g. `MediaQuery.sizeOf(context)`) is not flagged.
///
/// The local-variable trace is best-effort: it only inspects the declaration
/// initializer, not later reassignments. [visited] guards against cycles.
bool _carriesContext(
Expression? expression,
Set<Element> visited,
int depth,
) {
if (expression == null || depth > 20) {
return false;
}

final type = expression.staticType;
if (type != null && _isBuildContext(type)) {
Comment thread
cupofme marked this conversation as resolved.
Outdated
return true;
}

switch (expression) {
case InstanceCreationExpression(:final argumentList):
for (final argument in argumentList.arguments) {
if (_carriesContext(
argument.argumentExpression,
visited,
depth + 1,
)) {
return true;
}
}
case SimpleIdentifier(:final Element element?)
when element is LocalVariableElement && visited.add(element):
Comment thread
cupofme marked this conversation as resolved.
Outdated
final initializer = _localVariableInitializer(expression, element);
if (_carriesContext(initializer, visited, depth + 1)) {
return true;
}
}
Comment thread
cupofme marked this conversation as resolved.

return false;
}

Expression? _localVariableInitializer(
AstNode reference,
LocalVariableElement element,
) {
AstNode? body = reference;
while (body != null && body is! FunctionBody) {
body = body.parent;
}
if (body == null) {
return null;
}

final finder = _InitializerFinder(element);
body.accept(finder);
return finder.initializer;
}

bool _isBuildContext(DartType type) =>
_buildContextChecker.isExactlyType(type);
}

/// Finds the declaration initializer of a specific [LocalVariableElement].
class _InitializerFinder extends RecursiveAstVisitor<void> {
_InitializerFinder(this.element);

final LocalVariableElement element;
Expression? initializer;

@override
void visitVariableDeclaration(VariableDeclaration node) {
if (initializer == null &&
node.declaredFragment?.element == element &&
node.initializer != null) {
initializer = node.initializer;
}
super.visitVariableDeclaration(node);
}
Comment thread
cupofme marked this conversation as resolved.
}
1 change: 1 addition & 0 deletions packages/leancode_lint/test/mock_libraries/bloc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ abstract class Cubit<State> extends BlocBase<State> {

abstract class Bloc<Event, State> extends BlocBase<State> {
Bloc(State initialState) : super(initialState);
void add(Event event) {}
}
''');
super.setUp();
Expand Down
Loading
Loading