From e5fb3e921c364a6ca899f744ef45655ac027be31 Mon Sep 17 00:00:00 2001 From: sufftea <72755555+sufftea@users.noreply.github.com> Date: Thu, 2 May 2024 11:28:30 +0300 Subject: [PATCH] I166 (#170) * Rename to avoid_debug_print_in_release * Implement kDebugMode check * add doc comments * Look for the checks all the way up the tree; check for !kReleaseMode instead * Refactor & fix issues * fix comments * fix more comments --- CHANGELOG.md | 3 +- example/analysis_options.yaml | 2 +- lib/analysis_options.yaml | 2 +- lib/solid_lints.dart | 4 +- .../avoid_debug_print_rule.dart | 131 ----------- .../models/avoid_debug_print_func_model.dart | 61 ------ .../avoid_debug_print_in_release_rule.dart | 207 ++++++++++++++++++ lint_test/analysis_options.yaml | 2 +- ...id_debug_print_in_release_prefix_test.dart | 53 +++++ .../avoid_debug_print_in_release_test.dart | 58 +++++ .../avoid_debug_print_prefix_test.dart | 25 --- .../avoid_debug_print_test.dart | 28 --- 12 files changed, 325 insertions(+), 251 deletions(-) delete mode 100644 lib/src/lints/avoid_debug_print/avoid_debug_print_rule.dart delete mode 100644 lib/src/lints/avoid_debug_print/models/avoid_debug_print_func_model.dart create mode 100644 lib/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart create mode 100644 lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_prefix_test.dart create mode 100644 lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart delete mode 100644 lint_test/avoid_debug_print_test.dart/avoid_debug_print_prefix_test.dart delete mode 100644 lint_test/avoid_debug_print_test.dart/avoid_debug_print_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 481e164..3f95c7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ - Add a rule prefer_guard_clause for reversing nested if statements (https://github.com/solid-software/solid_lints/issues/91) - add exclude params support to avoid_returning_widgets rule (https://github.com/solid-software/solid_lints/issues/131) - add quick fix to avoid_final_with_getter (https://github.com/solid-software/solid_lints/pull/164) - +- Renamed `avoid_debug_print` to `avoid_debug_print_in_release` +- The `avoid_debug_print_in_release` no longer reports a warning if the `debugPrint` call is wrapped in a `!kReleaseMode` check. ## 0.1.5 diff --git a/example/analysis_options.yaml b/example/analysis_options.yaml index 1e39170..1ba2855 100644 --- a/example/analysis_options.yaml +++ b/example/analysis_options.yaml @@ -19,7 +19,7 @@ custom_lint: - avoid_unnecessary_setstate - double_literal_format - avoid_unnecessary_type_assertions - - avoid_debug_print + - avoid_debug_print_in_release - avoid_using_api: severity: info entries: diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index 19856ff..9eeaace 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -50,7 +50,7 @@ custom_lint: - avoid_unnecessary_type_casts - avoid_unrelated_type_assertions - avoid_unused_parameters - - avoid_debug_print + - avoid_debug_print_in_release - avoid_final_with_getter - cyclomatic_complexity: diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index 0e7f7b0..0ddfd58 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -1,7 +1,7 @@ library solid_metrics; import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/lints/avoid_debug_print/avoid_debug_print_rule.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart'; import 'package:solid_lints/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart'; import 'package:solid_lints/src/lints/avoid_global_state/avoid_global_state_rule.dart'; import 'package:solid_lints/src/lints/avoid_late_keyword/avoid_late_keyword_rule.dart'; @@ -62,7 +62,7 @@ class _SolidLints extends PluginBase { PreferLastRule.createRule(configs), PreferMatchFileNameRule.createRule(configs), ProperSuperCallsRule.createRule(configs), - AvoidDebugPrint.createRule(configs), + AvoidDebugPrintInReleaseRule.createRule(configs), PreferEarlyReturnRule.createRule(configs), AvoidFinalWithGetterRule.createRule(configs), ]; diff --git a/lib/src/lints/avoid_debug_print/avoid_debug_print_rule.dart b/lib/src/lints/avoid_debug_print/avoid_debug_print_rule.dart deleted file mode 100644 index 9d4b150..0000000 --- a/lib/src/lints/avoid_debug_print/avoid_debug_print_rule.dart +++ /dev/null @@ -1,131 +0,0 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/ast/syntactic_entity.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/lints/avoid_debug_print/models/avoid_debug_print_func_model.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; - -/// A `avoid_debug_print` rule which forbids calling or referencing -/// debugPrint function from flutter/foundation. -/// -/// See more here: https://github.com/flutter/flutter/issues/147141 -/// -/// ### Example -/// -/// #### BAD: -/// -/// ```dart -/// debugPrint(''); // LINT -/// var ref = debugPrint; // LINT -/// var ref2; -/// ref2 = debugPrint; // LINT -/// ``` -/// -/// #### GOOD: -/// -/// ```dart -/// if (kDebugMode) { -/// debugPrint(''); -/// } -/// ``` -/// -/// -class AvoidDebugPrint extends SolidLintRule { - /// The [LintCode] of this lint rule that represents - /// the error when debugPrint is called - static const lintName = 'avoid_debug_print'; - - AvoidDebugPrint._(super.config); - - /// Creates a new instance of [AvoidDebugPrint] - /// based on the lint configuration. - factory AvoidDebugPrint.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => "Avoid using 'debugPrint'", - ); - - return AvoidDebugPrint._(rule); - } - - @override - void run( - CustomLintResolver resolver, - ErrorReporter reporter, - CustomLintContext context, - ) { - context.registry.addFunctionExpressionInvocation( - (node) { - final func = node.function; - if (func is! Identifier) { - return; - } - _checkIdentifier( - identifier: func, - node: node, - reporter: reporter, - ); - }, - ); - - // addFunctionReference does not get triggered. - // addVariableDeclaration and addAssignmentExpression - // are used as a workaround for simple cases - - context.registry.addVariableDeclaration((node) { - _handleVariableAssignmentDeclaration( - node: node, - reporter: reporter, - ); - }); - - context.registry.addAssignmentExpression((node) { - _handleVariableAssignmentDeclaration( - node: node, - reporter: reporter, - ); - }); - } - - /// Checks whether the function identifier satisfies conditions - void _checkIdentifier({ - required Identifier identifier, - required AstNode node, - required ErrorReporter reporter, - }) { - final funcModel = AvoidDebugPrintFuncModel.parseExpression(identifier); - - if (funcModel.hasSameName && funcModel.hasTheSameSource) { - reporter.reportErrorForNode(code, node); - } - } - - /// Returns null if doesn't have right operand - SyntacticEntity? _getRightOperand(List entities) { - /// Example var t = 15; 15 is in 3d position - if (entities.length < 3) { - return null; - } - return entities[2]; - } - - /// Handles variable assignment and declaration - void _handleVariableAssignmentDeclaration({ - required AstNode node, - required ErrorReporter reporter, - }) { - final rightOperand = _getRightOperand(node.childEntities.toList()); - - if (rightOperand == null || rightOperand is! Identifier) { - return; - } - - _checkIdentifier( - identifier: rightOperand, - node: node, - reporter: reporter, - ); - } -} diff --git a/lib/src/lints/avoid_debug_print/models/avoid_debug_print_func_model.dart b/lib/src/lints/avoid_debug_print/models/avoid_debug_print_func_model.dart deleted file mode 100644 index c0fb542..0000000 --- a/lib/src/lints/avoid_debug_print/models/avoid_debug_print_func_model.dart +++ /dev/null @@ -1,61 +0,0 @@ -import 'package:analyzer/dart/ast/ast.dart'; - -/// A class used to parse function expression -class AvoidDebugPrintFuncModel { - /// Function name - final String name; - - /// Function's source path - final String sourcePath; - - /// A class used to parse function expression - const AvoidDebugPrintFuncModel({ - required this.name, - required this.sourcePath, - }); - - /// A constructor that parses identifier into [name] and [sourcePath] - factory AvoidDebugPrintFuncModel.parseExpression( - Identifier identifier, - ) { - switch (identifier) { - case PrefixedIdentifier(): - final prefix = identifier.prefix.name; - return AvoidDebugPrintFuncModel( - name: identifier.name.replaceAll('$prefix.', ''), - sourcePath: - identifier.staticElement?.librarySource?.uri.toString() ?? '', - ); - case SimpleIdentifier(): - return AvoidDebugPrintFuncModel( - name: identifier.name, - sourcePath: - identifier.staticElement?.librarySource?.uri.toString() ?? '', - ); - default: - return AvoidDebugPrintFuncModel._empty(); - } - } - - factory AvoidDebugPrintFuncModel._empty() { - return const AvoidDebugPrintFuncModel( - name: '', - sourcePath: '', - ); - } - - static const String _printPath = 'package:flutter/src/foundation/print.dart'; - - static const String _debugPrint = 'debugPrint'; - - /// Ehether the function has the same source library as debugPrint func - bool get hasTheSameSource => _printPath == sourcePath; - - /// Ehether the function has the same name as debugPrint - bool get hasSameName => _debugPrint == name; - - @override - String toString() { - return '$name, $sourcePath'; - } -} diff --git a/lib/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart b/lib/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart new file mode 100644 index 0000000..1a3e7aa --- /dev/null +++ b/lib/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart @@ -0,0 +1,207 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/syntactic_entity.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/models/solid_lint_rule.dart'; + +/// An `avoid_debug_print_in_release` rule which forbids calling or referencing +/// debugPrint function from flutter/foundation in release mode. +/// +/// See more here: https://github.com/flutter/flutter/issues/147141 +/// +/// ### Example +/// +/// #### BAD: +/// +/// ```dart +/// debugPrint(''); // LINT +/// var ref = debugPrint; // LINT +/// var ref2; +/// ref2 = debugPrint; // LINT +/// ``` +/// +/// #### GOOD: +/// +/// ```dart +/// if (!kReleaseMode) { +/// debugPrint(''); +/// } +/// ``` +/// +/// +class AvoidDebugPrintInReleaseRule extends SolidLintRule { + /// The [LintCode] of this lint rule that represents + /// the error when debugPrint is called + static const lintName = 'avoid_debug_print_in_release'; + + static const String _kReleaseModePath = + 'package:flutter/src/foundation/constants.dart'; + static const String _kReleaseModeName = 'kReleaseMode'; + static const _debugPrintPath = 'package:flutter/src/foundation/print.dart'; + static const _debugPrintName = 'debugPrint'; + + AvoidDebugPrintInReleaseRule._(super.config); + + /// Creates a new instance of [AvoidDebugPrintInReleaseRule] + /// based on the lint configuration. + factory AvoidDebugPrintInReleaseRule.createRule(CustomLintConfigs configs) { + final rule = RuleConfig( + configs: configs, + name: lintName, + problemMessage: (_) => """ +Avoid using 'debugPrint' in release mode. Wrap +your `debugPrint` call in a `!kReleaseMode` check.""", + ); + + return AvoidDebugPrintInReleaseRule._(rule); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addFunctionExpressionInvocation( + (node) { + final func = node.function; + if (func is! Identifier) { + return; + } + + _checkIdentifier( + identifier: func, + node: node, + reporter: reporter, + ); + }, + ); + + // addFunctionReference does not get triggered. + // addVariableDeclaration and addAssignmentExpression + // are used as a workaround for simple cases + + context.registry.addVariableDeclaration((node) { + _handleVariableAssignmentDeclaration( + node: node, + reporter: reporter, + ); + }); + + context.registry.addAssignmentExpression((node) { + _handleVariableAssignmentDeclaration( + node: node, + reporter: reporter, + ); + }); + } + + /// Checks whether the function identifier satisfies conditions + void _checkIdentifier({ + required Identifier identifier, + required AstNode node, + required ErrorReporter reporter, + }) { + if (!_isDebugPrintNode(identifier)) { + return; + } + + final debugCheck = node.thisOrAncestorMatching( + (node) { + if (node is IfStatement) { + return _isNotReleaseCheck(node.expression); + } + + return false; + }, + ); + + if (debugCheck != null) { + return; + } + + reporter.reportErrorForNode(code, node); + } + + /// Returns null if doesn't have right operand + SyntacticEntity? _getRightOperand(List entities) { + /// Example var t = 15; 15 is in 3d position + if (entities.length < 3) { + return null; + } + return entities[2]; + } + + /// Handles variable assignment and declaration + void _handleVariableAssignmentDeclaration({ + required AstNode node, + required ErrorReporter reporter, + }) { + final rightOperand = _getRightOperand(node.childEntities.toList()); + + if (rightOperand == null || rightOperand is! Identifier) { + return; + } + + _checkIdentifier( + identifier: rightOperand, + node: node, + reporter: reporter, + ); + } + + bool _isDebugPrintNode(Identifier node) { + final String name; + final String sourcePath; + switch (node) { + case PrefixedIdentifier(): + final prefix = node.prefix.name; + name = node.name.replaceAll('$prefix.', ''); + sourcePath = node.staticElement?.librarySource?.uri.toString() ?? ''; + + case SimpleIdentifier(): + name = node.name; + sourcePath = node.staticElement?.librarySource?.uri.toString() ?? ''; + + default: + return false; + } + + return name == _debugPrintName && sourcePath == _debugPrintPath; + } + + bool _isNotReleaseCheck(Expression node) { + if (node.childEntities.toList() + case [ + final Token token, + final Identifier identifier, + ]) { + return token.type == TokenType.BANG && + _isReleaseModeIdentifier(identifier); + } + + return false; + } + + bool _isReleaseModeIdentifier(Identifier node) { + final String name; + final String sourcePath; + + switch (node) { + case PrefixedIdentifier(): + final prefix = node.prefix.name; + + name = node.name.replaceAll('$prefix.', ''); + sourcePath = node.staticElement?.librarySource?.uri.toString() ?? ''; + case SimpleIdentifier(): + name = node.name; + sourcePath = node.staticElement?.librarySource?.uri.toString() ?? ''; + default: + return false; + } + + return name == _kReleaseModeName && sourcePath == _kReleaseModePath; + } +} diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index 0df2be6..3d8da19 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -24,7 +24,7 @@ custom_lint: - newline_before_return - no_empty_block - no_equal_then_else - - avoid_debug_print + - avoid_debug_print_in_release - prefer_early_return - member_ordering: alphabetize: true diff --git a/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_prefix_test.dart b/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_prefix_test.dart new file mode 100644 index 0000000..01b162a --- /dev/null +++ b/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_prefix_test.dart @@ -0,0 +1,53 @@ +// ignore_for_file: unused_local_variable + +import 'package:flutter/foundation.dart' as f; + +/// Test the avoid_debug_print_in_release +void avoidDebugPrintTest() { + // expect_lint: avoid_debug_print_in_release + f.debugPrint(''); + + // expect_lint: avoid_debug_print_in_release + final test = f.debugPrint; + + test('test'); + + // expect_lint: avoid_debug_print_in_release + final test2 = f.debugPrint(''); + + debugPrint(); + + debugPrint; + + if (!f.kReleaseMode) { + f.debugPrint(''); + + final test = f.debugPrint; + + var test2; + + test2 = debugPrint; + + test.call('test'); + + final test3 = f.debugPrint(''); + + if (true) { + f.debugPrint(''); + + final test = f.debugPrint; + + var test2; + + test2 = debugPrint; + + test.call('test'); + + final test3 = f.debugPrint(''); + } + } +} + +void debugPrint() { + return; +} diff --git a/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart b/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart new file mode 100644 index 0000000..9d6fa72 --- /dev/null +++ b/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart @@ -0,0 +1,58 @@ +// ignore_for_file: unused_local_variable + +import 'package:flutter/foundation.dart'; + +/// Test the avoid_debug_print_in_release +void avoidDebugPrintTest() { + // expect_lint: avoid_debug_print_in_release + debugPrint(''); + + // expect_lint: avoid_debug_print_in_release + final test = debugPrint; + + var test2; + + // expect_lint: avoid_debug_print_in_release + test2 = debugPrint; + + test.call('test'); + + // expect_lint: avoid_debug_print_in_release + final test3 = debugPrint(''); + + someOtherFunction(); + + if (!kReleaseMode) { + debugPrint(''); + + final test = debugPrint; + + var test2; + + test2 = debugPrint; + + test.call('test'); + + final test3 = debugPrint(''); + + someOtherFunction(); + + if (true) { + debugPrint(''); + + final test = debugPrint; + + var test2; + + test2 = debugPrint; + + test.call('test'); + + final test3 = debugPrint(''); + } + } +} + +void someOtherFunction() { + print('iii'); +} diff --git a/lint_test/avoid_debug_print_test.dart/avoid_debug_print_prefix_test.dart b/lint_test/avoid_debug_print_test.dart/avoid_debug_print_prefix_test.dart deleted file mode 100644 index 1767f8f..0000000 --- a/lint_test/avoid_debug_print_test.dart/avoid_debug_print_prefix_test.dart +++ /dev/null @@ -1,25 +0,0 @@ -// ignore_for_file: unused_local_variable - -import 'package:flutter/foundation.dart' as f; - -/// Test the avoid_debug_print -void avoidDebugPrintTest() { - // expect_lint: avoid_debug_print - f.debugPrint(''); - - // expect_lint: avoid_debug_print - final test = f.debugPrint; - - test('test'); - - // expect_lint: avoid_debug_print - final test2 = f.debugPrint(''); - - debugPrint(); - - debugPrint; -} - -void debugPrint() { - return; -} diff --git a/lint_test/avoid_debug_print_test.dart/avoid_debug_print_test.dart b/lint_test/avoid_debug_print_test.dart/avoid_debug_print_test.dart deleted file mode 100644 index 32cdc59..0000000 --- a/lint_test/avoid_debug_print_test.dart/avoid_debug_print_test.dart +++ /dev/null @@ -1,28 +0,0 @@ -// ignore_for_file: unused_local_variable - -import 'package:flutter/foundation.dart'; - -/// Test the avoid_debug_print -void avoidDebugPrintTest() { - // expect_lint: avoid_debug_print - debugPrint(''); - - // expect_lint: avoid_debug_print - final test = debugPrint; - - var test2; - - // expect_lint: avoid_debug_print - test2 = debugPrint; - - test.call('test'); - - // expect_lint: avoid_debug_print - final test3 = debugPrint(''); - - someOtherFunction(); -} - -void someOtherFunction() { - print('iii'); -}