diff --git a/packages/leancode_lint/.fvmrc b/packages/leancode_lint/.fvmrc index 5c08b3bc..a4c4660e 100644 --- a/packages/leancode_lint/.fvmrc +++ b/packages/leancode_lint/.fvmrc @@ -1,3 +1,3 @@ { - "flutter": "3.38.1" + "flutter": "3.41.2" } \ No newline at end of file diff --git a/packages/leancode_lint/README.md b/packages/leancode_lint/README.md index a59854a2..1241b027 100644 --- a/packages/leancode_lint/README.md +++ b/packages/leancode_lint/README.md @@ -219,7 +219,7 @@ None.
-`bloc_related_class_naming` +bloc_related_class_naming ### `bloc_related_class_naming` @@ -392,6 +392,58 @@ None.
+
+never_discard_build_context + +### `never_discard_build_context` + +**AVOID** discarding `BuildContext` parameters by naming them `_`. + +Discarding a `BuildContext` causes the body to use an ancestor context instead, +risking incorrect theme lookups, navigation, or inherited widget reads. + +**BAD:** + +```dart +Widget myBuilder(BuildContext _) { + return const SizedBox(); +} +``` + +**BAD:** + +```dart +Widget build(BuildContext context) { + return Builder( + builder: (_) => const SizedBox(), + ); +} +``` + +**GOOD:** + +```dart +Widget myBuilder(BuildContext context) { + return const SizedBox(); +} +``` + +**GOOD:** + +```dart +Widget build(BuildContext context) { + return Builder( + builder: (context) => const SizedBox(), + ); +} +``` + +#### Configuration + +None. + +
+
prefix_widgets_returning_slivers diff --git a/packages/leancode_lint/lib/plugin.dart b/packages/leancode_lint/lib/plugin.dart index 734b2bf0..e32f8a02 100644 --- a/packages/leancode_lint/lib/plugin.dart +++ b/packages/leancode_lint/lib/plugin.dart @@ -11,6 +11,7 @@ import 'package:leancode_lint/src/lints/bloc_related_class_naming.dart'; import 'package:leancode_lint/src/lints/catch_parameter_names.dart'; import 'package:leancode_lint/src/lints/constructor_parameters_and_fields_should_have_the_same_order.dart'; import 'package:leancode_lint/src/lints/hook_widget_does_not_use_hooks.dart'; +import 'package:leancode_lint/src/lints/never_discard_build_context.dart'; import 'package:leancode_lint/src/lints/prefer_equatable_mixin.dart'; import 'package:leancode_lint/src/lints/prefix_widgets_returning_slivers.dart'; import 'package:leancode_lint/src/lints/start_comments_with_space.dart'; @@ -63,6 +64,11 @@ final class LeanCodeLintPlugin extends Plugin { HookWidgetDoesNotUseHooks.code, ConvertHookWidgetToStatelessWidget.new, ) + ..registerWarningRule(NeverDiscardBuildContext()) + ..registerFixForRule( + NeverDiscardBuildContext.code, + RenameDiscardedBuildContextFix.new, + ) // TODO: disabled by default until stabilized. Add documentation. ..registerLintRule(ConstructorParametersAndFieldsShouldHaveTheSameOrder()) ..registerWarningRule(AvoidSingleChildInMultiChildWidgets()) diff --git a/packages/leancode_lint/lib/src/helpers.dart b/packages/leancode_lint/lib/src/helpers.dart index 9ebd49fa..671c2d5f 100644 --- a/packages/leancode_lint/lib/src/helpers.dart +++ b/packages/leancode_lint/lib/src/helpers.dart @@ -330,7 +330,7 @@ abstract class ChangeWidgetNameFix extends ResolvedCorrectionProducer { final String widgetName; @override - FixKind? get fixKind => .new( + FixKind get fixKind => .new( 'leancode_lint.fix.replaceWith$widgetName', DartFixKindPriority.standard, 'Replace with $widgetName', diff --git a/packages/leancode_lint/lib/src/lints/hook_widget_does_not_use_hooks.dart b/packages/leancode_lint/lib/src/lints/hook_widget_does_not_use_hooks.dart index 01fc755f..1173ed37 100644 --- a/packages/leancode_lint/lib/src/lints/hook_widget_does_not_use_hooks.dart +++ b/packages/leancode_lint/lib/src/lints/hook_widget_does_not_use_hooks.dart @@ -53,7 +53,7 @@ class ConvertHookWidgetToStatelessWidget extends ResolvedCorrectionProducer { ConvertHookWidgetToStatelessWidget({required super.context}); @override - FixKind? get fixKind => const FixKind( + FixKind get fixKind => const .new( 'leancode_lint.fix.convertHookWidgetToStatelessWidget', DartFixKindPriority.standard, 'Convert HookWidget to StatelessWidget', diff --git a/packages/leancode_lint/lib/src/lints/never_discard_build_context.dart b/packages/leancode_lint/lib/src/lints/never_discard_build_context.dart new file mode 100644 index 00000000..c92260c6 --- /dev/null +++ b/packages/leancode_lint/lib/src/lints/never_discard_build_context.dart @@ -0,0 +1,97 @@ +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/type_checker.dart'; + +/// Warns when a BuildContext parameter is explicitly discarded using `_`. +/// +/// Discarding a context causes the body to use an ancestor context instead, +/// risking incorrect theme lookups, navigation, or inherited widget reads. +class NeverDiscardBuildContext extends AnalysisRule { + NeverDiscardBuildContext() + : super(name: code.lowerCaseName, description: code.problemMessage); + + static const code = LintCode( + 'never_discard_build_context', + "Don't discard BuildContext parameters.", + correctionMessage: + 'Give the BuildContext parameter a name to avoid accidentally using an ancestor context.', + severity: .WARNING, + ); + + @override + LintCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + registry.addFormalParameterList(this, _Visitor(this)); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor(this.rule); + + final AnalysisRule rule; + + static const _buildContextChecker = TypeChecker.fromName( + 'BuildContext', + packageName: 'flutter', + ); + + @override + void visitFormalParameterList(FormalParameterList node) { + node.parameters.forEach(_checkParameter); + } + + void _checkParameter(FormalParameter parameter) { + final name = parameter.name; + if (name == null || name.lexeme != '_') { + return; + } + + final type = parameter.declaredFragment?.element.type; + if (type == null) { + return; + } + + if (_buildContextChecker.isExactlyType(type)) { + rule.reportAtToken(name); + } + } +} + +class RenameDiscardedBuildContextFix extends ResolvedCorrectionProducer { + RenameDiscardedBuildContextFix({required super.context}); + + @override + FixKind get fixKind => const .new( + 'leancode_lint.fix.renameDiscardedBuildContext', + DartFixKindPriority.standard, + "Rename to 'context'", + ); + + @override + CorrectionApplicability get applicability => .automatically; + + @override + Future compute(ChangeBuilder builder) async { + await builder.addDartFileEdit( + file, + (builder) => builder.addSimpleReplacement( + range.diagnostic(diagnostic!), + 'context', + ), + ); + } +} diff --git a/packages/leancode_lint/lib/src/lints/prefer_equatable_mixin.dart b/packages/leancode_lint/lib/src/lints/prefer_equatable_mixin.dart index 69d50157..7ed0096f 100644 --- a/packages/leancode_lint/lib/src/lints/prefer_equatable_mixin.dart +++ b/packages/leancode_lint/lib/src/lints/prefer_equatable_mixin.dart @@ -79,7 +79,7 @@ class ConvertToEquatableMixin extends ResolvedCorrectionProducer { ConvertToEquatableMixin({required super.context}); @override - FixKind? get fixKind => const FixKind( + FixKind get fixKind => const .new( 'leancode_lint.fix.convertToEquatableMixin', DartFixKindPriority.standard, 'Convert to EquatableMixin', diff --git a/packages/leancode_lint/lib/src/lints/start_comments_with_space.dart b/packages/leancode_lint/lib/src/lints/start_comments_with_space.dart index 5ad7c414..5eaf84f4 100644 --- a/packages/leancode_lint/lib/src/lints/start_comments_with_space.dart +++ b/packages/leancode_lint/lib/src/lints/start_comments_with_space.dart @@ -91,7 +91,7 @@ class AddStartingSpaceToComment extends ResolvedCorrectionProducer { AddStartingSpaceToComment({required super.context}); @override - FixKind? get fixKind => const .new( + FixKind get fixKind => const .new( 'leancode_lint.fix.addStartingSpaceToComment', DartFixKindPriority.standard, 'Add leading space to comment', diff --git a/packages/leancode_lint/lib/src/lints/use_dedicated_media_query_methods.dart b/packages/leancode_lint/lib/src/lints/use_dedicated_media_query_methods.dart index a6c6a62d..d1a29773 100644 --- a/packages/leancode_lint/lib/src/lints/use_dedicated_media_query_methods.dart +++ b/packages/leancode_lint/lib/src/lints/use_dedicated_media_query_methods.dart @@ -147,7 +147,7 @@ class ReplaceMediaQueryOfWithDedicatedMethodFix ReplaceMediaQueryOfWithDedicatedMethodFix({required super.context}); @override - FixKind? get fixKind => const .new( + FixKind get fixKind => const .new( 'leancode_lint.fix.replaceMediaQueryOfWithDedicatedMethod', DartFixKindPriority.standard, 'Replace with the dedicated MediaQuery method', diff --git a/packages/leancode_lint/lib/src/lints/use_padding.dart b/packages/leancode_lint/lib/src/lints/use_padding.dart index 91c92ac3..f916b238 100644 --- a/packages/leancode_lint/lib/src/lints/use_padding.dart +++ b/packages/leancode_lint/lib/src/lints/use_padding.dart @@ -65,7 +65,7 @@ class UsePaddingFix extends ResolvedCorrectionProducer { UsePaddingFix({required super.context}); @override - FixKind? get fixKind => const .new( + FixKind get fixKind => const .new( 'leancode_lint.fix.usePadding', DartFixKindPriority.standard, 'Replace with Padding', diff --git a/packages/leancode_lint/test/test_cases/never_discard_build_context_test.dart b/packages/leancode_lint/test/test_cases/never_discard_build_context_test.dart new file mode 100644 index 00000000..c8443e6a --- /dev/null +++ b/packages/leancode_lint/test/test_cases/never_discard_build_context_test.dart @@ -0,0 +1,211 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:leancode_lint/src/lints/never_discard_build_context.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../assert_ranges.dart'; +import '../mock_libraries.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(NeverDiscardBuildContextTest); + }); +} + +@reflectiveTest +class NeverDiscardBuildContextTest extends AnalysisRuleTest with MockFlutter { + @override + void setUp() { + rule = NeverDiscardBuildContext(); + + super.setUp(); + } + + Future test_namedParameter_flagged() async { + await assertDiagnosticsInRanges(''' +import 'package:flutter/material.dart'; + +Widget myBuilder(BuildContext [!_!]) { + return const SizedBox(); +} +'''); + } + + Future test_builderCallback_flagged() async { + await assertDiagnosticsInRanges(''' +import 'package:flutter/material.dart'; + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { + return Builder( + builder: (BuildContext [!_!]) => const SizedBox(), + ); + } +} +'''); + } + + Future test_builderCallback_inferredType_flagged() async { + await assertDiagnosticsInRanges(''' +import 'package:flutter/material.dart'; + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { + return Builder( + builder: ([!_!]) => const SizedBox(), + ); + } +} +'''); + } + + Future test_withExtraParameters_flagged() async { + await assertDiagnosticsInRanges(''' +import 'package:flutter/material.dart'; + +Widget myBuilder(BuildContext [!_!], Widget? child) { + return child ?? const SizedBox(); +} +'''); + } + + Future test_namedContext_ok() async { + await assertNoDiagnostics(''' +import 'package:flutter/material.dart'; + +Widget myBuilder(BuildContext context) { + return const SizedBox(); +} +'''); + } + + Future test_builderCallback_namedContext_ok() async { + await assertNoDiagnostics(''' +import 'package:flutter/material.dart'; + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { + return Builder( + builder: (context) => const SizedBox(), + ); + } +} +'''); + } + + Future test_buildMethod_ok() async { + await assertNoDiagnostics(''' +import 'package:flutter/material.dart'; + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { + return const SizedBox(); + } +} +'''); + } + + Future test_discardedNonBuildContext_ok() async { + await assertNoDiagnostics(''' +void doSomething(int _) {} +void doSomethingElse(String _) {} +'''); + } + + Future test_multipleParameters_onlyContextDiscarded_flagged() async { + await assertDiagnosticsInRanges(''' +import 'package:flutter/material.dart'; + +typedef WidgetBuilderWithChild = Widget Function(BuildContext context, Widget? child); + +Widget myBuilder(BuildContext [!_!], Widget? child) { + return child ?? const SizedBox(); +} +'''); + } + + Future test_multipleWildcards_onlyContextFlagged() async { + await assertDiagnosticsInRanges(''' +import 'package:flutter/material.dart'; + +Widget myBuilder(BuildContext [!_!], int _) { + return const SizedBox(); +} +'''); + } + + Future test_functionTypeInParameter_flagged() async { + await assertDiagnosticsInRanges(''' +import 'package:flutter/material.dart'; + +void run(Widget Function(BuildContext [!_!]) builder) {} +'''); + } + + Future test_localFunction_flagged() async { + await assertDiagnosticsInRanges(''' +import 'package:flutter/material.dart'; + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { + Widget inner(BuildContext [!_!]) => const SizedBox(); + return inner(context); + } +} +'''); + } + + Future test_namedUnderscore_nonContext_ok() async { + await assertNoDiagnostics(''' +import 'package:flutter/material.dart'; + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { + return Builder( + builder: (ctx) { + final result = [1, 2, 3].map((_) => const SizedBox()).toList(); + return Column(children: result); + }, + ); + } +} +'''); + } + + Future test_multipleBuilders_allFlagged() async { + await assertDiagnosticsInRanges(''' +import 'package:flutter/material.dart'; + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { + return Column( + children: [ + Builder(builder: (BuildContext /*[0*/_/*0]*/) => const SizedBox()), + Builder(builder: (BuildContext /*[1*/_/*1]*/) => const SizedBox()), + ], + ); + } +} +'''); + } +}