dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
628 stars 170 forks source link

avoid_print should be aware of debug mode constant #2616

Open mateusfccp opened 3 years ago

mateusfccp commented 3 years ago

Describe the issue avoid_print description is:

Avoid print calls in production code.

However, if I have the following:

if (kDebugMode) {
    print("foo");
}

The linter won't consider that this print will not be called in production mode.

I think avoid_print should consider debug mode constant(s).

I think we can let kProfileMode out of this, as profile should be as similar to production as possible.

bwilkerson commented 3 years ago

We could definitely special case kDebugMode, but that wouldn't allow invocations of print inside debug code in non-Flutter applications because we wouldn't know that the variable that the invocation is conditional on was only true when running in debug mode.

We could consider adding a @debugMode annotation that could be applied to variables like kDebugMode to make this more general. I don't know whether the value warrants the extra work, but I'm interested in hearing opinions.

mateusfccp commented 3 years ago

Alternatively, Dart could have a non-Flutter constant for debug mode? Then we would have a generalized constant instead of Flutter's one.

quetool commented 2 years ago

What is debugPrint function (from foundation) for? Could be the solution to this?

mateusfccp commented 2 years ago

According to the Dart 2.15 changelog, now avoid_print allows kDebugMode-wrapped print calls.

Can we consider this issue solved?

pq commented 2 years ago

Can we consider this issue solved?

Yes, thanks! Please do follow-up if you see any issues.

mvarendorff commented 1 year ago

I am not sure if this should be an additional issue but it fits so I am following up as requested!

The debug mode constant is not requested in more complex conditions:

if (kDebugMode && 1 == 2) print('Hello :)');

Will trigger a warning eventhough print can only ever be executed when in debug mode.

As a workaround, this can be split to two ifs:

if (kDebugMode) if (1 == 2) print('Hello :)');

But that can clutter the code a little, especially with potentially added indentation that might be required.

pq commented 1 year ago

/fyi @bwilkerson

bwilkerson commented 1 year ago

I think it would be reasonable for the rule to look at the subexpressions in an && expression. There's a limit to how much data flow we want to implement, but this seems inside the limit.

If we need more than this then I'd strongly suggest the annotation approach. We could use the annotation on a field or variable to mean that it's true when we're in debug mode, and on a method or function to mean that it should only be invoked in debug mode. That would allow utility methods used to print larger amounts of data to not require guarding every individual print while ensuring that they won't be invoked in production code.