diff --git a/CHANGELOG.md b/CHANGELOG.md index b6c4c5ae..d94591fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.3.4 + +- Added `use_nearest_context` rule. + ## 0.3.3 - Fix pub.dev analysis issue diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index 4fc66f58..82fec98b 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -30,6 +30,7 @@ import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart'; import 'package:solid_lints/src/lints/prefer_last/prefer_last_rule.dart'; import 'package:solid_lints/src/lints/prefer_match_file_name/prefer_match_file_name_rule.dart'; import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/use_nearest_context_rule.dart'; import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// Creates a plugin for our custom linter @@ -69,6 +70,7 @@ class _SolidLints extends PluginBase { AvoidFinalWithGetterRule.createRule(configs), NamedParametersOrderingRule.createRule(configs), AvoidUnnecessaryReturnVariableRule.createRule(configs), + UseNearestContextRule.createRule(configs), ]; // Return only enabled rules diff --git a/lib/src/lints/use_nearest_context/fixes/use_nearest_context_fix.dart b/lib/src/lints/use_nearest_context/fixes/use_nearest_context_fix.dart new file mode 100644 index 00000000..fe3887c0 --- /dev/null +++ b/lib/src/lints/use_nearest_context/fixes/use_nearest_context_fix.dart @@ -0,0 +1,64 @@ +part of '../use_nearest_context_rule.dart'; + +/// A Quick fix for `use_nearest_context` rule +/// Suggests to renaming the nearest BuildContext variable +/// to the one that is being used +class _UseNearestContextFix extends DartFix { + static const _replaceComment = "Rename nearest BuildContext parameter"; + + final Expando _diagnosticsInfoExpando; + + _UseNearestContextFix(this._diagnosticsInfoExpando); + + @override + void run( + CustomLintResolver resolver, + ChangeReporter reporter, + CustomLintContext context, + Diagnostic diagnostic, + List others, + ) { + final statementInfo = _diagnosticsInfoExpando[diagnostic]; + if (statementInfo == null) return; + final parameterName = statementInfo.parameter.name; + if (parameterName == null) return; + + _addReplacement(reporter, parameterName, statementInfo.name); + } + + void _addReplacement( + ChangeReporter reporter, + Token? token, + String correction, + ) { + if (token == null) return; + final changeBuilder = reporter.createChangeBuilder( + message: _replaceComment, + priority: 1, + ); + + changeBuilder.addDartFileEdit((builder) { + builder.addSimpleReplacement( + token.sourceRange, + correction, + ); + }); + } +} + +/// Data class that holds info required for the [_UseNearestContextFix]. +class StatementInfo { + /// Creates instance of a [StatementInfo]. + const StatementInfo({ + required this.name, + required this.parameter, + }); + + /// The name of the outer BuildContext variable that was used + /// instead of the nearest one. + final String name; + + /// The nearest [SimpleFormalParameter] of type BuildContext + /// that should have been used instead. + final SimpleFormalParameter parameter; +} diff --git a/lib/src/lints/use_nearest_context/use_nearest_context_rule.dart b/lib/src/lints/use_nearest_context/use_nearest_context_rule.dart new file mode 100644 index 00000000..e81fd3e8 --- /dev/null +++ b/lib/src/lints/use_nearest_context/use_nearest_context_rule.dart @@ -0,0 +1,160 @@ +// ignore_for_file: avoid_print, lines_longer_than_80_chars + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/diagnostic/diagnostic.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/models/solid_lint_rule.dart'; +import 'package:solid_lints/src/utils/types_utils.dart'; + +part 'fixes/use_nearest_context_fix.dart'; + +/// A rule which checks that we use BuildContext from the nearest available +/// scope. +/// +/// ### Example: +/// #### BAD: +/// ```dart +/// class SomeWidget extends StatefulWidget { +/// ... +/// } +/// +/// class _SomeWidgetState extends State { +/// ... +/// void _showDialog() { +/// showModalBottomSheet( +/// context: context, +/// builder: (BuildContext _) { +/// final someProvider = context.watch(); // LINT, BuildContext is used not from the nearest available scope +/// +/// return const SizedBox.shrink(); +/// }, +/// ); +/// } +/// } +/// ``` +/// #### GOOD: +/// ```dart +/// class SomeWidget extends StatefulWidget { +/// ... +/// } +/// +/// class _SomeWidgetState extends State { +/// ... +/// void _showDialog() { +/// showModalBottomSheet( +/// context: context, +/// builder: (BuildContext context) +/// final someProvider = context.watch(); // OK +/// +/// return const SizedBox.shrink(); +/// }, +/// ); +/// } +/// } +/// ``` +/// +class UseNearestContextRule extends SolidLintRule { + /// This lint rule represents the error if BuildContext is used not from the + /// nearest available scope + static const lintName = 'use_nearest_context'; + + final _diagnosticsInfoExpando = Expando(); + + UseNearestContextRule._(super.rule); + + /// Creates a new instance of [UseNearestContextRule] + /// based on the lint configuration. + factory UseNearestContextRule.createRule(CustomLintConfigs configs) { + final rule = RuleConfig( + configs: configs, + name: lintName, + problemMessage: (value) => + 'Use the nearest BuildContext parameter instead of the outer one.', + ); + + return UseNearestContextRule._(rule); + } + + @override + void run( + CustomLintResolver resolver, + DiagnosticReporter reporter, + CustomLintContext context, + ) { + context.registry.addSimpleIdentifier((node) { + if (!isBuildContext(node.staticType)) return; + if (_isPropertyOfOtherObject(node)) return; + + final closestBuildContext = _findClosestBuildContext(node); + if (closestBuildContext == null) return; + if (closestBuildContext.name?.lexeme != node.name) { + if (_isDeclaredInNearestScope(node, closestBuildContext)) return; + + final diagnostic = reporter.atNode(node, code); + _diagnosticsInfoExpando[diagnostic] = StatementInfo( + name: node.name, + parameter: closestBuildContext, + ); + } + }); + } + + /// Returns `true` if [node] is a property accessed on another object + /// (e.g. `state.context`), but not on `this` (e.g. `this.context`). + bool _isPropertyOfOtherObject(SimpleIdentifier node) { + final parent = node.parent; + if (parent is PrefixedIdentifier && node == parent.identifier) { + return true; + } + if (parent is PropertyAccess && node == parent.propertyName) { + return parent.target is! ThisExpression; + } + return false; + } + + /// Returns `true` if [node] refers to a variable declared inside the body + /// of the function that owns [closestParam] (i.e. a local variable + /// in the same scope, like `final localCtx = innerContext;`). + bool _isDeclaredInNearestScope( + SimpleIdentifier node, + SimpleFormalParameter closestParam, + ) { + final element = node.element; + if (element is! LocalVariableElement) return false; + + final nearestFunction = closestParam.parent?.parent; + if (nearestFunction is! FunctionExpression) return false; + + final body = nearestFunction.body; + final declOffset = element.firstFragment.nameOffset; + if (declOffset == null) return false; + return declOffset >= body.offset && declOffset < body.end; + } + + SimpleFormalParameter? _findClosestBuildContext(SimpleIdentifier node) { + AstNode? current = node.parent; + + while (current != null) { + if (current is FunctionExpression) { + final functionParams = current.parameters?.parameters ?? []; + for (final param in functionParams) { + final actualParam = + param is DefaultFormalParameter ? param.parameter : param; + if (actualParam is SimpleFormalParameter && + isBuildContext(actualParam.declaredFragment?.element.type)) { + return actualParam; + } + } + } + current = current.parent; + } + return null; + } + + @override + List getFixes() => [_UseNearestContextFix(_diagnosticsInfoExpando)]; +} diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index 286e774d..1f83285e 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -90,3 +90,4 @@ custom_lint: - nullable - default - avoid_unnecessary_return_variable + - use_nearest_context diff --git a/lint_test/use_nearest_context_test.dart b/lint_test/use_nearest_context_test.dart new file mode 100644 index 00000000..d4cf816b --- /dev/null +++ b/lint_test/use_nearest_context_test.dart @@ -0,0 +1,187 @@ +// ignore_for_file: avoid_unused_parameters, unused_local_variable, function_lines_of_code, newline_before_return, member_ordering, avoid_non_null_assertion +import 'package:flutter/material.dart'; + +/// Check the `use_nearest_context` rule +void showDialog(BuildContext context) { + final outerContext = context; + + showModalBottomSheet( + context: context, + builder: (BuildContext _) { + /// expect_lint: use_nearest_context + return SizedBox.fromSize(size: outerContext.size); + }, + ); + + showModalBottomSheet( + context: context, + builder: (BuildContext _) { + /// expect_lint: use_nearest_context + return SizedBox.fromSize(size: context.size); + }, + ); + + final fun = ({required BuildContext context}) { + /// expect_lint: use_nearest_context + outerContext.mounted; + }; + + showModalBottomSheet( + context: context, + builder: (_) { + /// expect_lint: use_nearest_context + return SizedBox.fromSize(size: context.size); + }, + ); + + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + /// expect_lint: use_nearest_context + return SizedBox.fromSize(size: context.size); + }, + ); + + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + ///Allowed + return SizedBox.fromSize(size: innerContext.size); + }); + + showModalBottomSheet( + ///Allowed + context: context, + builder: (BuildContext context) { + ///Allowed + return SizedBox.fromSize(size: context.size); + }, + ); + + /// Nested builders: using outer builder's context instead of inner + showModalBottomSheet( + context: context, + builder: (BuildContext outerBuilderCtx) { + return Builder( + builder: (BuildContext innerBuilderCtx) { + /// expect_lint: use_nearest_context + return SizedBox.fromSize(size: outerBuilderCtx.size); + }, + ); + }, + ); + + /// Callback without BuildContext inside builder: + /// should lint when using outer context through a non-BuildContext callback + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + Future.microtask(() { + /// expect_lint: use_nearest_context + Navigator.of(context).pop(); + }); + + return const SizedBox.shrink(); + }, + ); + + /// Allowed: callback without BuildContext using nearest builder's context + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + Future.microtask(() { + /// Allowed: innerContext is the nearest BuildContext + Navigator.of(innerContext).pop(); + }); + + return const SizedBox.shrink(); + }, + ); +} + +/// Check the `use_nearest_context` rule with class method context +class UseNearestContextTest extends StatefulWidget { + @override + State createState() => _UseNearestContextTestState(); + + const UseNearestContextTest({super.key}); +} + +class _UseNearestContextTestState extends State { + @override + Widget build(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext _) { + /// expect_lint: use_nearest_context + return SizedBox.fromSize(size: context.size); + }, + ); + + /// Allowed: using method's own BuildContext directly + return SizedBox.fromSize(size: context.size); + } +} + +/// Check the `use_nearest_context` rule within a constructor body. +class QuickFixBrokenTest { + QuickFixBrokenTest(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext _) { + /// expect_lint: use_nearest_context + return SizedBox.fromSize(size: context.size); + }, + ); + } +} + +/// False positive checks: none of these should trigger the lint. +class FalsePositiveTest extends StatefulWidget { + const FalsePositiveTest({super.key}); + + @override + State createState() => _FalsePositiveTestState(); +} + +class _FalsePositiveTestState extends State { + @override + Widget build(BuildContext context) { + /// Case 1: Local variable assigned from nearest context — + /// should NOT trigger, but currently does (false positive). + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + final BuildContext localContext = innerContext; + + /// Allowed: localContext is assigned from innerContext + return SizedBox.fromSize(size: localContext.size); + }, + ); + + /// Case 2: Property of another object — state.context + /// should NOT trigger, but currently does (false positive). + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + final state = + (innerContext as Element).findAncestorStateOfType()!; + + /// Allowed: state.context is a property access, not a direct use + return SizedBox.fromSize(size: state.context.size); + }, + ); + + /// Case 3: this.context inside a builder — SHOULD trigger, + /// because this.context is the outer scope's BuildContext. + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + /// expect_lint: use_nearest_context + return SizedBox.fromSize(size: this.context.size); + }, + ); + + return const SizedBox.shrink(); + } +} diff --git a/pubspec.yaml b/pubspec.yaml index 80a1415e..15e32450 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -2,7 +2,7 @@ name: solid_lints description: Lints for Dart and Flutter based on software industry standards and best practices. -version: 0.3.3 +version: 0.3.4 homepage: https://github.com/solid-software/solid_lints/ documentation: https://solid-software.github.io/solid_lints/docs/intro topics: [lints, linter, lint, analysis, analyzer]