diff --git a/lib/main.dart b/lib/main.dart index 4b1be247..78ee2a4c 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -1,5 +1,6 @@ import 'package:analysis_server_plugin/plugin.dart'; import 'package:analysis_server_plugin/registry.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_global_state/avoid_global_state_rule.dart'; import 'package:solid_lints/src/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart'; 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 index e9b52edb..a5d3d44b 100644 --- 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 @@ -1,10 +1,8 @@ -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'; +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/error/error.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print_in_release/visitors/avoid_debug_print_in_release_visitor.dart'; /// An `avoid_debug_print_in_release` rule which forbids calling or referencing /// debugPrint function from flutter/foundation in release mode. @@ -31,176 +29,32 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ``` /// /// -class AvoidDebugPrintInReleaseRule extends SolidLintRule { - /// This lint rule 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'; +class AvoidDebugPrintInReleaseRule extends AnalysisRule { + /// The name of the lint + static const String lintName = 'avoid_debug_print_in_release'; - AvoidDebugPrintInReleaseRule._(super.config); + /// Lint code used for suppression and reporting. + static const LintCode _code = LintCode( + lintName, + 'Avoid debugPrint in release mode.', + correctionMessage: 'Wrap in a kReleaseMode check or use a logging package.', + ); - /// 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.""", - ); + /// Creates an instance of [AvoidDebugPrintInReleaseRule]. + AvoidDebugPrintInReleaseRule() + : super(name: lintName, description: 'Avoid debugPrint in release mode.'); - return AvoidDebugPrintInReleaseRule._(rule); - } + @override + LintCode get diagnosticCode => _code; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext 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 DiagnosticReporter 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.atNode(node, code); - } - - /// 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 DiagnosticReporter 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.element?.library?.uri.toString() ?? ''; - case SimpleIdentifier(): - name = node.name; - sourcePath = node.element?.library?.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.element?.library?.uri.toString() ?? ''; - case SimpleIdentifier(): - name = node.name; - sourcePath = node.element?.library?.uri.toString() ?? ''; - default: - return false; - } - - return name == _kReleaseModeName && sourcePath == _kReleaseModePath; + final visitor = AvoidDebugPrintInReleaseVisitor(this); + registry.addMethodInvocation(this, visitor); + registry.addSimpleIdentifier(this, visitor); } } diff --git a/lib/src/lints/avoid_debug_print_in_release/visitors/avoid_debug_print_in_release_visitor.dart b/lib/src/lints/avoid_debug_print_in_release/visitors/avoid_debug_print_in_release_visitor.dart new file mode 100644 index 00000000..8b794a1b --- /dev/null +++ b/lib/src/lints/avoid_debug_print_in_release/visitors/avoid_debug_print_in_release_visitor.dart @@ -0,0 +1,86 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart'; + +/// Visitor for [AvoidDebugPrintInReleaseRule]. +class AvoidDebugPrintInReleaseVisitor extends SimpleAstVisitor { + /// The rule associated with this visitor. + final AvoidDebugPrintInReleaseRule rule; + static const _foundationUri = 'package:flutter/foundation.dart'; + static const _debugPrint = 'debugPrint'; + static const _kReleaseMode = 'kReleaseMode'; + static const _kDebugMode = 'kDebugMode'; + static const _callMethod = 'call'; + + /// Creates an instance of [AvoidDebugPrintInReleaseVisitor]. + AvoidDebugPrintInReleaseVisitor(this.rule); + + @override + void visitMethodInvocation(MethodInvocation node) { + final target = node.target; + final methodName = node.methodName; + + if (methodName.name == _callMethod && target is SimpleIdentifier) { + _check(node, target); + return; + } + + _check(node, methodName); + } + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + if (node.parent is! MethodInvocation) { + _check(node, node); + } + } + + void _check(AstNode node, SimpleIdentifier identifier) { + final element = identifier.element; + if (element == null) return; + + if (element.name == _debugPrint) { + final sourceUri = element.library?.uri.toString() ?? ''; + + final isFlutterFoundation = sourceUri.contains( + _foundationUri, + ); + + if (isFlutterFoundation) { + if (!_isWrappedInReleaseCheck(node)) { + rule.reportAtNode(identifier); + } + } + } + } + + bool _isWrappedInReleaseCheck(AstNode node) { + AstNode? parent = node.parent; + + while (parent != null) { + if (parent is IfStatement) { + final condition = parent.expression; + + if (_isNotReleaseModeCheck(condition) || _isDebugModeCheck(condition)) { + return true; + } + } + + parent = parent.parent; + } + + return false; + } + + bool _isNotReleaseModeCheck(Expression condition) { + return condition is PrefixExpression && + condition.operator.type == TokenType.BANG && + condition.operand is SimpleIdentifier && + (condition.operand as SimpleIdentifier).name == _kReleaseMode; + } + + bool _isDebugModeCheck(Expression condition) { + return condition is SimpleIdentifier && condition.name == _kDebugMode; + } +} 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 deleted file mode 100644 index 9d6fa72a..00000000 --- a/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart +++ /dev/null @@ -1,58 +0,0 @@ -// 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/test/avoid_debug_print_in_release_rule_test.dart b/test/avoid_debug_print_in_release_rule_test.dart new file mode 100644 index 00000000..74c6554b --- /dev/null +++ b/test/avoid_debug_print_in_release_rule_test.dart @@ -0,0 +1,186 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidDebugPrintInReleaseRuleTest); + }); +} + +@reflectiveTest +class AvoidDebugPrintInReleaseRuleTest extends AnalysisRuleTest { + @override + void setUp() { + final flutter = newPackage('flutter'); + + flutter.addFile('lib/foundation.dart', r''' +const bool kReleaseMode = false; +const bool kDebugMode = true; +void debugPrint(String? message) {} +'''); + + flutter.addFile('lib/material.dart', r''' +export 'package:flutter/foundation.dart'; +'''); + + flutter.addFile('lib/cupertino.dart', r''' +export 'package:flutter/foundation.dart'; +'''); + + rule = AvoidDebugPrintInReleaseRule(); + + super.setUp(); + } + + @override + String get analysisRule => AvoidDebugPrintInReleaseRule.lintName; + + void test_reports_debug_print_with_package_import() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + debugPrint('This should be flagged'); +} +''', + [lint(59, 10)], + ); + } + + void test_reports_aliased_debug_print_from_package() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart' as f; + +void test() { + f.debugPrint('This should be flagged'); +} +''', + [lint(66, 10)], + ); + } + + void test_reports_debug_print_as_callback() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + ['a'].forEach(debugPrint); +} +''', + [lint(73, 10)], + ); + } + + void test_reports_inside_kReleaseMode_guard() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + if (kReleaseMode) { + debugPrint('This should be flagged'); + } +} +''', + [lint(83, 10)], + ); + } + + void test_reports_inside_not_kDebugMode_guard() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + if (!kDebugMode) { + debugPrint('Should still be flagged'); + } +} +''', + [lint(82, 10)], + ); + } + + void test_does_not_report_inside_not_kReleaseMode() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + if (!kReleaseMode) { + debugPrint('Safe'); + } +} +''', + ); + } + + void test_does_not_report_inside_kDebugMode() async { + await assertNoDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + if (kDebugMode) { + debugPrint('Safe'); + } +} +''', + ); + } + + void test_no_report_when_debugPrint_is_not_from_foundation() async { + await assertNoDiagnostics( + r''' +void debugPrint(String message) {} + +void test() { + debugPrint('Not a flutter call'); +} +''', + ); + } + + void test_reports_when_imported_via_material() async { + await assertDiagnostics( + r''' +import 'package:flutter/material.dart'; + +void test() { + debugPrint('Flagged via material'); +} +''', + [lint(57, 10)], + ); + } + + void test_reports_when_imported_via_cupertino() async { + await assertDiagnostics( + r''' +import 'package:flutter/cupertino.dart'; + +void test() { + debugPrint('Flagged via cupertino'); +} +''', + [lint(58, 10)], + ); + } + + void test_reports_debug_print_call_method() async { + await assertDiagnostics( + r''' +import 'package:flutter/foundation.dart'; + +void test() { + debugPrint.call('This should be flagged'); +} +''', + [lint(59, 10)], + ); + } +}