Skip to content

Add detekt static analysis #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
maestromac merged 1 commit into from
Sep 16, 2019
Merged

Add detekt static analysis #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,7 @@ lint/intermediates/
lint/generated/
lint/outputs/
lint/tmp/
# lint/reports/
# lint/reports/

# Detekt Reports
reports/
17 changes: 17 additions & 0 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ buildscript {
}
plugins {
buildSrcVersions
detekt
}

allprojects {
Expand All @@ -37,4 +38,20 @@ task("clean") {

buildSrcVersions {
indent = " "
}

detekt {
input = files("$projectDir/app/src/main/java")
Copy link

@joanb joanb Sep 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding also test files?
Something like:
input = files( "src/main/java", "src/test/java", "src/androidTest/java", )

Copy link
Contributor

@sierisimo sierisimo Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my experience, the test files cause more warnings for the way they are written. I know it sounds like an excuse but it happens for example on methods or names.

Copy link
Contributor Author

@xuhaibahmad xuhaibahmad Sep 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @sierisimo. I have used static analysis tools with test code before and it really looks bad if your CI pipeline fails because of code smells in your test code.

Copy link

@joanb joanb Sep 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's harder to keep maintainability on tests, and while I haven't been in a situation where this has been a real problem, I understand your concerns. LGTM :)

config = files("$projectDir/config/detekt/detekt.yml")
reports {
xml {
enabled = true
destination = file("$projectDir/reports/detekt.xml")
}
html {
enabled = true
destination = file("$projectDir/reports/detekt.html")
}
}
parallel = true
}
6 changes: 6 additions & 0 deletions buildSrc/src/main/kotlin/Versions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ object Versions {

const val org_jetbrains_kotlin: String = "1.3.41" // available: "1.3.50"

const val io_gitlab_arturbosch_detekt: String = "1.0.1"

/**
*
* See issue 19: How to update Gradle itself?
Expand All @@ -55,3 +57,7 @@ object Versions {
val PluginDependenciesSpec.buildSrcVersions: PluginDependencySpec
inline get() =
id("de.fayard.buildSrcVersions").version(Versions.de_fayard_buildsrcversions_gradle_plugin)

val PluginDependenciesSpec.detekt: PluginDependencySpec
inline get() =
Copy link
Contributor

@sierisimo sierisimo Sep 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First time I see the inline + get in the same usage. Nice!

Copy link
Contributor Author

@xuhaibahmad xuhaibahmad Sep 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha thanks! Honestly, I just saw this in the previous line and did a quick search for usefulness. Turns out inline getters are quite handy, so I kept it.

id("io.gitlab.arturbosch.detekt").version(Versions.io_gitlab_arturbosch_detekt)
Loading