diff --git a/packages/leancode_lint/CHANGELOG.md b/packages/leancode_lint/CHANGELOG.md index 8398ea77..f4db7a3b 100644 --- a/packages/leancode_lint/CHANGELOG.md +++ b/packages/leancode_lint/CHANGELOG.md @@ -1,3 +1,7 @@ +# 24.1.0 + +- Update [`prefer_equatable_mixin`](https://github.com/leancodepl/flutter_corelibrary/tree/master/packages/leancode_lint#prefer_equatable_mixin) to suggest mixing in `Equatable` directly when the linted package depends on `equatable` 2.1.0 or higher, where `EquatableMixin` is deprecated. + # 24.0.0 - Add new custom lints: diff --git a/packages/leancode_lint/README.md b/packages/leancode_lint/README.md index fa473322..06b6f620 100644 --- a/packages/leancode_lint/README.md +++ b/packages/leancode_lint/README.md @@ -826,7 +826,12 @@ None ### `prefer_equatable_mixin` -**DO** mix in `EquatableMixin` instead of extending `Equatable`. +**DO** mix in `Equatable` instead of extending it. + +Since `equatable` 2.1.0, `Equatable` can be used as a mixin and `EquatableMixin` +is deprecated. This lint suggests mixing in `Equatable` directly when the package +depends on `equatable` 2.1.0 or higher, and falls back to suggesting `EquatableMixin` +for older versions. **BAD:** @@ -848,6 +853,17 @@ class Foobar extends Equatable { ```dart import 'package:equatable/equatable.dart'; +// `equatable` >= 2.1.0 +class Foobar with Equatable { + const Foobar(this.value); + + final int value; + + @override + List get props => [value]; +} + +// `equatable` < 2.1.0 class Foobar with EquatableMixin { const Foobar(this.value); diff --git a/packages/leancode_lint/lib/src/lints/missing_equatable_props.dart b/packages/leancode_lint/lib/src/lints/missing_equatable_props.dart index 4c10f3ea..d0d1b4f0 100644 --- a/packages/leancode_lint/lib/src/lints/missing_equatable_props.dart +++ b/packages/leancode_lint/lib/src/lints/missing_equatable_props.dart @@ -242,7 +242,7 @@ List _findMissingFieldNames( /// /// Returns `false` when: /// - the superclass has no Equatable-shaped ancestor (e.g. a plain class that -/// only mixes in `EquatableMixin` — its supertype is [Object]); +/// only mixes in `Equatable`/`EquatableMixin` — its supertype is [Object]); /// - the superclass is `Equatable` or `EquatableMixin` itself (both declare /// `props` as abstract, so `super.props` would target the abstract member); /// - the superclass is an intermediate Equatable-shaped class that does not 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 7ed0096f..91d2011e 100644 --- a/packages/leancode_lint/lib/src/lints/prefer_equatable_mixin.dart +++ b/packages/leancode_lint/lib/src/lints/prefer_equatable_mixin.dart @@ -5,6 +5,7 @@ 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/error/error.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; @@ -17,7 +18,7 @@ class PreferEquatableMixin extends AnalysisRule { static const code = LintCode( 'prefer_equatable_mixin', - 'The class {0} should mix in EquatableMixin instead of extending Equatable.', + 'The class {0} should mix in {1} instead of extending Equatable.', correctionMessage: 'Replace with a mixin.', severity: .WARNING, ); @@ -69,12 +70,23 @@ class _Visitor extends SimpleAstVisitor { if (isEquatable && !isEquatableMixin) { rule.reportAtNode( extendsClause.superclass, - arguments: [node.namePart.typeName.lexeme], + arguments: [ + node.namePart.typeName.lexeme, + _recommendedMixinName(extendsClause.superclass), + ], ); } } } +/// Recommends `Equatable` for `equatable` >= 2.1.0 (where it's a `mixin class`) +/// and the deprecated `EquatableMixin` for older versions. +String _recommendedMixinName(NamedType equatableType) => + switch (equatableType.element) { + ClassElement(isMixinClass: true) => 'Equatable', + _ => 'EquatableMixin', + }; + class ConvertToEquatableMixin extends ResolvedCorrectionProducer { ConvertToEquatableMixin({required super.context}); @@ -82,7 +94,7 @@ class ConvertToEquatableMixin extends ResolvedCorrectionProducer { FixKind get fixKind => const .new( 'leancode_lint.fix.convertToEquatableMixin', DartFixKindPriority.standard, - 'Convert to EquatableMixin', + 'Convert to a mixin', ); @override @@ -93,6 +105,7 @@ class ConvertToEquatableMixin extends ResolvedCorrectionProducer { final classDeclaration = node.thisOrAncestorOfType()!; final extendsClause = classDeclaration.extendsClause!; final withClause = classDeclaration.withClause; + final mixinName = _recommendedMixinName(extendsClause.superclass); await builder.addDartFileEdit(file, (builder) { if (withClause != null) { @@ -103,12 +116,12 @@ class ConvertToEquatableMixin extends ResolvedCorrectionProducer { ) ..addSimpleInsertion( withClause.mixinTypes.first.offset, - 'EquatableMixin, ', + '$mixinName, ', ); } else { builder.addSimpleReplacement( extendsClause.sourceRange, - 'with EquatableMixin', + 'with $mixinName', ); } }); diff --git a/packages/leancode_lint/pubspec.yaml b/packages/leancode_lint/pubspec.yaml index fe004534..fdf4a337 100644 --- a/packages/leancode_lint/pubspec.yaml +++ b/packages/leancode_lint/pubspec.yaml @@ -1,5 +1,5 @@ name: leancode_lint -version: 24.0.0 +version: 24.1.0 homepage: https://github.com/leancodepl/flutter_corelibrary/tree/master/packages/leancode_lint repository: https://github.com/leancodepl/flutter_corelibrary description: Robust and high-quality lint rules used at LeanCode. diff --git a/packages/leancode_lint/test/assert_ranges.dart b/packages/leancode_lint/test/assert_ranges.dart index dce8b936..9f3d43fb 100644 --- a/packages/leancode_lint/test/assert_ranges.dart +++ b/packages/leancode_lint/test/assert_ranges.dart @@ -9,6 +9,7 @@ extension AssertDiagnosticsInRangesX on AnalysisRuleTest { bool positionShorthand = true, bool rangeShorthand = true, bool zeroWidthMarker = true, + List> messageContainsAll = const [], }) { final code = TestCode.parse( content, @@ -17,8 +18,12 @@ extension AssertDiagnosticsInRangesX on AnalysisRuleTest { zeroWidthMarker: zeroWidthMarker, ); return assertDiagnostics(code.code, [ - for (final range in code.ranges) - lint(range.sourceRange.offset, range.sourceRange.length), + for (final (index, range) in code.ranges.indexed) + lint( + range.sourceRange.offset, + range.sourceRange.length, + messageContainsAll: messageContainsAll.elementAtOrNull(index) ?? [], + ), for (final position in code.positions) lint(position.offset, 0), ]); } diff --git a/packages/leancode_lint/test/mock_libraries/equatable.dart b/packages/leancode_lint/test/mock_libraries/equatable.dart index d9d47d92..21f438b6 100644 --- a/packages/leancode_lint/test/mock_libraries/equatable.dart +++ b/packages/leancode_lint/test/mock_libraries/equatable.dart @@ -1,9 +1,31 @@ part of '../mock_libraries.dart'; +/// Mocks `equatable` 2.1.0 or higher, where `Equatable` can be used as a mixin +/// and `EquatableMixin` is deprecated. mixin MockEquatable on AnalysisRuleTest { @override void setUp() { newPackage('equatable').addFile('lib/equatable.dart', ''' +abstract mixin class Equatable { + const Equatable(); + List get props; +} + +@Deprecated('use Equatable as a mixin instead') +mixin EquatableMixin { + List get props; +} +'''); + super.setUp(); + } +} + +/// Mocks `equatable` older than 2.1.0, where `Equatable` cannot be used as a +/// mixin and `EquatableMixin` should be used instead. +mixin MockOldEquatable on AnalysisRuleTest { + @override + void setUp() { + newPackage('equatable').addFile('lib/equatable.dart', ''' class Equatable { const Equatable(); List get props; diff --git a/packages/leancode_lint/test/test_cases/missing_equatable_props_test.dart b/packages/leancode_lint/test/test_cases/missing_equatable_props_test.dart index 15825c28..d2c19938 100644 --- a/packages/leancode_lint/test/test_cases/missing_equatable_props_test.dart +++ b/packages/leancode_lint/test/test_cases/missing_equatable_props_test.dart @@ -24,7 +24,7 @@ class MissingEquatablePropsTest extends AnalysisRuleTest with MockEquatable { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; -class MyState with EquatableMixin { +class MyState with Equatable { MyState(this.a, this.b); final int a; @@ -40,6 +40,24 @@ class MyState with EquatableMixin { await assertDiagnosticsInRanges(''' import 'package:equatable/equatable.dart'; +class MyState with Equatable { + MyState(this.a, this.b, this.c); + + final int a; + final String b; + final double c; + + @override + List get props => /*[0*/[a]/*0]*/; +} +'''); + } + + Future test_missing_fields_in_deprecated_mixin_class() async { + await assertDiagnosticsInRanges(''' +import 'package:equatable/equatable.dart'; + +// ignore: deprecated_member_use class MyState with EquatableMixin { MyState(this.a, this.b, this.c); @@ -73,7 +91,7 @@ class MyState extends Equatable { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; -class MyState with EquatableMixin { +class MyState with Equatable { MyState(this.a); static const int unused = 0; @@ -89,7 +107,7 @@ class MyState with EquatableMixin { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; -class MyState with EquatableMixin { +class MyState with Equatable { MyState(this.a); final int a; @@ -105,7 +123,7 @@ class MyState with EquatableMixin { await assertDiagnosticsInRanges(''' import 'package:equatable/equatable.dart'; -class MyState with EquatableMixin { +class MyState with Equatable { MyState(this.a, this.b); final int a; @@ -135,7 +153,7 @@ class Plain { await assertDiagnosticsInRanges(''' import 'package:equatable/equatable.dart'; -class MyState with EquatableMixin { +class MyState with Equatable { MyState(this.a, this.b); final int a, b; @@ -150,7 +168,7 @@ class MyState with EquatableMixin { await assertDiagnosticsInRanges(''' import 'package:equatable/equatable.dart'; -class MyState with EquatableMixin { +class MyState with Equatable { MyState(this.a, this.onTap); final int a; @@ -166,7 +184,7 @@ class MyState with EquatableMixin { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; -class MyState with EquatableMixin { +class MyState with Equatable { MyState(this.a, this.b); final int a; @@ -182,7 +200,7 @@ class MyState with EquatableMixin { await assertDiagnosticsInRanges(''' import 'package:equatable/equatable.dart'; -class Parent with EquatableMixin { +class Parent with Equatable { Parent(this.a); final int a; @@ -206,7 +224,7 @@ class Sub extends Parent { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; -class Parent with EquatableMixin { +class Parent with Equatable { Parent(this.a); final int a; @@ -230,7 +248,7 @@ class Sub extends Parent { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; -class Parent with EquatableMixin { +class Parent with Equatable { Parent(this.a); final int a; @@ -250,7 +268,8 @@ class Sub extends Parent { '''); } - Future test_super_props_not_suggested_for_direct_equatable_subclass() async { + Future + test_super_props_not_suggested_for_direct_equatable_subclass() async { await assertDiagnosticsInRanges(''' import 'package:equatable/equatable.dart'; @@ -266,7 +285,8 @@ class MyState extends Equatable { '''); } - Future test_super_props_not_suggested_when_parent_has_no_concrete_props() async { + Future + test_super_props_not_suggested_when_parent_has_no_concrete_props() async { await assertDiagnosticsInRanges(''' import 'package:equatable/equatable.dart'; @@ -286,7 +306,8 @@ class Sub extends AbstractBase { '''); } - Future test_no_diagnostic_when_parent_has_no_concrete_props_and_all_fields_listed() async { + Future + test_no_diagnostic_when_parent_has_no_concrete_props_and_all_fields_listed() async { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; @@ -306,7 +327,8 @@ class Sub extends AbstractBase { '''); } - Future test_super_props_suggested_when_abstract_parent_has_concrete_props() async { + Future + test_super_props_suggested_when_abstract_parent_has_concrete_props() async { await assertDiagnosticsInRanges(''' import 'package:equatable/equatable.dart'; @@ -334,7 +356,7 @@ class Sub extends AbstractBase { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; -class MyState with EquatableMixin { +class MyState with Equatable { MyState(this.a, this.b); final int a; @@ -352,7 +374,7 @@ class MyState with EquatableMixin { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; -class MyState with EquatableMixin { +class MyState with Equatable { MyState(this.a, this.b); final int a; diff --git a/packages/leancode_lint/test/test_cases/prefer_equatable_mixin_test.dart b/packages/leancode_lint/test/test_cases/prefer_equatable_mixin_test.dart index a839bbfe..bd7dd99a 100644 --- a/packages/leancode_lint/test/test_cases/prefer_equatable_mixin_test.dart +++ b/packages/leancode_lint/test/test_cases/prefer_equatable_mixin_test.dart @@ -8,6 +8,7 @@ import '../mock_libraries.dart'; void main() { defineReflectiveSuite(() { defineReflectiveTests(PreferEquatableMixinTest); + defineReflectiveTests(PreferEquatableMixinWithOldEquatableTest); }); } @@ -40,6 +41,18 @@ class MyState2 extends MyState { await assertNoDiagnostics(''' import 'package:equatable/equatable.dart'; +class MyState3 with Equatable { + @override + List get props => []; +} +'''); + } + + Future test_deprecated_equatable_mixin_still_legal() async { + await assertNoDiagnostics(''' +import 'package:equatable/equatable.dart'; + +// ignore: deprecated_member_use class MyState3 with EquatableMixin { @override List get props => []; @@ -63,10 +76,82 @@ class MyState2 extends MyState with SomethingElse { List get props => []; } -class MyState3 with SomethingElse, EquatableMixin { +class MyState3 with SomethingElse, Equatable { @override List get props => []; } '''); } + + Future test_recommends_equatable() async { + await assertDiagnosticsInRanges( + ''' +import 'package:equatable/equatable.dart'; + +class MyState extends [!Equatable!] { + @override + List get props => []; +} +''', + messageContainsAll: const [ + ['mix in Equatable instead'], + ], + ); + } +} + +/// Tests the behavior with `equatable` older than 2.1.0, where `Equatable` +/// cannot be used as a mixin and `EquatableMixin` should be used instead. +@reflectiveTest +class PreferEquatableMixinWithOldEquatableTest extends AnalysisRuleTest + with MockOldEquatable { + @override + void setUp() { + rule = PreferEquatableMixin(); + + super.setUp(); + } + + Future test_only_directly_extending_equatable() async { + await assertDiagnosticsInRanges(''' +import 'package:equatable/equatable.dart'; + +class MyState extends [!Equatable!] { + @override + List get props => []; +} + +class MyState2 extends MyState { + @override + List get props => []; +} +'''); + } + + Future test_mixin_not_flagged() async { + await assertNoDiagnostics(''' +import 'package:equatable/equatable.dart'; + +class MyState3 with EquatableMixin { + @override + List get props => []; +} +'''); + } + + Future test_recommends_equatable_mixin() async { + await assertDiagnosticsInRanges( + ''' +import 'package:equatable/equatable.dart'; + +class MyState extends [!Equatable!] { + @override + List get props => []; +} +''', + messageContainsAll: const [ + ['mix in EquatableMixin instead'], + ], + ); + } }