Skip to content

Commit

Permalink
Apply Detekt (#39)
Browse files Browse the repository at this point in the history
## Context
- Improving static analysis using [Detekt](https://detekt.github.io/) tool

## Code
- Improving pre-commit install and management
- Run detekt pre-commit/ci:
    - all files when any detekt configuration files change
    - for diff files otherwise

## Additional notes
- Have screenshots? Attention points?
  • Loading branch information
cmorigaki authored Nov 15, 2021
1 parent 4b2e63d commit d535df3
Show file tree
Hide file tree
Showing 20 changed files with 145 additions and 51 deletions.
10 changes: 7 additions & 3 deletions .github/workflows/pr_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ jobs:
runs-on: ubuntu-18.04

steps:
- uses: actions/checkout@v2
- name: Checkout repository
uses: actions/checkout@v2

- name: set up JDK 11
uses: actions/setup-java@v1
Expand All @@ -24,8 +25,11 @@ jobs:
- name: Decrypting api-properties
run: gpg --quiet --batch --yes --decrypt --passphrase "${{ secrets.API_PROPERTIES_PASSPHRASE }}" project-config/api-properties/api.properties.asc > project-config/api-properties/api.properties

- name: ktlint
run: ./gradlew ktlint
- name: Generate env vars
uses: FranzDiebold/github-env-vars-action@v2

- name: Run detekt
run: ./quality/detekt/detekt-ci.sh -t $GITHUB_BASE_REF -s $CI_ACTION_REF_NAME

- name: Unit tests
run: ./gradlew test --stacktrace
3 changes: 2 additions & 1 deletion .github/workflows/release_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ jobs:
runs-on: ubuntu-18.04

steps:
- uses: actions/checkout@v2
- name: Checkout repository
uses: actions/checkout@v2

- name: set up JDK 11
uses: actions/setup-java@v1
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,6 @@ lint/tmp/
#secrets
*.keystore
api.properties

#tools
.tools/
15 changes: 9 additions & 6 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ buildscript {
}
ext {
kotlinVersion = "1.5.20"
gradleVersion = '7.0.2'
gradleVersion = '7.0.3'
}

dependencies {
Expand All @@ -45,12 +45,15 @@ subprojects {
apply from: "$rootDir/quality/ktlint.gradle"
}

task installGitHook(type: Copy) {
from new File(rootProject.rootDir, 'git-hooks/pre-commit')
into { new File(rootProject.rootDir, '.git/hooks') }
fileMode 0777
task installGitHooks(type: Exec) {
group 'Quality'
description 'Installs the git hooks'
println("Installing git hooks")

workingDir rootDir
commandLine "quality/git-hooks/install-hooks.sh"
}
tasks.getByPath(':app:preBuild').dependsOn installGitHook
tasks.getByPath(':app:preBuild').dependsOn installGitHooks

task clean(type: Delete) {
delete rootProject.buildDir
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import br.com.recipebook.utilitykotlin.CommonError
import com.github.michaelbull.result.Result

internal class RecipeCollectionRepositoryImpl constructor(
private val dataSourceLocal: RecipeCollectionDataSourceLocal,
private val dataSourceRemote: RecipeCollectionDataSourceRemote
) : RecipeCollectionRepository {
override suspend fun getRecipeCollection(): Result<List<RecipeModel>, CommonError> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ val recipeCollectionDomainModule = module {
val recipeCollectionDataModule = module {
factory<RecipeCollectionRepository> {
RecipeCollectionRepositoryImpl(
dataSourceLocal = get(),
dataSourceRemote = get()
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import br.com.recipebook.recipecollection.domain.model.RecipeModel
import br.com.recipebook.recipecollection.domain.usecase.GetRecipeCollectionUseCase
import br.com.recipebook.recipecollection.view.RecipeItem
import br.com.recipebook.utilityandroid.presentation.BaseViewModel
import br.com.recipebook.utilitykotlin.CommonError
import com.github.michaelbull.result.onFailure
import com.github.michaelbull.result.onSuccess
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down Expand Up @@ -55,7 +54,7 @@ class RecipeCollectionViewModel(

getRecipeCollection()
.onSuccess { onLoadRecipeListSuccess(it) }
.onFailure { onLoadRecipeListError(it) }
.onFailure { onLoadRecipeListError() }

viewState.isLoading.value = false
}
Expand All @@ -72,7 +71,7 @@ class RecipeCollectionViewModel(
}
}

private fun onLoadRecipeListError(error: CommonError) {
private fun onLoadRecipeListError() {
sendViewEvent(false)
viewState.hasError.value = true
viewState.recipes.value = emptyList()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import br.com.recipebook.recipedetail.presentation.model.InstructionHeaderItem
import br.com.recipebook.recipedetail.presentation.model.RecipeDetailItem
import br.com.recipebook.recipedetail.view.RecipeDetailSafeArgs
import br.com.recipebook.utilityandroid.presentation.BaseViewModel
import br.com.recipebook.utilitykotlin.CommonError
import com.github.michaelbull.result.onFailure
import com.github.michaelbull.result.onSuccess
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand All @@ -29,12 +28,11 @@ class RecipeDetailViewModel(
viewModelScope.launch {
viewState.title.value = safeArgs.title
setLoadingState()
getRecipeDetail(safeArgs.recipeId).onSuccess(::onLoadSuccess).onFailure(::onLoadError)
getRecipeDetail(safeArgs.recipeId).onSuccess(::onLoadSuccess).onFailure { onLoadError() }
}
}

override fun dispatchAction(action: RecipeDetailAction) {
}
override fun dispatchAction(action: RecipeDetailAction) = Unit

private fun setLoadingState() {
viewState.isLoading.value = true
Expand Down Expand Up @@ -72,7 +70,7 @@ class RecipeDetailViewModel(
setSuccessState()
}

private fun onLoadError(error: CommonError) {
private fun onLoadError() {
sendViewEvent(false)
setErrorState()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import br.com.recipebook.settings.theme.domain.model.UserThemePreferenceModel
import br.com.recipebook.settings.theme.domain.usecase.GetUserThemePreferenceUseCase
import br.com.recipebook.settings.theme.domain.usecase.SetUserThemePreferenceUseCase
import br.com.recipebook.utilityandroid.presentation.BaseViewModel
import br.com.recipebook.utilitykotlin.CommonError
import com.github.michaelbull.result.onFailure
import com.github.michaelbull.result.onSuccess
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand All @@ -24,7 +23,7 @@ class SettingsThemeViewModel(
init {
viewModelScope.launch {
setLoadingState()
getUserThemePreference().onSuccess(::onLoadSuccess).onFailure(::onLoadError)
getUserThemePreference().onSuccess(::onLoadSuccess).onFailure{ onLoadError() }
}
}

Expand Down Expand Up @@ -78,7 +77,7 @@ class SettingsThemeViewModel(
setSuccessState()
}

private fun onLoadError(error: CommonError) {
private fun onLoadError() {
sendViewEvent(false)
setErrorState()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class SettingsViewModel(
viewState.appVersion.value = buildConfiguration.appInfo.version
viewModelScope.launch {
setLoadingState()
getSettingsList().onSuccess(::onLoadSuccess).onFailure(::onLoadError)
getSettingsList().onSuccess(::onLoadSuccess).onFailure { onLoadError() }
}
}

Expand Down Expand Up @@ -62,7 +62,7 @@ class SettingsViewModel(
setSuccessState()
}

private fun onLoadError(error: CommonError) {
private fun onLoadError() {
sendViewEvent(false)
setErrorState()
}
Expand Down
12 changes: 0 additions & 12 deletions git-hooks/pre-commit

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ class ActivityProvider(application: Application) {
override fun onActivityCreated(
activity: Activity,
savedInstanceState: Bundle?
) {
}
) = Unit

override fun onActivityStarted(activity: Activity) {
}
override fun onActivityStarted(activity: Activity) = Unit

override fun onActivityResumed(activity: Activity) {
activeActivity = activity
Expand All @@ -26,17 +24,14 @@ class ActivityProvider(application: Application) {
activeActivity = null
}

override fun onActivityStopped(activity: Activity) {
}
override fun onActivityStopped(activity: Activity) = Unit

override fun onActivitySaveInstanceState(
activity: Activity,
outState: Bundle
) {
}
) = Unit

override fun onActivityDestroyed(activity: Activity) {
}
override fun onActivityDestroyed(activity: Activity) = Unit
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class CoroutineTestRule : TestRule {
}

@ExperimentalCoroutinesApi
class TestDispatcherProvider() : DispatcherProvider {
class TestDispatcherProvider : DispatcherProvider {
private val testCoroutineDispatcher = TestCoroutineDispatcher()
override fun main() = testCoroutineDispatcher
override fun default() = testCoroutineDispatcher
Expand Down
25 changes: 25 additions & 0 deletions quality/detekt/detekt-ci.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/bin/bash

while getopts t:s: flag
do
case "${flag}" in
t) TARGET_BRANCH=${OPTARG};;
s) SOURCE_BRANCH=${OPTARG};;
esac
done

git fetch origin $TARGET_BRANCH:$TARGET_BRANCH $SOURCE_BRANCH:$SOURCE_BRANCH --no-tags

MODIFIED_DETEKT_FILES=$(git diff --diff-filter=ACMRd --name-only $TARGET_BRANCH...$SOURCE_BRANCH | grep 'detekt')

if [[ $MODIFIED_DETEKT_FILES ]]; then
echo "Configuration files have changed. All files need to be analyzed."
quality/detekt/detekt-run.sh
else
FILES_TO_ANALYZE=$(git diff --diff-filter=ACMRd --name-only $TARGET_BRANCH...$SOURCE_BRANCH | grep '\.kt$')
if [[ -z "$FILES_TO_ANALYZE" ]]; then
echo "No files to analyze!"
else
quality/detekt/detekt-run.sh -i $FILES_TO_ANALYZE
fi
fi
16 changes: 16 additions & 0 deletions quality/detekt/detekt-install.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/bin/bash

# Update detekt-run.sh on version update

mkdir -p .tools

echo "Downloading detekt cli"
curl -sSL https://github.com/detekt/detekt/releases/download/v1.19.0-RC1/detekt-cli-1.19.0-RC1.zip -o .tools/detekt-cli.zip

echo "Extracting..."
unzip -q .tools/detekt-cli.zip -d .tools/

mv .tools/detekt-cli-1.19.0-RC1 .tools/detekt-cli
rm .tools/detekt-cli.zip

echo "Installation finished"
24 changes: 24 additions & 0 deletions quality/detekt/detekt-run.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/bash

while getopts i:e: flag
do
case "${flag}" in
i) INCLUDE_PATHS=${OPTARG};;
e) EXCLUDE_PATHS=${OPTARG};;
esac
done

# Install detekt cli if necessary
DETEKT_VERSION=$(.tools/detekt-cli/bin/detekt-cli --version)
if [[ $DETEKT_VERSION != "1.19.0-RC1" ]]; then
./quality/detekt/detekt-install.sh
fi

CMD=".tools/detekt-cli/bin/detekt-cli --all-rules \
--excludes \"**/build/**,$EXCLUDE_PATHS\" "

if [[ ! -z "$INCLUDE_PATHS" ]]; then
CMD+=" --input $INCLUDE_PATHS"
fi

eval $CMD
3 changes: 3 additions & 0 deletions quality/git-hooks/install-hooks.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/bash

ln -f quality/git-hooks/pre-commit .git/hooks/pre-commit
22 changes: 22 additions & 0 deletions quality/git-hooks/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/bin/bash

hook_dir="quality/git-hooks/pre-commit.d"

echo $hook_dir

if [[ -d $hook_dir ]]; then
stdin=$(cat /dev/stdin)

for hook in $hook_dir/*; do
echo "$stdin" | $hook "$@"

exit_code=$?

if [ $exit_code != 0 ]; then
exit $exit_code
fi
done
fi

exit 0

17 changes: 17 additions & 0 deletions quality/git-hooks/pre-commit.d/pre-commit-detekt.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/bin/bash

echo "[Pre-commit] Detekt"

MODIFIED_DETEKT_FILES=$(git diff --diff-filter=ACMRd --name-only --cached --relative | grep 'detekt')

if [[ $MODIFIED_DETEKT_FILES ]]; then
echo "Configuration files have changed. All files need to be analyzed."
quality/detekt/detekt-run.sh
else
FILES_TO_ANALYZE=$(git diff --diff-filter=ACMRd --name-only --cached --relative | grep '\.kt$')
if [[ -z "$FILES_TO_ANALYZE" ]]; then
echo "No files to analyze!"
else
quality/detekt/detekt-run.sh -i $FILES_TO_ANALYZE
fi
fi
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ import kotlinx.coroutines.withContext
import retrofit2.HttpException
import java.io.IOException

/**
* FIXME: This class need some improvements
*/
@Suppress("TooGenericExceptionCaught", "ForbiddenComment")
// FIXME: This class need some improvements
suspend fun <T> safeApiCall(
dispatcher: CoroutineDispatcher,
apiCall: suspend () -> T
Expand Down

0 comments on commit d535df3

Please sign in to comment.