Linter for print statements

[danschultz]

Published code should use logging rather than print() statements, so consumers can decide how debugging should be handled.

[danschultz]

I'm considering taking a stab at this, but I have a couple questions.

1. What is the best way to check if a method is from a particular library? After stepping through the debugger I finally found that I can access the library like so:

   class Visitor extends SimpleAstVisitor {
     LintRule rule;
     Visitor(this.rule);
   
     @override
     visitMethodInvocation(MethodInvocation node) {
       if (node.methodName.staticElement.enclosingElement.enclosingElement.name == "dart.core") {
         // report lint
       }
     }
   }

   Is there a better or easier way?

2. There are two types of built in groups defined: pub and style. This lint doesn't really belong to either of these. I was thinking debugging, but frankly I'm having a hard time coming up with a group name for this one.

[seaneagan]

This would be useful, but I wouldn't want it on by default (if there is such a thing).

For example, I think it's fine to print in tests and command-line scripts, and if you're using logging, there will still probably be a PrintLogger or something somewhere which does call print.

[zoechi]

I don't care if it is on by default or not, but I would like to have this very much. Currently it's still common that debugging doesn't work and then I start adding print statements and it would be cool if the lint check would complain about them.

[pq]

What is the best way to check if a method is from a particular library?

Paging @bwilkerson ! :)

As for a grouping, we definitely need more categories. Totally open for ideas as to where this one should land! Actually @bwilkerson, I'm curious, did you do anything similar in CodePro?

[bwilkerson]

What is the best way to check if a method is from a particular library?

Assuming you have a resolved AST, then ask the MethodInvocation node for the staticElement it resolved to. That will get you a MethodElement, and you can ask it for the LibraryElement that it's part of. In this case it's easy because you can then ask the library element if it is in the SDK.

Yes, we had a similar rule in CodePro that looked for uses of System.out. We had a "Debugging" category that held rules that looked for left-over debugging code. There weren't very many rules in that category. Rules to catch uses of the new break-forcing methods would also be good candidates for this category.

[pq]

@bwilkerson :

We had a "Debugging" category

Thanks! I started to add just this but given that there are likely to be so few entries in this category, I wonder if we could lump this under something more general. What about under a possible errors catch-all?

[pq]

A new errors catch-all group proposed here: dart-archive/linter#90

[bwilkerson]

In my experience, it becomes much too easy for people to just not think about the category very much and use a catch-all group for everything. But I think it's really useful to users to have meaningful categories to help them find what they're looking for. I'd strongly recommend not creating a catch-all group, but the decision is yours.

[isoos]

Bumping this issue. It would be nice to have print statements listed as hints (if the linter rule is active).

[maxlapides]

Hey I'm sorry to do the annoying thing where I'm basically just commenting to +1 an issue, but here we are 😳

This would be a really useful lint rule for my team. We frequently review PRs where team members accidentally left a print statement in their code. We want to be careful to avoid this, lest our console get clogged up with meaningless information.

Thanks!

[pq]

No worries @maxlapides . Any feedback is greatly appreciated and helps a lot!

[maxlapides]

Thanks so much @pq! Looking forward to enabling this rule soon :)

[pq]

Good deal. Thanks for the nudge!