Pro Yearly is on sale from $80 to $50! »

Improving Code Quality With Static Analysis

Improving Code Quality With Static Analysis

Short presentation given at the NY Android Meetup in October 2019 about KtLint and Detekt.

Bc87ea9c7a0f85b8761b716a677c6694?s=128

Adam McNeilly

October 21, 2019
Tweet

Transcript

  1. Improving Code Quality With Sta7c Analysis Adam McNeilly - @AdamMc331

    @AdamMc331 @NYAndroidMeetup 1
  2. What Do I Mean By Code Quality? @AdamMc331 @NYAndroidMeetup 2

  3. What Do I Mean By Code Quality? • Code should

    be well forma1ed @AdamMc331 @NYAndroidMeetup 2
  4. What Do I Mean By Code Quality? • Code should

    be well forma1ed • Code should be free of code smells @AdamMc331 @NYAndroidMeetup 2
  5. Forma&ng Code With KtLint An an%-bikeshedding Kotlin linter with built-in

    forma8er.1 "Bikeshedding" is a term used to describe was5ng 5me on trivial details. 1 h$ps:/ /ktlint.github.io/ @AdamMc331 @NYAndroidMeetup 3
  6. What Does KtLint Enforce? @AdamMc331 @NYAndroidMeetup 4

  7. What Does KtLint Enforce? • Consistent indenta,on @AdamMc331 @NYAndroidMeetup 4

  8. What Does KtLint Enforce? • Consistent indenta,on • No wildcard

    imports @AdamMc331 @NYAndroidMeetup 4
  9. What Does KtLint Enforce? • Consistent indenta,on • No wildcard

    imports • Consistent spacing @AdamMc331 @NYAndroidMeetup 4
  10. What Does KtLint Enforce? • Consistent indenta,on • No wildcard

    imports • Consistent spacing • No empty class bodies @AdamMc331 @NYAndroidMeetup 4
  11. What Does KtLint Enforce? • Consistent indenta,on • No wildcard

    imports • Consistent spacing • No empty class bodies • More: h:ps:/ /github.com/pinterest/ktlint#standard-rules @AdamMc331 @NYAndroidMeetup 4
  12. Why Should We Care? @AdamMc331 @NYAndroidMeetup 5

  13. Why Should We Care? • Consistency is important @AdamMc331 @NYAndroidMeetup

    5
  14. Why Should We Care? • Consistency is important • Create

    a source of truth that everyone agrees on and no one can argue with @AdamMc331 @NYAndroidMeetup 5
  15. Why Should We Care? • Consistency is important • Create

    a source of truth that everyone agrees on and no one can argue with • Allows us to focus on the substance of code reviews instead of styling @AdamMc331 @NYAndroidMeetup 5
  16. Adding KtLint To Our Projects @AdamMc331 @NYAndroidMeetup 6

  17. Adding KtLint To Our Projects2 // App module build.gradle ktlint

    { version = "0.35.0" android = true enableExperimentalRules = true reporters = [ReporterType.PLAIN] additionalEditorconfigFile = file("../.editorConfig") } 2 h$ps:/ /github.com/JLLeitschuh/ktlint-gradle @AdamMc331 @NYAndroidMeetup 7
  18. Two Helpful Gradle Tasks @AdamMc331 @NYAndroidMeetup 8

  19. Two Helpful Gradle Tasks • ./gradlew ktlintCheck @AdamMc331 @NYAndroidMeetup 8

  20. Two Helpful Gradle Tasks • ./gradlew ktlintCheck • ./gradlew ktlintFormat

    @AdamMc331 @NYAndroidMeetup 8
  21. KtLint Example // Long line, no spacing data class Test(val

    name:String,val age:Int,val location:String,val isRegistered:Boolean,val test:String) // Error output: Test.kt:3:1: Exceeded max line length (100) (cannot be auto-corrected) Test.kt:3:17: Parameter should be on a separate line (unless all parameters can fit a single line) Test.kt:3:26: Missing spacing after ":" // Gets formatted into data class Test( val name: String, val age: Int, val location: String, val isRegistered: Boolean, val test: String ) @AdamMc331 @NYAndroidMeetup 9
  22. KtLint Example // Long line, no spacing data class Test(val

    name:String,val age:Int,val location:String,val isRegistered:Boolean,val test:String) // Error output: Test.kt:3:1: Exceeded max line length (100) (cannot be auto-corrected) Test.kt:3:17: Parameter should be on a separate line (unless all parameters can fit a single line) Test.kt:3:26: Missing spacing after ":" // Gets formatted into data class Test( val name: String, val age: Int, val location: String, val isRegistered: Boolean, val test: String ) @AdamMc331 @NYAndroidMeetup 9
  23. KtLint Example // Long line, no spacing data class Test(val

    name:String,val age:Int,val location:String,val isRegistered:Boolean,val test:String) // Error output: Test.kt:3:1: Exceeded max line length (100) (cannot be auto-corrected) Test.kt:3:17: Parameter should be on a separate line (unless all parameters can fit a single line) Test.kt:3:26: Missing spacing after ":" // Gets formatted into data class Test( val name: String, val age: Int, val location: String, val isRegistered: Boolean, val test: String ) @AdamMc331 @NYAndroidMeetup 9
  24. Remembering To Format Code Is Hard @AdamMc331 @NYAndroidMeetup 10

  25. Auto Format With Git Hook3 // .git/hooks/pre-commit CHANGED_FILES="$(git --no-pager diff

    --name-status --no-color --cached | awk '$1 != "D" && $2 ~ /\.kts|\.kt/ { print $2}')" ./gradlew --quiet ktlintFormat -PinternalKtlintGitFilter="$CHANGED_FILES" echo "$CHANGED_FILES" | while read -r file; do if [ -f $file ]; then git add $file fi done 3 h$ps:/ /github.com/AdamMc331/PokeDex/blob/master/scripts/git-hooks/pre-commit.sh @AdamMc331 @NYAndroidMeetup 11
  26. KtLint Sample @AdamMc331 @NYAndroidMeetup 12

  27. Avoid Code Smells With Detekt4 4 h$ps:/ /arturbosch.github.io/detekt/ @AdamMc331 @NYAndroidMeetup

    13
  28. Avoid Code Smells With Detekt4 • Avoid complex methods 4

    h$ps:/ /arturbosch.github.io/detekt/ @AdamMc331 @NYAndroidMeetup 13
  29. Avoid Code Smells With Detekt4 • Avoid complex methods •

    Enforce method counts in classes 4 h$ps:/ /arturbosch.github.io/detekt/ @AdamMc331 @NYAndroidMeetup 13
  30. Avoid Code Smells With Detekt4 • Avoid complex methods •

    Enforce method counts in classes • Limit lines of code in a class/method 4 h$ps:/ /arturbosch.github.io/detekt/ @AdamMc331 @NYAndroidMeetup 13
  31. Avoid Code Smells With Detekt4 • Avoid complex methods •

    Enforce method counts in classes • Limit lines of code in a class/method • Prevent magic numbers 4 h$ps:/ /arturbosch.github.io/detekt/ @AdamMc331 @NYAndroidMeetup 13
  32. Why Should We Care? @AdamMc331 @NYAndroidMeetup 14

  33. Why Should We Care? • Large collec+ons of code (classes,

    methods, or whatever else) are unmanageable @AdamMc331 @NYAndroidMeetup 14
  34. Why Should We Care? • Large collec+ons of code (classes,

    methods, or whatever else) are unmanageable • Magic numbers, deeply nested methods, and other code smells can be causes of bugs and make them difficult to resolve @AdamMc331 @NYAndroidMeetup 14
  35. Why Should We Care? • Large collec+ons of code (classes,

    methods, or whatever else) are unmanageable • Magic numbers, deeply nested methods, and other code smells can be causes of bugs and make them difficult to resolve • These sorts of code smells are difficult to detekt5 in a code review 5 I'm sorry... @AdamMc331 @NYAndroidMeetup 14
  36. Adding Detekt To Our Projects // App module build.gradle detekt

    { filters = ".*/resources/.*,.*/build/.*" baseline = file("my-detekt-baseline.xml") config = rootProject.files('detekt-config.yml') } @AdamMc331 @NYAndroidMeetup 15
  37. Detekt Is Highly Configurable6 // detekt-config.yml complexity: // Rule Set

    active: true ComplexMethod: // Rule active: true threshold: 10 ignoreSingleWhenExpression: false ignoreSimpleWhenEntries: false LongParameterList: // Rule active: true threshold: 6 ignoreDefaultParameters: false NestedBlockDepth: // Rule active: true threshold: 4 6 h$ps:/ /github.com/arturbosch/detekt/blob/master/detekt-cli/src/main/resources/default-detekt-config.yml @AdamMc331 @NYAndroidMeetup 16
  38. Detekt Is Highly Configurable7 // detekt-config.yml complexity: active: true TooManyFunctions:

    active: true thresholdInFiles: 11 thresholdInClasses: 11 thresholdInInterfaces: 11 thresholdInObjects: 11 thresholdInEnums: 11 ignoreDeprecated: false ignorePrivate: false ignoreOverridden: false 7 h$ps:/ /github.com/arturbosch/detekt/blob/master/detekt-cli/src/main/resources/default-detekt-config.yml @AdamMc331 @NYAndroidMeetup 17
  39. Detekt Tasks @AdamMc331 @NYAndroidMeetup 18

  40. Detekt Tasks • ./gradlew detektGenerateConfig @AdamMc331 @NYAndroidMeetup 18

  41. Detekt Tasks • ./gradlew detektGenerateConfig • ./gradlew detektBaseline @AdamMc331 @NYAndroidMeetup

    18
  42. Detekt Tasks • ./gradlew detektGenerateConfig • ./gradlew detektBaseline • ./gradlew

    detekt @AdamMc331 @NYAndroidMeetup 18
  43. HTML Report Output Example @AdamMc331 @NYAndroidMeetup 19

  44. Novoda Gradle Sta-c Analysis Plugin8 // ./gradlew evaluateViolations // Will

    run all the tools below staticAnalysis { penalty { maxErrors = 0 maxWarnings = 0 } ktlint { } detekt { } lintOptions { } ... { } } 8 h$ps:/ /github.com/novoda/gradle-sta:c-analysis-plugin @AdamMc331 @NYAndroidMeetup 20
  45. When Should I Run This Process? @AdamMc331 @NYAndroidMeetup 21

  46. When Should I Run This Process? • As a step

    in CI flow @AdamMc331 @NYAndroidMeetup 21
  47. When Should I Run This Process? • As a step

    in CI flow • Pre push hook @AdamMc331 @NYAndroidMeetup 21
  48. When Should I Run This Process? • As a step

    in CI flow • Pre push hook • Make your build task depend on it @AdamMc331 @NYAndroidMeetup 21
  49. Some Best Prac-ces I just added these tools, my build

    failed, what do I do? @AdamMc331 @NYAndroidMeetup 22
  50. Some Best Prac-ces I just added these tools, my build

    failed, what do I do? • Fix it! @AdamMc331 @NYAndroidMeetup 22
  51. Some Best Prac-ces I just added these tools, my build

    failed, what do I do? • Fix it! • Modify the threshold for the relevant error @AdamMc331 @NYAndroidMeetup 22
  52. Some Best Prac-ces I just added these tools, my build

    failed, what do I do? • Fix it! • Modify the threshold for the relevant error • Suppress this individual case @AdamMc331 @NYAndroidMeetup 22
  53. Some Best Prac-ces I just added these tools, my build

    failed, what do I do? • Fix it! • Modify the threshold for the relevant error • Suppress this individual case • Turn off this rule or rule set @AdamMc331 @NYAndroidMeetup 22
  54. Thank You! Sample app: h*ps:/ /github.com/adammc331/pokedex Slides: h*p:/ /bit.ly/NYAndroid-StaAcAnalysis @AdamMc331

    @NYAndroidMeetup 23