DebugPrintRule #1314
DebugPrintRule #1314
Conversation
### What's done: * initial changes
### What's done: * supported detection of print\println
### What's done: * fixed diktat findings
Codecov Report
@@ Coverage Diff @@
## master #1314 +/- ##
============================================
+ Coverage 82.01% 82.03% +0.02%
- Complexity 2538 2561 +23
============================================
Files 105 106 +1
Lines 7239 7275 +36
Branches 2071 2085 +14
============================================
+ Hits 5937 5968 +31
Misses 354 354
- Partials 948 953 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
…ool/diktat into feature/debug_print#954
info/available-rules.md
Outdated
| | Chap | Standard | Rule name | Description | Fix | Config | FixMe | |
There was a problem hiding this comment.
btw - why do we see changes here? Line ending again?
There was a problem hiding this comment.
looks like, also IDEA adds additional - to header.
Will look at it
| @@ -136,6 +136,7 @@ enum class Warnings( | |||
| FILE_NAME_MATCH_CLASS(true, "3.1.2", "file name is incorrect - it should match with the class described in it if there is the only one class declared"), | |||
| COLLAPSE_IF_STATEMENTS(true, "3.16.1", "avoid using redundant nested if-statements, which could be collapsed into a single one"), | |||
| CONVENTIONAL_RANGE(true, "3.17.1", "use conventional rule for range case"), | |||
| DEBUG_PRINT(false, "3.18.1", "avoid using print()/println() or console.log() for debug logging"), | |||
There was a problem hiding this comment.
Could you please also add a description to a code style (chapter 3).
Something like:
Try to avoid the usage of print methods directly, use loggers instead, blabla
| ) { | ||
| override fun logic(node: ASTNode) { | ||
| // check kotlin.io.print()/kotlin.io.println() | ||
| if (node.elementType == ElementType.CALL_EXPRESSION) { |
There was a problem hiding this comment.
May be we will have when by elementType?
And move logic in these branches of when to separate methods - method for console and method for print?
Sorry for this note, depends on you
| @@ -136,6 +136,7 @@ enum class Warnings( | |||
| FILE_NAME_MATCH_CLASS(true, "3.1.2", "file name is incorrect - it should match with the class described in it if there is the only one class declared"), | |||
| COLLAPSE_IF_STATEMENTS(true, "3.16.1", "avoid using redundant nested if-statements, which could be collapsed into a single one"), | |||
| CONVENTIONAL_RANGE(true, "3.17.1", "use conventional rule for range case"), | |||
| DEBUG_PRINT(false, "3.18.1", "avoid using print()/println() or console.log() for debug logging"), | |||
There was a problem hiding this comment.
I think we could reword the message: don't mention particular method names (they will be available when the whole warning text is generated) and instead suggest to use a dedicated logging library
|
Lgtm |
Which rule and warnings did you add?
Detect println and print in code -- assumption that it's a debug logging
This pull request closes #954
Actions checklist
Fixme