Upgrade to Pro — share decks privately, control downloads, hide ads and more …

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.

Adam McNeilly

October 21, 2019
Tweet

More Decks by Adam McNeilly

Other Decks in Programming

Transcript

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

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

    be well forma1ed • Code should be free of code smells @AdamMc331 @NYAndroidMeetup 2
  3. 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
  4. What Does KtLint Enforce? • Consistent indenta,on • No wildcard

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

    imports • Consistent spacing • No empty class bodies @AdamMc331 @NYAndroidMeetup 4
  6. 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
  7. 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
  8. 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
  9. 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
  10. 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
  11. 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
  12. 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
  13. 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
  14. Avoid Code Smells With Detekt4 • Avoid complex methods 4

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

    Enforce method counts in classes 4 h$ps:/ /arturbosch.github.io/detekt/ @AdamMc331 @NYAndroidMeetup 13
  16. 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
  17. 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
  18. Why Should We Care? • Large collec+ons of code (classes,

    methods, or whatever else) are unmanageable @AdamMc331 @NYAndroidMeetup 14
  19. 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
  20. 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
  21. 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
  22. 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
  23. 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
  24. 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
  25. When Should I Run This Process? • As a step

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

    in CI flow • Pre push hook @AdamMc331 @NYAndroidMeetup 21
  27. 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
  28. Some Best Prac-ces I just added these tools, my build

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

    failed, what do I do? • Fix it! @AdamMc331 @NYAndroidMeetup 22
  30. 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
  31. 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
  32. 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