diff --git a/CHANGELOG.md b/CHANGELOG.md index 52bd7af499..4f9ab6b996 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -* feat: add static code diagnostics `avoid-unnecessary-type-assertions`, `avoid-unnecessary-type-casts` +* feat: add static code diagnostics `avoid-throw-in-catch-block`, `avoid-unnecessary-type-assertions`, `avoid-unnecessary-type-casts` * feat: introduce file metrics * chore: activate self implemented rules: `avoid-unnecessary-type-assertions`, `avoid-unnecessary-type-casts`, `prefer-first`, `prefer-last`, `prefer-match-file-name` * refactor: cleanup anti-patterns, metrics and rules documentation diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart index cd721290d2..d76af85ed8 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart @@ -7,6 +7,7 @@ import 'rules_list/avoid_nested_conditional_expressions/avoid_nested_conditional import 'rules_list/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart'; import 'rules_list/avoid_preserve_whitespace_false/avoid_preserve_whitespace_false_rule.dart'; import 'rules_list/avoid_returning_widgets/avoid_returning_widgets_rule.dart'; +import 'rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule.dart'; import 'rules_list/avoid_unnecessary_setstate/avoid_unnecessary_setstate_rule.dart'; import 'rules_list/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart'; import 'rules_list/avoid_unused_parameters/avoid_unused_parameters_rule.dart'; @@ -52,6 +53,8 @@ final _implementedRules = )>{ AvoidPreserveWhitespaceFalseRule(config), AvoidReturningWidgetsRule.ruleId: (config) => AvoidReturningWidgetsRule(config), + AvoidThrowInCatchBlockRule.ruleId: (config) => + AvoidThrowInCatchBlockRule(config), AvoidUnnecessarySetStateRule.ruleId: (config) => AvoidUnnecessarySetStateRule(config), AvoidUnnecessaryTypeAssertionsRule.ruleId: (config) => diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule.dart new file mode 100644 index 0000000000..37368622a0 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule.dart @@ -0,0 +1,44 @@ +// ignore_for_file: public_member_api_docs + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; + +import '../../../../../utils/node_utils.dart'; +import '../../../lint_utils.dart'; +import '../../../models/internal_resolved_unit_result.dart'; +import '../../../models/issue.dart'; +import '../../../models/severity.dart'; +import '../../models/common_rule.dart'; +import '../../rule_utils.dart'; + +part 'visitor.dart'; + +class AvoidThrowInCatchBlockRule extends CommonRule { + static const String ruleId = 'avoid-throw-in-catch-block'; + + static const _warningMessage = + 'Call throw in a catch block loses the original stack trace and the original exception.'; + + AvoidThrowInCatchBlockRule([Map config = const {}]) + : super( + id: ruleId, + severity: readSeverity(config, Severity.warning), + excludes: readExcludes(config), + ); + + @override + Iterable check(InternalResolvedUnitResult source) { + final _visitor = _Visitor(); + + source.unit.visitChildren(_visitor); + + return _visitor.throwExpression.map((expression) => createIssue( + rule: this, + location: nodeLocation( + node: expression, + source: source, + ), + message: _warningMessage, + )); + } +} diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/visitor.dart new file mode 100644 index 0000000000..d583e1a46e --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/visitor.dart @@ -0,0 +1,30 @@ +part of 'avoid_throw_in_catch_block_rule.dart'; + +class _Visitor extends RecursiveAstVisitor { + final _throwExpression = []; + + Iterable get throwExpression => _throwExpression; + + @override + void visitCatchClause(CatchClause node) { + super.visitCatchClause(node); + + final visitor = _CatchClauseVisitor(); + node.visitChildren(visitor); + + _throwExpression.addAll(visitor.throwExpression); + } +} + +class _CatchClauseVisitor extends RecursiveAstVisitor { + final _throwExpression = []; + + Iterable get throwExpression => _throwExpression; + + @override + void visitThrowExpression(ThrowExpression node) { + super.visitThrowExpression(node); + + _throwExpression.add(node); + } +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule_test.dart new file mode 100644 index 0000000000..6a9164ffe9 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule_test.dart @@ -0,0 +1,44 @@ +@TestOn('vm') +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart'; +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule.dart'; +import 'package:test/test.dart'; + +import '../../../../../helpers/rule_test_helper.dart'; + +const _examplePath = 'avoid_throw_in_catch_block/examples/example.dart'; + +void main() { + group('AvoidThrowInCatchBlockRule', () { + test('initialization', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = AvoidThrowInCatchBlockRule().check(unit); + + RuleTestHelper.verifyInitialization( + issues: issues, + ruleId: 'avoid-throw-in-catch-block', + severity: Severity.warning, + ); + }); + + test('reports about found issues', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = AvoidThrowInCatchBlockRule().check(unit); + + RuleTestHelper.verifyIssues( + issues: issues, + startOffsets: [343, 465], + startLines: [17, 25], + startColumns: [5, 5], + endOffsets: [370, 494], + locationTexts: [ + 'throw RepositoryException()', + 'throw DataProviderException()', + ], + messages: [ + 'Call throw in a catch block loses the original stack trace and the original exception.', + 'Call throw in a catch block loses the original stack trace and the original exception.', + ], + ); + }); + }); +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/examples/example.dart new file mode 100644 index 0000000000..cf286893b7 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_throw_in_catch_block/examples/example.dart @@ -0,0 +1,35 @@ +void main() { + try { + repository(); + } on Object catch (error, stackTrace) { + logError(error, stackTrace); + } +} + +/// Where did the original error occur based on the log? +void logError(Object error, StackTrace stackTrace) => + print('$error\n$stackTrace'); + +void repository() { + try { + networkDataProvider(); + } on Object { + throw RepositoryException(); // LINT + } +} + +void networkDataProvider() { + try { + networkClient(); + } on Object { + throw DataProviderException(); // LINT + } +} + +void networkClient() { + throw 'Some serious problem'; +} + +class RepositoryException {} + +class DataProviderException {} diff --git a/website/docs/rules/common/avoid-throw-in-catch-block.md b/website/docs/rules/common/avoid-throw-in-catch-block.md new file mode 100644 index 0000000000..b7940d751b --- /dev/null +++ b/website/docs/rules/common/avoid-throw-in-catch-block.md @@ -0,0 +1,23 @@ +# Avoid throw in catch block + +## Rule id + +avoid-throw-in-catch-block + +## Description + +Call throw in a catch block loses the original stack trace and the original exception. + +Sine 2.16 version you can use [Error.throwWithStackTrace](https://api.dart.dev/dev/2.16.0-9.0.dev/dart-core/Error/throwWithStackTrace.html). + +### Example + +```dart +void repository() { + try { + networkDataProvider(); + } on Object { + throw RepositoryException(); // LINT + } +} +``` diff --git a/website/docs/rules/common/avoid-unnecessary-type-assertions.md b/website/docs/rules/common/avoid-unnecessary-type-assertions.md index ae9d07f7f8..7d747922a7 100644 --- a/website/docs/rules/common/avoid-unnecessary-type-assertions.md +++ b/website/docs/rules/common/avoid-unnecessary-type-assertions.md @@ -5,6 +5,7 @@ avoid-unnecessary-type-assertions ## Description + Warns about unnecessary usage of 'is' and 'whereType' operators. ### Example diff --git a/website/docs/rules/common/avoid-unnecessary-type-casts.md b/website/docs/rules/common/avoid-unnecessary-type-casts.md index edd8e05832..044f6079de 100644 --- a/website/docs/rules/common/avoid-unnecessary-type-casts.md +++ b/website/docs/rules/common/avoid-unnecessary-type-casts.md @@ -5,6 +5,7 @@ avoid-unnecessary-type-casts ## Description + Warns about of unnecessary use of casting operators. ### Example diff --git a/website/docs/rules/overview.md b/website/docs/rules/overview.md index f68aad7fdb..ae08cd8b6c 100644 --- a/website/docs/rules/overview.md +++ b/website/docs/rules/overview.md @@ -27,6 +27,10 @@ Rules configuration is [described here](../getting-started/configuration#configu Warns when non null assertion operator (or “bang” operator) is used for a property access or method invocation. The operator check works at runtime and it may fail and throw a runtime exception. +- [avoid-throw-in-catch-block](./common/avoid-throw-in-catch-block.md) + + Warns when call `throw` in a catch block. + - [avoid-unnecessary-type-casts](./common/avoid-unnecessary-type-casts.md) Warns about unnecessary usage of 'as' operators.