diff --git a/lib/main.dart b/lib/main.dart index f2bab9aa..7bdc9cdc 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -10,6 +10,8 @@ import 'package:solid_lints/src/lints/avoid_returning_widgets/avoid_returning_wi import 'package:solid_lints/src/lints/avoid_returning_widgets/models/avoid_returning_widgets_parameters.dart'; 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/prefer_first/fixes/prefer_first_fix.dart'; +import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart'; import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; /// The entry point for the Solid Lints analyser server plugin. @@ -31,6 +33,8 @@ class SolidLintsPlugin extends Plugin { final analysisLoader = AnalysisOptionsLoader(); final doubleLiteralFormatRule = DoubleLiteralFormatRule(); + final preferFirstRule = PreferFirstRule(); + final lintRules = [ AvoidFinalWithGetterRule(), AvoidGlobalStateRule(), @@ -42,6 +46,10 @@ class SolidLintsPlugin extends Plugin { analysisOptionsLoader: analysisLoader, parametersParser: AvoidReturningWidgetsParameters.fromJson, ), + preferFirstRule, + // TODO: Add more lint rules and use analysisLoader + // for rules that need parameters + // For example: `CyclomaticComplexityRule(analysisLoader)` ]; for (final lintRule in lintRules) { @@ -56,5 +64,9 @@ class SolidLintsPlugin extends Plugin { AvoidFinalWithGetterRule.code, AvoidFinalWithGetterFix.new, ); + registry.registerFixForRule( + preferFirstRule.diagnosticCode, + PreferFirstFix.new, + ); } } diff --git a/lib/src/lints/prefer_first/fixes/prefer_first_fix.dart b/lib/src/lints/prefer_first/fixes/prefer_first_fix.dart index d677ffde..0dad920b 100644 --- a/lib/src/lints/prefer_first/fixes/prefer_first_fix.dart +++ b/lib/src/lints/prefer_first/fixes/prefer_first_fix.dart @@ -1,67 +1,70 @@ +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/diagnostic/diagnostic.dart'; -import 'package:analyzer/source/source_range.dart'; -import 'package:custom_lint_builder/custom_lint_builder.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/prefer_first/prefer_first_rule.dart'; /// A Quick fix for `prefer_first` rule /// Suggests to replace iterable access expressions -class PreferFirstFix extends DartFix { +class PreferFirstFix extends ParsedCorrectionProducer { static const _replaceComment = "Replace with 'first'."; + /// Creates a new instance of [PreferFirstFix] + PreferFirstFix({required super.context}); + @override - void run( - CustomLintResolver resolver, - ChangeReporter reporter, - CustomLintContext context, - Diagnostic analysisError, - List others, - ) { - context.registry.addMethodInvocation((node) { - if (analysisError.sourceRange.intersects(node.sourceRange)) { - final correction = _createCorrection(node); + FixKind get fixKind => const FixKind( + 'solid_lints.fix.${PreferFirstRule.lintName}', + DartFixKindPriority.standard, + _replaceComment, + ); - _addReplacement(reporter, node, correction); - } - }); + @override + FixKind get multiFixKind => const FixKind( + 'solid_lints.fix.multi.${PreferFirstRule.lintName}', + DartFixKindPriority.standard, + '$_replaceComment across files', + ); - context.registry.addIndexExpression((node) { - if (analysisError.sourceRange.intersects(node.sourceRange)) { - final correction = _createCorrection(node); + @override + CorrectionApplicability get applicability => + CorrectionApplicability.automatically; - _addReplacement(reporter, node, correction); - } - }); + @override + Future compute(ChangeBuilder builder) async { + final targetNode = node.thisOrAncestorMatching( + (n) => n is MethodInvocation || n is IndexExpression, + ); + if (targetNode is! Expression) return; + + final correction = _createCorrection(targetNode); + await _addReplacement(builder, targetNode, correction); } String _createCorrection(Expression expression) { - if (expression is MethodInvocation) { - return expression.isCascaded - ? '..first' - : '${expression.target ?? ''}.first'; - } else if (expression is IndexExpression) { - return expression.isCascaded - ? '..first' - : '${expression.target ?? ''}.first'; - } else { - return '.first'; + switch (expression) { + case MethodInvocation(isCascaded: true, :final isNullAware): + case IndexExpression(isCascaded: true, :final isNullAware): + return isNullAware ? '?.first' : '..first'; + + case MethodInvocation(:final target?, :final isNullAware): + case IndexExpression(:final target?, :final isNullAware): + return isNullAware ? '$target?.first' : '$target.first'; + + default: + return '.first'; } } - void _addReplacement( - ChangeReporter reporter, - Expression node, + Future _addReplacement( + ChangeBuilder builder, + AstNode node, String correction, - ) { - final changeBuilder = reporter.createChangeBuilder( - message: _replaceComment, - priority: 1, + ) async { + await builder.addDartFileEdit( + file, + (builder) => builder.addSimpleReplacement(node.sourceRange, correction), ); - - changeBuilder.addDartFileEdit((builder) { - builder.addSimpleReplacement( - SourceRange(node.offset, node.length), - correction, - ); - }); } } diff --git a/lib/src/lints/prefer_first/prefer_first_rule.dart b/lib/src/lints/prefer_first/prefer_first_rule.dart index 0af19607..b644cffe 100644 --- a/lib/src/lints/prefer_first/prefer_first_rule.dart +++ b/lib/src/lints/prefer_first/prefer_first_rule.dart @@ -1,8 +1,7 @@ -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/lints/prefer_first/fixes/prefer_first_fix.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/prefer_first/visitors/prefer_first_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// Warns about usage of iterable[0] or iterable.elementAt(0) instead of @@ -31,37 +30,23 @@ class PreferFirstRule extends SolidLintRule { /// parameters reaches the maximum value. static const lintName = 'prefer_first'; - PreferFirstRule._(super.config); + static const _code = LintCode( + lintName, + "Use first instead of accessing the element at zero index.", + ); - /// Creates a new instance of [PreferFirstRule] - /// based on the lint configuration. - factory PreferFirstRule.createRule(CustomLintConfigs configs) { - final config = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (value) => - 'Use first instead of accessing the element at zero index.', - ); + @override + LintCode get diagnosticCode => _code; - return PreferFirstRule._(config); - } + /// Creates a new instance of [PreferFirstRule] + PreferFirstRule() : super(name: lintName, description: _code.problemMessage); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addCompilationUnit((node) { - final visitor = PreferFirstVisitor(); - node.accept(visitor); - - for (final element in visitor.expressions) { - reporter.atNode(element, code); - } - }); + final visitor = PreferFirstVisitor(this); + registry.addCompilationUnit(this, visitor); } - - @override - List getFixes() => [PreferFirstFix()]; } diff --git a/lib/src/lints/prefer_first/visitors/prefer_first_visitor.dart b/lib/src/lints/prefer_first/visitors/prefer_first_visitor.dart index 984d4515..222734c6 100644 --- a/lib/src/lints/prefer_first/visitors/prefer_first_visitor.dart +++ b/lib/src/lints/prefer_first/visitors/prefer_first_visitor.dart @@ -1,14 +1,15 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart'; import 'package:solid_lints/src/utils/types_utils.dart'; /// The AST visitor that will collect all Iterable access expressions /// which can be replaced with .first class PreferFirstVisitor extends RecursiveAstVisitor { - final _expressions = []; + final PreferFirstRule _rule; - /// List of all Iterable access expressions - Iterable get expressions => _expressions; + /// Creates a new instance of [PreferFirstVisitor] + PreferFirstVisitor(this._rule); @override void visitMethodInvocation(MethodInvocation node) { @@ -16,12 +17,11 @@ class PreferFirstVisitor extends RecursiveAstVisitor { final isIterable = isIterableOrSubclass(node.realTarget?.staticType); final isElementAt = node.methodName.name == 'elementAt'; - if (isIterable && isElementAt) { - final arg = node.argumentList.arguments.first; + if (!isIterable || !isElementAt) return; - if (arg is IntegerLiteral && arg.value == 0) { - _expressions.add(node); - } + final arg = node.argumentList.arguments.firstOrNull; + if (arg case IntegerLiteral(value: 0)) { + _rule.reportAtNode(node); } } @@ -29,12 +29,12 @@ class PreferFirstVisitor extends RecursiveAstVisitor { void visitIndexExpression(IndexExpression node) { super.visitIndexExpression(node); - if (isListOrSubclass(node.realTarget.staticType)) { - final index = node.index; + if (!isListOrSubclass(node.realTarget.staticType)) return; - if (index is IntegerLiteral && index.value == 0) { - _expressions.add(node); - } + final index = node.index; + + if (index case IntegerLiteral(value: 0)) { + _rule.reportAtNode(node); } } } diff --git a/lint_test/prefer_first_test.dart b/lint_test/prefer_first_test.dart deleted file mode 100644 index 2527d828..00000000 --- a/lint_test/prefer_first_test.dart +++ /dev/null @@ -1,21 +0,0 @@ -/// Check the `prefer_first` rule -void fun() { - const zero = 0; - final list = [0, 1, 2, 3]; - final set = {0, 1, 2, 3}; - final map = {0: 0, 1: 1, 2: 2, 3: 3}; - - // expect_lint: prefer_first - list[0]; - list[zero]; - // expect_lint: prefer_first - list.elementAt(0); - list.elementAt(zero); - // expect_lint: prefer_first - set.elementAt(0); - - // expect_lint: prefer_first - map.keys.elementAt(0); - // expect_lint: prefer_first - map.values.elementAt(0); -} diff --git a/test/lints/prefer_first/prefer_first_rule_test.dart b/test/lints/prefer_first/prefer_first_rule_test.dart new file mode 100644 index 00000000..aa1af3fb --- /dev/null +++ b/test/lints/prefer_first/prefer_first_rule_test.dart @@ -0,0 +1,113 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(PreferFirstRuleTest); + }); +} + +@reflectiveTest +class PreferFirstRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = PreferFirstRule(); + super.setUp(); + } + + void test_reports_on_list_index_access_with_zero_literal() async { + await assertDiagnostics( + r''' +final list1 = [0, 1, 2, 3]; +var a = list1[0]; + +void main () { + final list2 = [1, 0, 2, 3]; + list1[0]; + list2[0]; +} +''', + [lint(36, 8), lint(94, 8), lint(106, 8)], + ); + } + + void + test_does_not_report_on_list_index_access_with_variable_or_constant() async { + await assertNoDiagnostics(r''' +const zero = 0; +final zeroVar = 0; + +final list = [0, 1, 2, 3]; + +var a = list[zero]; +var b = list[1 - 1]; +var c = list[zeroVar]; +'''); + } + + void test_reports_on_list_subclasses() async { + await assertDiagnostics( + r''' +abstract class MyList implements List {} + +T getFirst(MyList list) { + return list[0]; +} +''', + [lint(89, 7)], + ); + } + + void test_reports_on_element_at_access_with_zero_literal() async { + await assertDiagnostics( + r''' +final list1 = [0, 1, 2, 3]; +var a = list1.elementAt(0); + +void main () { + final list2 = [1, 0, 2, 3]; + list1.elementAt(0); + list2.elementAt(0); +} +''', + [lint(36, 18), lint(104, 18), lint(126, 18)], + ); + } + + void + test_does_not_report_on_element_at_access_with_variable_or_constant() async { + await assertNoDiagnostics(r''' +const zero = 0; +final zeroVar = 0; + +final list = [0, 1, 2, 3]; + +var a = list.elementAt(zero); +var b = list.elementAt(1 - 1); +var c = list.elementAt(zeroVar); +'''); + } + + void test_reports_on_iterable_subclasses() async { + await assertDiagnostics( + r''' +abstract class MyIterable implements Iterable {} + +T getFirst(MyIterable iterable) { + return iterable.elementAt(0); +} + +void main () { + final set = {0, 1, 2, 3}; + final map = {0: 0, 1: 1, 2: 2, 3: 3}; + + set.elementAt(0); + map.keys.elementAt(0); + map.values.elementAt(0); +} +''', + [lint(105, 21), lint(217, 16), lint(237, 21), lint(262, 23)], + ); + } +}