Skip to content

Commit 42397ea

Browse files
refactored code according to suggestions from review
1 parent 5229dda commit 42397ea

2 files changed

Lines changed: 54 additions & 28 deletions

File tree

lib/src/lints/avoid_debug_print_in_release/visitors/avoid_debug_print_in_release_visitor.dart

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,30 @@ class AvoidDebugPrintInReleaseVisitor extends SimpleAstVisitor<void> {
5050

5151
bool _isWrappedInReleaseCheck(AstNode node) {
5252
AstNode? parent = node.parent;
53+
5354
while (parent != null) {
5455
if (parent is IfStatement) {
55-
final expression = parent.expression;
56-
final source = expression.toString();
56+
final condition = parent.expression;
57+
58+
// if (!kReleaseMode)
59+
if (condition is PrefixExpression &&
60+
condition.operator.lexeme == '!' &&
61+
condition.operand is SimpleIdentifier) {
62+
final operand = condition.operand as SimpleIdentifier;
63+
if (operand.name == _kReleaseMode) {
64+
return true;
65+
}
66+
}
5767

58-
if (source.contains(_kReleaseMode) || source.contains(_kDebugMode)) {
68+
// if (kDebugMode)
69+
if (condition is SimpleIdentifier && condition.name == _kDebugMode) {
5970
return true;
6071
}
6172
}
73+
6274
parent = parent.parent;
6375
}
76+
6477
return false;
6578
}
6679
}

test/avoid_debug_print_in_release_rule_test.dart

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,21 @@ void main() {
1212
class AvoidDebugPrintInReleaseRuleTest extends AnalysisRuleTest {
1313
@override
1414
void setUp() {
15-
// Setting up a mock Flutter package structure
1615
final flutter = newPackage('flutter');
1716

18-
// Core foundation providing the constants and the function
1917
flutter.addFile('lib/foundation.dart', r'''
20-
const bool kReleaseMode = false;
21-
const bool kDebugMode = true;
22-
void debugPrint(String? message) {}
23-
''');
18+
const bool kReleaseMode = false;
19+
const bool kDebugMode = true;
20+
void debugPrint(String? message) {}
21+
''');
2422

25-
// UI libraries that export foundation
2623
flutter.addFile('lib/material.dart', r'''
27-
export 'package:flutter/foundation.dart';
28-
''');
24+
export 'package:flutter/foundation.dart';
25+
''');
2926

3027
flutter.addFile('lib/cupertino.dart', r'''
31-
export 'package:flutter/foundation.dart';
32-
''');
28+
export 'package:flutter/foundation.dart';
29+
''');
3330

3431
rule = AvoidDebugPrintInReleaseRule();
3532

@@ -54,49 +51,67 @@ void test() {
5451

5552
void test_reports_aliased_debug_print_from_package() async {
5653
await assertDiagnostics(
57-
r'''import 'package:flutter/foundation.dart' as f;
54+
r'''
55+
import 'package:flutter/foundation.dart' as f;
56+
5857
void test() {
5958
f.debugPrint('This should be flagged');
60-
}''',
61-
[lint(65, 10)],
59+
}
60+
''',
61+
[lint(66, 10)],
6262
);
6363
}
6464

6565
void test_reports_debug_print_as_callback() async {
6666
await assertDiagnostics(
6767
r'''
6868
import 'package:flutter/foundation.dart';
69+
6970
void test() {
7071
['a'].forEach(debugPrint);
7172
}
7273
''',
73-
[lint(72, 10)],
74+
[lint(73, 10)],
7475
);
7576
}
7677

77-
/// Case: if (kReleaseMode) { ... } is considered safe/safe-guarded logic.
78-
void test_does_not_report_kReleaseMode_guard() async {
79-
await assertNoDiagnostics(
78+
void test_reports_inside_kReleaseMode_guard() async {
79+
await assertDiagnostics(
8080
r'''
8181
import 'package:flutter/foundation.dart';
8282
8383
void test() {
8484
if (kReleaseMode) {
85-
debugPrint('This is safe because it only runs in release');
85+
debugPrint('This should be flagged');
8686
}
8787
}
8888
''',
89+
[lint(83, 10)],
8990
);
9091
}
9192

92-
/// Case: if (!kDebugMode) { ... } is considered safe.
93-
void test_does_not_report_not_kDebugMode_guard() async {
94-
await assertNoDiagnostics(
93+
void test_reports_inside_not_kDebugMode_guard() async {
94+
await assertDiagnostics(
9595
r'''
9696
import 'package:flutter/foundation.dart';
9797
9898
void test() {
9999
if (!kDebugMode) {
100+
debugPrint('Should still be flagged');
101+
}
102+
}
103+
''',
104+
[lint(82, 10)],
105+
);
106+
}
107+
108+
void test_does_not_report_inside_not_kReleaseMode() async {
109+
await assertNoDiagnostics(
110+
r'''
111+
import 'package:flutter/foundation.dart';
112+
113+
void test() {
114+
if (!kReleaseMode) {
100115
debugPrint('Safe');
101116
}
102117
}
@@ -108,6 +123,7 @@ void test() {
108123
await assertNoDiagnostics(
109124
r'''
110125
import 'package:flutter/foundation.dart';
126+
111127
void test() {
112128
if (kDebugMode) {
113129
debugPrint('Safe');
@@ -117,7 +133,6 @@ void test() {
117133
);
118134
}
119135

120-
/// Case: debugPrint is defined locally, not from Flutter.
121136
void test_no_report_when_debugPrint_is_not_from_foundation() async {
122137
await assertNoDiagnostics(
123138
r'''
@@ -130,7 +145,6 @@ void test() {
130145
);
131146
}
132147

133-
/// Case: debugPrint is imported via material.dart.
134148
void test_reports_when_imported_via_material() async {
135149
await assertDiagnostics(
136150
r'''
@@ -144,7 +158,6 @@ void test() {
144158
);
145159
}
146160

147-
/// Case: debugPrint is imported via cupertino.dart.
148161
void test_reports_when_imported_via_cupertino() async {
149162
await assertDiagnostics(
150163
r'''

0 commit comments

Comments
 (0)