From dce7bcc4de2d087651628ff468b96558117bbef6 Mon Sep 17 00:00:00 2001 From: David Zou Date: Fri, 12 Jul 2019 16:13:53 -0400 Subject: [PATCH] Code style improvements This contains a number of changes regarding code style: - The Intrepid code style is now checked into version control - All files are reformatted using the updated style - Add ktlint to do basic style check, it will be run as part of prCheck - Add editorConfig to automatically add/enforce line feed at end of file --- .editorconfig | 3 + .gitignore | 4 +- .idea/codeStyles/Project.xml | 236 ++++++++++++++++++ .idea/codeStyles/codeStyleConfig.xml | 6 + .idea/vcs.xml | 6 + app/build.gradle.kts | 9 +- .../testutils/CoroutineIdlingResource.kt | 1 - .../skotlinton/testutils/TestFileUtils.kt | 1 - .../intrepid/skotlinton/base/BaseFragment.kt | 8 +- .../skotlinton/base/BaseMvvmActivity.kt | 9 +- .../intrepid/skotlinton/base/BaseViewModel.kt | 2 - .../io/intrepid/skotlinton/rest/RestApi.kt | 1 - .../skotlinton/utils/RxSchedulerExtensions.kt | 7 +- .../utils/RxSchedulerExtensionsTest.kt | 6 +- build.gradle.kts | 10 +- buildSrc/src/main/kotlin/CodeCoverage.kt | 134 +++++----- buildSrc/src/main/kotlin/Utils.kt | 2 +- 17 files changed, 356 insertions(+), 89 deletions(-) create mode 100644 .editorconfig create mode 100644 .idea/codeStyles/Project.xml create mode 100644 .idea/codeStyles/codeStyleConfig.xml create mode 100644 .idea/vcs.xml diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..736c034 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,3 @@ +root = true +[*] +insert_final_newline = true diff --git a/.gitignore b/.gitignore index 709017b..6e5cb19 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,9 @@ *.iml .gradle /local.properties -/.idea/ +/.idea/* +!.idea/codeStyles/ +!.idea/vcs.xml .DS_Store /build /captures diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml new file mode 100644 index 0000000..af61ee3 --- /dev/null +++ b/.idea/codeStyles/Project.xml @@ -0,0 +1,236 @@ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/.idea/codeStyles/codeStyleConfig.xml b/.idea/codeStyles/codeStyleConfig.xml new file mode 100644 index 0000000..80e519c --- /dev/null +++ b/.idea/codeStyles/codeStyleConfig.xml @@ -0,0 +1,6 @@ + + + + diff --git a/.idea/vcs.xml b/.idea/vcs.xml new file mode 100644 index 0000000..dcb6b8c --- /dev/null +++ b/.idea/vcs.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 19cab3d..3e573ed 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -4,8 +4,9 @@ plugins { id("kotlin-android") id("kotlin-kapt") id("io.intrepid.static-analysis") + id("org.jlleitschuh.gradle.ktlint") // Uncomment the following line after adding fabric.properties file - //id("io.fabric") + // id("io.fabric") id("jacoco") } @@ -84,7 +85,11 @@ android { } } -//https://github.com/mockk/mockk/issues/281 +ktlint { + version.set("0.33.0") +} + +// https://github.com/mockk/mockk/issues/281 configurations.all { resolutionStrategy { force("org.objenesis:objenesis:2.6") diff --git a/app/src/androidTest/kotlin/io/intrepid/skotlinton/testutils/CoroutineIdlingResource.kt b/app/src/androidTest/kotlin/io/intrepid/skotlinton/testutils/CoroutineIdlingResource.kt index 14a8b01..47dd2b4 100644 --- a/app/src/androidTest/kotlin/io/intrepid/skotlinton/testutils/CoroutineIdlingResource.kt +++ b/app/src/androidTest/kotlin/io/intrepid/skotlinton/testutils/CoroutineIdlingResource.kt @@ -17,7 +17,6 @@ class CoroutineIdlingResource(dispatcher: InterceptableDispatcher) : IdlingResou callback?.onTransitionToIdle() } } - } } diff --git a/app/src/androidTest/kotlin/io/intrepid/skotlinton/testutils/TestFileUtils.kt b/app/src/androidTest/kotlin/io/intrepid/skotlinton/testutils/TestFileUtils.kt index 153d20e..8320d78 100644 --- a/app/src/androidTest/kotlin/io/intrepid/skotlinton/testutils/TestFileUtils.kt +++ b/app/src/androidTest/kotlin/io/intrepid/skotlinton/testutils/TestFileUtils.kt @@ -4,7 +4,6 @@ import java.io.BufferedReader import java.io.IOException import java.io.InputStream import java.io.InputStreamReader -import java.lang.StringBuilder object TestFileUtils { fun convertStreamToString(inputStream: InputStream): String { diff --git a/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseFragment.kt b/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseFragment.kt index baa9d93..eae7ed3 100644 --- a/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseFragment.kt +++ b/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseFragment.kt @@ -99,10 +99,10 @@ abstract class BaseFragment : Fragment(), LiveDataObserv Timber.v("Lifecycle onStart: $this") super.onStart() viewEventDisposables += viewModel.eventObservable - .observeOn(AndroidSchedulers.mainThread()) - .subscribeBy(onNext = { - onViewEvent(it) - }) + .observeOn(AndroidSchedulers.mainThread()) + .subscribeBy(onNext = { + onViewEvent(it) + }) } @CallSuper diff --git a/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseMvvmActivity.kt b/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseMvvmActivity.kt index cd23d85..4fc4239 100644 --- a/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseMvvmActivity.kt +++ b/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseMvvmActivity.kt @@ -46,16 +46,15 @@ abstract class BaseMvvmActivity : BaseActivity() { * Override this method to do any additional view initialization (ex: setup RecycleView adapter) */ protected open fun onViewCreated(savedInstanceState: Bundle?) { - } override fun onStart() { super.onStart() viewEventDisposables += viewModel.eventObservable - .observeOn(AndroidSchedulers.mainThread()) - .subscribeBy(onNext = { - onViewEvent(it) - }) + .observeOn(AndroidSchedulers.mainThread()) + .subscribeBy(onNext = { + onViewEvent(it) + }) } override fun onStop() { diff --git a/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseViewModel.kt b/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseViewModel.kt index 819aff8..86ca6e3 100644 --- a/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseViewModel.kt +++ b/app/src/main/kotlin/io/intrepid/skotlinton/base/BaseViewModel.kt @@ -4,9 +4,7 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel import io.intrepid.skotlinton.utils.ViewEvent -import io.intrepid.skotlinton.utils.applySchedulers import io.reactivex.Observable -import io.reactivex.Single import io.reactivex.disposables.CompositeDisposable import io.reactivex.subjects.PublishSubject import kotlinx.coroutines.cancel diff --git a/app/src/main/kotlin/io/intrepid/skotlinton/rest/RestApi.kt b/app/src/main/kotlin/io/intrepid/skotlinton/rest/RestApi.kt index f0bc918..c2e18f6 100644 --- a/app/src/main/kotlin/io/intrepid/skotlinton/rest/RestApi.kt +++ b/app/src/main/kotlin/io/intrepid/skotlinton/rest/RestApi.kt @@ -1,7 +1,6 @@ package io.intrepid.skotlinton.rest import io.intrepid.skotlinton.models.IpModel -import io.reactivex.Single import retrofit2.http.GET interface RestApi { diff --git a/app/src/main/kotlin/io/intrepid/skotlinton/utils/RxSchedulerExtensions.kt b/app/src/main/kotlin/io/intrepid/skotlinton/utils/RxSchedulerExtensions.kt index f055c38..e84e65e 100644 --- a/app/src/main/kotlin/io/intrepid/skotlinton/utils/RxSchedulerExtensions.kt +++ b/app/src/main/kotlin/io/intrepid/skotlinton/utils/RxSchedulerExtensions.kt @@ -1,6 +1,11 @@ package io.intrepid.skotlinton.utils -import io.reactivex.* +import io.reactivex.Completable +import io.reactivex.Flowable +import io.reactivex.Maybe +import io.reactivex.Observable +import io.reactivex.Scheduler +import io.reactivex.Single fun Observable.applySchedulers(subscribeOnScheduler: Scheduler, observeOnScheduler: Scheduler): Observable { return this.subscribeOn(subscribeOnScheduler).observeOn(observeOnScheduler) diff --git a/app/src/test/kotlin/io/intrepid/skotlinton/utils/RxSchedulerExtensionsTest.kt b/app/src/test/kotlin/io/intrepid/skotlinton/utils/RxSchedulerExtensionsTest.kt index e01d906..c231c97 100644 --- a/app/src/test/kotlin/io/intrepid/skotlinton/utils/RxSchedulerExtensionsTest.kt +++ b/app/src/test/kotlin/io/intrepid/skotlinton/utils/RxSchedulerExtensionsTest.kt @@ -1,6 +1,10 @@ package io.intrepid.skotlinton.utils -import io.reactivex.* +import io.reactivex.Completable +import io.reactivex.Flowable +import io.reactivex.Maybe +import io.reactivex.Observable +import io.reactivex.Single import io.reactivex.schedulers.TestScheduler import org.junit.Test diff --git a/build.gradle.kts b/build.gradle.kts index 6070c4a..060bc8f 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -15,6 +15,7 @@ buildscript { classpath("com.squareup.spoon:spoon-runner:2.0.0-SNAPSHOT") // TODO update/remove this once spoon 2.0.0 is stable classpath("io.fabric.tools:gradle:1.26.1") classpath("gradle.plugin.io.intrepid:static-analysis:1.2.2") + classpath("org.jlleitschuh.gradle:ktlint-gradle:8.0.0") } } @@ -32,12 +33,13 @@ tasks.register("clean") { // Top level tasks that should be run by CI val verificationTasks = listOf( - "app:coverageMinimumDebugUnitTest", - "app:lintDebug" + "app:coverageMinimumDebugUnitTest", + "app:ktlintCheck", + "app:lintDebug" ) tasks.register("prCheck") { val tasks = verificationTasks + listOf( - "app:assembleDebug" + "app:assembleDebug" ) dependsOn(tasks) group = "verification" @@ -45,7 +47,7 @@ tasks.register("prCheck") { } tasks.register("continuousBuild") { val tasks = verificationTasks + listOf( - "app:assembleQa" + "app:assembleQa" ) dependsOn(tasks) group = "build" diff --git a/buildSrc/src/main/kotlin/CodeCoverage.kt b/buildSrc/src/main/kotlin/CodeCoverage.kt index 514f010..2a9e41b 100644 --- a/buildSrc/src/main/kotlin/CodeCoverage.kt +++ b/buildSrc/src/main/kotlin/CodeCoverage.kt @@ -15,31 +15,31 @@ object CodeCoverage { const val JACOCO_VERSION = "0.8.4" val generatedClasses = setOf( - "**/R.class", - "**/R$*.class", - "**/BuildConfig.*", - "**/Manifest*.*", - "com/android/**/*.class", - "**/*_ViewBind*" // ButterKnife auto generated classes + "**/R.class", + "**/R$*.class", + "**/BuildConfig.*", + "**/Manifest*.*", + "com/android/**/*.class", + "**/*_ViewBind*" // ButterKnife auto generated classes ) val androidClasses = setOf( - "**/*Activity.class", - "**/*Activity$*.class", - "**/*Fragment.class", - "**/*Fragment$*.class", - "**/*Service.class", - "**/*Service$*.class", - "**/*Application.class", - "**/*Application$*.class", - "**/*Dialog.class", - "**/*Dialog$*.class", - "**/*Adapter.class", - "**/*Adapter$*.class", - "**/*ViewHolder.class", - "**/*ViewHolder$*.class", - "**/*Animation.class", - "**/*Animation$*.class" + "**/*Activity.class", + "**/*Activity$*.class", + "**/*Fragment.class", + "**/*Fragment$*.class", + "**/*Service.class", + "**/*Service$*.class", + "**/*Application.class", + "**/*Application$*.class", + "**/*Dialog.class", + "**/*Dialog$*.class", + "**/*Adapter.class", + "**/*Adapter$*.class", + "**/*ViewHolder.class", + "**/*ViewHolder$*.class", + "**/*Animation.class", + "**/*Animation$*.class" ) /*** @@ -61,13 +61,13 @@ object CodeCoverage { * @param uiTestExcludedClasses classes that are excluded only from UI tests */ fun configure( - project: Project, - minimumCombinedCoverage: Double = 0.7, - minimumUnitCoverage: Double = 0.5, - minimumUiCoverage: Double = 0.6, - commonExcludedClasses: Set = generatedClasses, - unitTestExcludedClasses: Set = androidClasses, - uiTestExcludedClasses: Set = emptySet() + project: Project, + minimumCombinedCoverage: Double = 0.7, + minimumUnitCoverage: Double = 0.5, + minimumUiCoverage: Double = 0.6, + commonExcludedClasses: Set = generatedClasses, + unitTestExcludedClasses: Set = androidClasses, + uiTestExcludedClasses: Set = emptySet() ) { with(project) { plugins.apply("jacoco") @@ -98,36 +98,40 @@ object CodeCoverage { project.afterEvaluate { allVariants(project)?.forEach { configureVariant( - it, - project, - minimumCombinedCoverage, - minimumUnitCoverage, - minimumUiCoverage, - commonExcludedClasses, - unitTestExcludedClasses, - uiTestExcludedClasses) + it, + project, + minimumCombinedCoverage, + minimumUnitCoverage, + minimumUiCoverage, + commonExcludedClasses, + unitTestExcludedClasses, + uiTestExcludedClasses + ) } } } - private fun configureVariant(variant: BaseVariant, - project: Project, - minimumCombinedCoverage: Double, - minimumUnitCoverage: Double, - minimumUiCoverage: Double, - commonExcludedClasses: Set, - unitTestExcludedClasses: Set, - uiTestExcludedClasses: Set + private fun configureVariant( + variant: BaseVariant, + project: Project, + minimumCombinedCoverage: Double, + minimumUnitCoverage: Double, + minimumUiCoverage: Double, + commonExcludedClasses: Set, + unitTestExcludedClasses: Set, + uiTestExcludedClasses: Set ) { val variantNameCapitalized = variant.name.capitalize() val variantNameDecapitalized = variant.name.decapitalize() val buildDir = project.buildDir - fun createJacocoTasks(testType: String, - testTasks: Array, - minimumCoverage: Double, - excludedClasses: Set, - ecData: ConfigurableFileCollection) { + fun createJacocoTasks( + testType: String, + testTasks: Array, + minimumCoverage: Double, + excludedClasses: Set, + ecData: ConfigurableFileCollection + ) { val reportTaskName = "coverageReport${variantNameCapitalized}${testType}Test" val coverageSourceDirs = project.files("src/main/kotlin") val coverageClassDirs = project.fileTree("$buildDir/tmp/kotlin-classes/$variantNameDecapitalized") { @@ -174,28 +178,28 @@ object CodeCoverage { val unitTestExecutionData = "$buildDir/jacoco/test${variantNameCapitalized}UnitTest.exec" createJacocoTasks( - testType = "Unit", - testTasks = arrayOf("test${variantNameCapitalized}UnitTest"), - minimumCoverage = minimumUnitCoverage, - excludedClasses = commonExcludedClasses + unitTestExcludedClasses, - ecData = project.files(unitTestExecutionData) + testType = "Unit", + testTasks = arrayOf("test${variantNameCapitalized}UnitTest"), + minimumCoverage = minimumUnitCoverage, + excludedClasses = commonExcludedClasses + unitTestExcludedClasses, + ecData = project.files(unitTestExecutionData) ) val uiTestExecutionData = "$buildDir/spoon-output/$variantNameDecapitalized/coverage/merged-coverage.ec" createJacocoTasks( - testType = "Ui", - testTasks = arrayOf("spoon${variantNameCapitalized}AndroidTest"), - minimumCoverage = minimumUiCoverage, - excludedClasses = commonExcludedClasses + uiTestExcludedClasses, - ecData = project.files(uiTestExecutionData) + testType = "Ui", + testTasks = arrayOf("spoon${variantNameCapitalized}AndroidTest"), + minimumCoverage = minimumUiCoverage, + excludedClasses = commonExcludedClasses + uiTestExcludedClasses, + ecData = project.files(uiTestExecutionData) ) createJacocoTasks( - testType = "Combined", - testTasks = arrayOf("test${variantNameCapitalized}UnitTest", "spoon${variantNameCapitalized}AndroidTest"), - minimumCoverage = minimumCombinedCoverage, - excludedClasses = commonExcludedClasses, - ecData = project.files(unitTestExecutionData, uiTestExecutionData) + testType = "Combined", + testTasks = arrayOf("test${variantNameCapitalized}UnitTest", "spoon${variantNameCapitalized}AndroidTest"), + minimumCoverage = minimumCombinedCoverage, + excludedClasses = commonExcludedClasses, + ecData = project.files(unitTestExecutionData, uiTestExecutionData) ) } } diff --git a/buildSrc/src/main/kotlin/Utils.kt b/buildSrc/src/main/kotlin/Utils.kt index dd27bbb..ecaa990 100644 --- a/buildSrc/src/main/kotlin/Utils.kt +++ b/buildSrc/src/main/kotlin/Utils.kt @@ -6,7 +6,7 @@ import com.android.build.gradle.api.BaseVariant import org.gradle.api.DomainObjectSet import org.gradle.api.Project import org.gradle.kotlin.dsl.findPlugin -import java.util.* +import java.util.Properties private var localProperties: Properties? = null