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. 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