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/main.dart b/lib/main.dart index 018d598f..8853debf 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -13,6 +13,9 @@ import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/fixes/av import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart'; import 'package:solid_lints/src/lints/double_literal_format/fixes/double_literal_format_fix.dart'; import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/fixes/rename_nearest_context_parameter_fix.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/fixes/replace_with_nearest_context_parameter_fix.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/use_nearest_context_rule.dart'; /// The entry point for the Solid Lints analyser server plugin. /// @@ -47,6 +50,7 @@ class SolidLintsPlugin extends Plugin { analysisOptionsLoader: analysisLoader, parametersParser: AvoidReturningWidgetsParameters.fromJson, ), + UseNearestContextRule(), ]; for (final lintRule in lintRules) { @@ -61,9 +65,20 @@ class SolidLintsPlugin extends Plugin { AvoidFinalWithGetterRule.code, AvoidFinalWithGetterFix.new, ); + registry.registerFixForRule( avoidUnnecessaryTypeAssertionsRule.diagnosticCode, AvoidUnnecessaryTypeAssertionsFix.new, ); + + registry.registerFixForRule( + UseNearestContextRule.code, + RenameNearestContextParameterFix.new, + ); + + registry.registerFixForRule( + UseNearestContextRule.code, + ReplaceWithNearestContextParameterFix.new, + ); } } diff --git a/lib/src/lints/use_nearest_context/fixes/rename_nearest_context_parameter_fix.dart b/lib/src/lints/use_nearest_context/fixes/rename_nearest_context_parameter_fix.dart new file mode 100644 index 00000000..8f10e84b --- /dev/null +++ b/lib/src/lints/use_nearest_context/fixes/rename_nearest_context_parameter_fix.dart @@ -0,0 +1,58 @@ +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/dart/ast/ast.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/use_nearest_context_rule.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/utils/use_nearest_context_utils.dart'; + +/// A Quick fix for [UseNearestContextRule] rule +/// Suggests to rename the nearest BuildContext parameter +/// to the one that is being used. +class RenameNearestContextParameterFix extends ResolvedCorrectionProducer { + static const _renameParameterKind = FixKind( + 'solid_lints.fix.${UseNearestContextRule.lintName}.rename_parameter', + DartFixKindPriority.standard, + "Rename nearest BuildContext parameter", + ); + + /// Creates a new instance of [RenameNearestContextParameterFix]. + RenameNearestContextParameterFix({required super.context}); + + @override + FixKind get fixKind => _renameParameterKind; + + @override + CorrectionApplicability get applicability => + CorrectionApplicability.automatically; + + @override + Future compute(ChangeBuilder builder) async { + final identifierNode = node; + if (identifierNode is! SimpleIdentifier) return; + + // Do not offer renaming the parameter if this is an access on `this` + // or `super`. + final parent = identifierNode.parent; + if (parent is PropertyAccess) { + var target = parent.target; + while (target is ParenthesizedExpression) { + target = target.expression; + } + if (target is ThisExpression || target is SuperExpression) return; + } + + final closestBuildContext = findClosestBuildContext(identifierNode); + if (closestBuildContext == null) return; + + final parameterName = closestBuildContext.name; + if (parameterName == null) return; + + await builder.addDartFileEdit(file, (builder) { + builder.addSimpleReplacement( + parameterName.sourceRange, + identifierNode.name, + ); + }); + } +} diff --git a/lib/src/lints/use_nearest_context/fixes/replace_with_nearest_context_parameter_fix.dart b/lib/src/lints/use_nearest_context/fixes/replace_with_nearest_context_parameter_fix.dart new file mode 100644 index 00000000..8baaa491 --- /dev/null +++ b/lib/src/lints/use_nearest_context/fixes/replace_with_nearest_context_parameter_fix.dart @@ -0,0 +1,68 @@ +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/dart/ast/ast.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/use_nearest_context_rule.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/utils/use_nearest_context_utils.dart'; + +/// A Quick fix for [UseNearestContextRule] rule +/// Suggests to replace the outer BuildContext expression +/// with the nearest available parameter. +class ReplaceWithNearestContextParameterFix extends ResolvedCorrectionProducer { + static const _replaceExpressionKind = FixKind( + 'solid_lints.fix.${UseNearestContextRule.lintName}.replace_expression', + DartFixKindPriority.standard, + "Replace with nearest BuildContext parameter", + ); + + /// Creates a new instance of [ReplaceWithNearestContextParameterFix]. + ReplaceWithNearestContextParameterFix({required super.context}); + + @override + FixKind get fixKind => _replaceExpressionKind; + + @override + CorrectionApplicability get applicability => + CorrectionApplicability.automatically; + + @override + Future compute(ChangeBuilder builder) async { + final errorNode = node; + + final closestBuildContext = findClosestBuildContext(errorNode); + if (closestBuildContext == null) return; + + final parameterName = closestBuildContext.name?.lexeme; + if (parameterName == null) return; + + if (errorNode is ThisExpression) { + await builder.addDartFileEdit(file, (builder) { + builder.addSimpleReplacement( + errorNode.sourceRange, + parameterName, + ); + }); + return; + } + + if (errorNode is SimpleIdentifier) { + final parent = errorNode.parent; + if (parent is PropertyAccess) { + var target = parent.target; + while (target is ParenthesizedExpression) { + target = target.expression; + } + if (target is ThisExpression || target is SuperExpression) { + await builder.addDartFileEdit(file, (builder) { + builder.addSimpleReplacement( + parent.sourceRange, + parameterName, + ); + }); + return; + } + } + } + } +} 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..56af784a --- /dev/null +++ b/lib/src/lints/use_nearest_context/use_nearest_context_rule.dart @@ -0,0 +1,82 @@ +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/error/error.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/visitors/use_nearest_context_visitor.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 AnalysisRule { + /// This lint rule represents the error if BuildContext is used not from the + /// nearest available scope + static const lintName = 'use_nearest_context'; + + /// The code to report for a violation + static const LintCode code = LintCode( + lintName, + 'Use the nearest BuildContext parameter instead of the outer one.', + ); + + /// Creates a new instance of [UseNearestContextRule]. + UseNearestContextRule() + : super( + name: lintName, + description: code.problemMessage, + ); + + @override + LintCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = UseNearestContextVisitor(this); + registry.addSimpleIdentifier(this, visitor); + registry.addThisExpression(this, visitor); + } +} diff --git a/lib/src/lints/use_nearest_context/utils/use_nearest_context_utils.dart b/lib/src/lints/use_nearest_context/utils/use_nearest_context_utils.dart new file mode 100644 index 00000000..8e79d9d8 --- /dev/null +++ b/lib/src/lints/use_nearest_context/utils/use_nearest_context_utils.dart @@ -0,0 +1,23 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:solid_lints/src/utils/types_utils.dart'; + +/// Finds the closest BuildContext parameter in the AST parent chain of [node]. +SimpleFormalParameter? findClosestBuildContext(AstNode 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; +} diff --git a/lib/src/lints/use_nearest_context/visitors/use_nearest_context_visitor.dart b/lib/src/lints/use_nearest_context/visitors/use_nearest_context_visitor.dart new file mode 100644 index 00000000..e5886740 --- /dev/null +++ b/lib/src/lints/use_nearest_context/visitors/use_nearest_context_visitor.dart @@ -0,0 +1,37 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/use_nearest_context_rule.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/utils/use_nearest_context_utils.dart'; +import 'package:solid_lints/src/utils/node_utils.dart'; +import 'package:solid_lints/src/utils/types_utils.dart'; + +/// A visitor for [UseNearestContextRule]. +class UseNearestContextVisitor extends SimpleAstVisitor { + final UseNearestContextRule _rule; + + /// Creates a new instance of [UseNearestContextVisitor]. + UseNearestContextVisitor(this._rule); + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + if (!isBuildContext(node.staticType)) return; + if (node.isPropertyOfOtherObject) return; + + final closestBuildContext = findClosestBuildContext(node); + if (closestBuildContext == null) return; + if (closestBuildContext.name?.lexeme == node.name) return; + if (node.isDeclaredInSameFunction(as: closestBuildContext)) return; + + _rule.reportAtNode(node); + } + + @override + void visitThisExpression(ThisExpression node) { + if (!isBuildContext(node.staticType)) return; + + final closestBuildContext = findClosestBuildContext(node); + if (closestBuildContext == null) return; + + _rule.reportAtNode(node); + } +} diff --git a/lib/src/utils/node_utils.dart b/lib/src/utils/node_utils.dart index 3cea0c01..2b008879 100644 --- a/lib/src/utils/node_utils.dart +++ b/lib/src/utils/node_utils.dart @@ -1,5 +1,7 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/element/element.dart'; + /// Check node is override method from its metadata bool isOverride(List metadata) => metadata.any( @@ -22,3 +24,40 @@ String humanReadableNodeType(AstNode? node) { return 'Node'; } + +/// Extension on [SimpleIdentifier] to provide scope and property utility +/// checks. +extension SimpleIdentifierExtension on SimpleIdentifier { + /// Returns `true` if this identifier is a property accessed on another + /// object (e.g. `state.context`), but not on `this` (e.g. `this.context`). + bool get isPropertyOfOtherObject { + final parent = this.parent; + if (parent is PrefixedIdentifier && this == parent.identifier) { + return true; + } + if (parent is PropertyAccess && this == parent.propertyName) { + var target = parent.target; + while (target is ParenthesizedExpression) { + target = target.expression; + } + return target is! ThisExpression && target is! SuperExpression; + } + return false; + } + + /// Returns `true` if this identifier refers to a variable declared inside + /// the body of the function that owns [as] (i.e. a local variable in the + /// same scope). + bool isDeclaredInSameFunction({required SimpleFormalParameter as}) { + final element = this.element; + if (element is! LocalVariableElement) return false; + + final nearestFunction = as.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; + } +} diff --git a/lib/src/utils/types_utils.dart b/lib/src/utils/types_utils.dart index bdea71e5..64558270 100644 --- a/lib/src/utils/types_utils.dart +++ b/lib/src/utils/types_utils.dart @@ -129,8 +129,11 @@ bool isRowWidget(DartType? type) => type?.getDisplayString() == 'Row'; bool isPaddingWidget(DartType? type) => type?.getDisplayString() == 'Padding'; -bool isBuildContext(DartType? type) => - type?.getDisplayString() == 'BuildContext'; +bool isBuildContext(DartType? type) { + if (type == null) return false; + final displayString = type.getDisplayString(); + return displayString == 'BuildContext' || displayString == 'BuildContext?'; +} bool isGameWidget(DartType? type) => type?.getDisplayString() == 'GameWidget'; diff --git a/pubspec.yaml b/pubspec.yaml index b1a226bb..56c55842 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] diff --git a/test/src/lints/use_nearest_context/use_nearest_context_rule_test.dart b/test/src/lints/use_nearest_context/use_nearest_context_rule_test.dart new file mode 100644 index 00000000..070b71e2 --- /dev/null +++ b/test/src/lints/use_nearest_context/use_nearest_context_rule_test.dart @@ -0,0 +1,332 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:analyzer_testing/utilities/utilities.dart'; +import 'package:solid_lints/src/lints/use_nearest_context/use_nearest_context_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../../../lints/auto_test_lint_offsets.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(UseNearestContextRuleTest); + }); +} + +@reflectiveTest +class UseNearestContextRuleTest extends AnalysisRuleTest + with AutoTestLintOffsets { + @override + void setUp() { + rule = UseNearestContextRule(); + newPackage('flutter')..addFile('lib/material.dart', r''' +class BuildContext { + BuildContext get context => this; + Object? get size => null; +} +class Widget {} +void showModalBottomSheet({required BuildContext context, required Widget Function(BuildContext) builder}) {} +class Builder extends Widget { + final Function builder; + Builder({required this.builder}); +} +abstract class Element implements BuildContext { + State? findAncestorStateOfType(); + @override + BuildContext get context => this; + @override + Object? get size => null; +} +abstract class State { + BuildContext get context; +} +class Future { + static void microtask(void Function() callback) {} +} +'''); + super.setUp(); + + newAnalysisOptionsYamlFile( + testPackageRootPath, + analysisOptionsContent(rules: [rule.name]), + ); + } + + Future test_reports_on_outer_context_usage() async { + await assertAutoDiagnostics(''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext context) { + final outerContext = context; + + showModalBottomSheet( + context: context, + builder: (BuildContext _) { + final s = ${expectLint('outerContext')}.size; + return Widget(); + }, + ); +} +'''); + } + + Future test_does_not_report_on_nearest_context_usage() async { + await assertNoDiagnostics(r''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + final s = innerContext.size; + return Widget(); + }, + ); +} +'''); + } + + Future test_does_not_report_on_property_of_other_object() async { + await assertNoDiagnostics(r''' +import 'package:flutter/material.dart'; + +class FalsePositiveTest { + void build(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + final state = (innerContext as Element).findAncestorStateOfType()!; + final s = state.context.size; + return Widget(); + }, + ); + } +} +'''); + } + + Future test_reports_on_this_context_inside_builder() async { + await assertAutoDiagnostics(''' +import 'package:flutter/material.dart'; + +class FalsePositiveTest { + BuildContext get context => throw ''; + + void build(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + final s = this.${expectLint('context')}.size; + return Widget(); + }, + ); + } +} +'''); + } + + Future + test_does_not_report_on_local_variable_assigned_from_nearest_context() async { + await assertNoDiagnostics(r''' +import 'package:flutter/material.dart'; + +class FalsePositiveTest { + void build(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + final localContext = innerContext; + final s = localContext.size; + return Widget(); + }, + ); + } +} +'''); + } + + Future + test_reports_on_outer_builder_context_usage_in_nested_builder() async { + await assertAutoDiagnostics(''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext outerBuilderCtx) { + return Builder( + builder: (BuildContext innerBuilderCtx) { + final s = ${expectLint('outerBuilderCtx')}.size; + return Widget(); + }, + ); + }, + ); +} +'''); + } + + Future test_reports_on_untyped_builder_parameter_usage() async { + await assertAutoDiagnostics(''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (_) { + final s = ${expectLint('context')}.size; + return Widget(); + }, + ); +} +'''); + } + + Future test_reports_on_outer_context_usage_in_async_callback() async { + await assertAutoDiagnostics(''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + Future.microtask(() { + final s = ${expectLint('context')}.size; + }); + return Widget(); + }, + ); +} +'''); + } + + Future + test_does_not_report_on_nearest_context_usage_in_async_callback() async { + await assertNoDiagnostics(r''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext innerContext) { + Future.microtask(() { + final s = innerContext.size; + }); + return Widget(); + }, + ); +} +'''); + } + + Future test_reports_on_constructor_parameter_usage_in_builder() async { + await assertAutoDiagnostics(''' +import 'package:flutter/material.dart'; + +class QuickFixBrokenTest { + QuickFixBrokenTest(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext _) { + final s = ${expectLint('context')}.size; + return Widget(); + }, + ); + } +} +'''); + } + + Future test_does_not_report_on_shadowed_parameter_name_usage() async { + await assertNoDiagnostics(r''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext context) { + final s = context.size; + return Widget(); + }, + ); +} +'''); + } + + Future test_reports_on_nullable_outer_context_usage() async { + await assertAutoDiagnostics(''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext? context) { + showModalBottomSheet( + context: context!, + builder: (BuildContext _) { + final s = ${expectLint('context')}; + return Widget(); + }, + ); +} +'''); + } + + Future test_reports_on_this_context_inside_extension_method() async { + await assertAutoDiagnostics(''' +import 'package:flutter/material.dart'; + +extension BuildContextX on BuildContext { + void someMethod() { + showModalBottomSheet( + context: this, + builder: (BuildContext innerContext) { + final s = ${expectLint('this')}.size; + return Widget(); + }, + ); + } +} +'''); + } + + Future test_does_not_report_on_this_context_outside_builder() async { + await assertNoDiagnostics(r''' +import 'package:flutter/material.dart'; + +extension BuildContextX on BuildContext { + void someMethod() { + final s = this.size; + } +} +'''); + } + + Future test_does_not_report_on_nullable_nearest_context_usage() async { + await assertNoDiagnostics(r''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext? innerContext) { + final s = innerContext?.size; + return Widget(); + }, + ); +} +'''); + } + + Future + test_reports_on_outer_context_usage_with_nullable_nearest_context() async { + await assertAutoDiagnostics(''' +import 'package:flutter/material.dart'; + +void showDialog(BuildContext context) { + showModalBottomSheet( + context: context, + builder: (BuildContext? innerContext) { + final s = ${expectLint('context')}.size; + return Widget(); + }, + ); +} +'''); + } +}