diff --git a/modulecheck-api/src/main/kotlin/modulecheck/api/context/DependentProjects.kt b/modulecheck-api/src/main/kotlin/modulecheck/api/context/DependentProjects.kt index d530c424f2..9260db53c6 100644 --- a/modulecheck-api/src/main/kotlin/modulecheck/api/context/DependentProjects.kt +++ b/modulecheck-api/src/main/kotlin/modulecheck/api/context/DependentProjects.kt @@ -15,8 +15,10 @@ package modulecheck.api.context +import kotlinx.coroutines.flow.toSet import modulecheck.project.McProject import modulecheck.project.ProjectContext +import modulecheck.utils.filterAsync data class DependentProjects( private val delegate: Set @@ -30,7 +32,7 @@ data class DependentProjects( override suspend operator fun invoke(project: McProject): DependentProjects { val others = project.projectCache .values - .filter { otherProject -> + .filterAsync { otherProject -> project.path in otherProject .classpathDependencies() .all() diff --git a/modulecheck-api/src/main/kotlin/modulecheck/api/finding/FindingFactory.kt b/modulecheck-api/src/main/kotlin/modulecheck/api/finding/FindingFactory.kt index f539415e52..fbb61709d5 100644 --- a/modulecheck-api/src/main/kotlin/modulecheck/api/finding/FindingFactory.kt +++ b/modulecheck-api/src/main/kotlin/modulecheck/api/finding/FindingFactory.kt @@ -17,7 +17,9 @@ package modulecheck.api.finding import modulecheck.project.McProject -fun interface FindingFactory { +interface FindingFactory { - suspend fun evaluate(projects: List): List + suspend fun evaluateFixable(projects: List): List + suspend fun evaluateSorts(projects: List): List + suspend fun evaluateReports(projects: List): List } diff --git a/modulecheck-api/src/main/kotlin/modulecheck/api/rule/ModuleCheckRule.kt b/modulecheck-api/src/main/kotlin/modulecheck/api/rule/ModuleCheckRule.kt index 9399b39766..262ce3516d 100644 --- a/modulecheck-api/src/main/kotlin/modulecheck/api/rule/ModuleCheckRule.kt +++ b/modulecheck-api/src/main/kotlin/modulecheck/api/rule/ModuleCheckRule.kt @@ -33,6 +33,9 @@ interface ModuleCheckRule { fun McProject.kotlinBuildFileOrNull(): KtFile? = buildFile.asKtsFileOrNull() } +interface ReportOnlyRule : ModuleCheckRule +interface SortRule : ModuleCheckRule + fun interface RuleFactory { fun create(settings: ModuleCheckSettings): List> diff --git a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/DepthRule.kt b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/DepthRule.kt index e5cfdcfe91..97856c7b8c 100644 --- a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/DepthRule.kt +++ b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/DepthRule.kt @@ -17,11 +17,11 @@ package modulecheck.core.rule import modulecheck.api.DepthFinding import modulecheck.api.context.depthForSourceSetName -import modulecheck.api.rule.ModuleCheckRule +import modulecheck.api.rule.ReportOnlyRule import modulecheck.api.settings.ChecksSettings import modulecheck.project.McProject -class DepthRule : ModuleCheckRule { +class DepthRule : ReportOnlyRule { override val id = "Depth" override val description = "The longest path between this module and its leaf nodes" diff --git a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/DisableViewBindingRule.kt b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/DisableViewBindingRule.kt index 21890f5cfe..e5e721e901 100644 --- a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/DisableViewBindingRule.kt +++ b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/DisableViewBindingRule.kt @@ -86,7 +86,11 @@ class DisableViewBindingRule : ModuleCheckRule> ) : FindingFactory { - override suspend fun evaluate(projects: List): List { + override suspend fun evaluateFixable(projects: List): List { + return evaluate(projects) { it !is SortRule && it !is ReportOnlyRule } + } + + override suspend fun evaluateSorts(projects: List): List { + return evaluate(projects) { it is SortRule } + } + + override suspend fun evaluateReports(projects: List): List { + return evaluate(projects) { it is ReportOnlyRule } + } + + private suspend fun evaluate( + projects: List, + predicate: (ModuleCheckRule<*>) -> Boolean + ): List { return coroutineScope { projects.flatMap { project -> rules - .filter { it.shouldApply(settings.checks) } + .filter { predicate(it) && it.shouldApply(settings.checks) } .map { rule -> async { rule.check(project) } } } .awaitAll() diff --git a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SingleRuleFindingFactory.kt b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SingleRuleFindingFactory.kt index 1152fb12cc..64be6957cf 100644 --- a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SingleRuleFindingFactory.kt +++ b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SingleRuleFindingFactory.kt @@ -21,13 +21,33 @@ import kotlinx.coroutines.coroutineScope import modulecheck.api.finding.Finding import modulecheck.api.finding.FindingFactory import modulecheck.api.rule.ModuleCheckRule +import modulecheck.api.rule.ReportOnlyRule +import modulecheck.api.rule.SortRule import modulecheck.project.McProject class SingleRuleFindingFactory( val rule: ModuleCheckRule ) : FindingFactory { - override suspend fun evaluate(projects: List): List { + override suspend fun evaluateFixable(projects: List): List { + return if (rule !is SortRule && rule !is ReportOnlyRule) { + evaluateRule(projects) + } else emptyList() + } + + override suspend fun evaluateSorts(projects: List): List { + return if (rule is SortRule) { + evaluateRule(projects) + } else emptyList() + } + + override suspend fun evaluateReports(projects: List): List { + return if (rule is ReportOnlyRule) { + evaluateRule(projects) + } else emptyList() + } + + private suspend fun evaluateRule(projects: List): List { return coroutineScope { projects .map { project -> async { rule.check(project) } } diff --git a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SortDependenciesRule.kt b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SortDependenciesRule.kt index 8a207ed322..c5d6680e07 100644 --- a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SortDependenciesRule.kt +++ b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SortDependenciesRule.kt @@ -15,7 +15,7 @@ package modulecheck.core.rule -import modulecheck.api.rule.ModuleCheckRule +import modulecheck.api.rule.SortRule import modulecheck.api.settings.ChecksSettings import modulecheck.api.settings.ModuleCheckSettings import modulecheck.core.parse @@ -27,7 +27,7 @@ import java.util.Locale class SortDependenciesRule( settings: ModuleCheckSettings -) : ModuleCheckRule { +) : SortRule { override val id = "SortDependencies" override val description = "Sorts all dependencies within a dependencies { ... } block" diff --git a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SortPluginsRule.kt b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SortPluginsRule.kt index b02d3ea28e..715a4d3a71 100644 --- a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SortPluginsRule.kt +++ b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/SortPluginsRule.kt @@ -15,7 +15,7 @@ package modulecheck.core.rule -import modulecheck.api.rule.ModuleCheckRule +import modulecheck.api.rule.SortRule import modulecheck.api.settings.ChecksSettings import modulecheck.api.settings.ModuleCheckSettings import modulecheck.core.parse @@ -27,7 +27,7 @@ import modulecheck.project.McProject class SortPluginsRule( settings: ModuleCheckSettings -) : ModuleCheckRule { +) : SortRule { override val id = "SortPlugins" override val description = diff --git a/modulecheck-core/src/test/kotlin/modulecheck/core/DepthOutputTest.kt b/modulecheck-core/src/test/kotlin/modulecheck/core/DepthOutputTest.kt index 96d5fc1fee..05211290c9 100644 --- a/modulecheck-core/src/test/kotlin/modulecheck/core/DepthOutputTest.kt +++ b/modulecheck-core/src/test/kotlin/modulecheck/core/DepthOutputTest.kt @@ -17,6 +17,7 @@ package modulecheck.core import modulecheck.core.rule.DepthRule import modulecheck.core.rule.ModuleCheckRuleFactory +import modulecheck.core.rule.MultiRuleFindingFactory import modulecheck.core.rule.SingleRuleFindingFactory import modulecheck.project.ConfigurationName import modulecheck.runtime.test.RunnerTest @@ -62,6 +63,54 @@ internal class DepthOutputTest : RunnerTest() { """ } + @Test + fun `reported depths should be from after fixes are applied`() { + + settings.checks.depths = true + + val runner = runner( + autoCorrect = true, + findingFactory = MultiRuleFindingFactory( + settings, + ruleFactory.create(settings) + ) + ) + + val lib1 = project(":lib1") + + project(":lib2") { + addDependency(ConfigurationName.implementation, lib1) + + buildFile.writeText( + """ + plugins { + kotlin("jvm") + } + + dependencies { + implementation(project(path = ":lib1")) + } + """.trimIndent() + ) + } + + runner.run(allProjects()).isSuccess shouldBe true + + logger.collectReport() + .joinToString() + .clean() shouldBe """ + :lib2 + dependency name source build file + ✔ :lib1 unusedDependency /lib2/build.gradle.kts: (6, 3): + + -- ModuleCheck main source set depth results -- + depth modules + 0 [:lib1, :lib2] + + ModuleCheck found 1 issue + """ + } + @Test fun `test source set depths should not be reported`() { diff --git a/modulecheck-core/src/test/kotlin/modulecheck/core/DepthReportTest.kt b/modulecheck-core/src/test/kotlin/modulecheck/core/DepthReportTest.kt index 814b2a6add..7b7cb03296 100644 --- a/modulecheck-core/src/test/kotlin/modulecheck/core/DepthReportTest.kt +++ b/modulecheck-core/src/test/kotlin/modulecheck/core/DepthReportTest.kt @@ -18,6 +18,7 @@ package modulecheck.core import modulecheck.api.test.TestSettings import modulecheck.core.rule.DepthRule import modulecheck.core.rule.ModuleCheckRuleFactory +import modulecheck.core.rule.MultiRuleFindingFactory import modulecheck.core.rule.SingleRuleFindingFactory import modulecheck.project.ConfigurationName import modulecheck.project.SourceSet @@ -246,6 +247,112 @@ internal class DepthReportTest : RunnerTest() { """ } + @Test + fun `depth report should be calculated after fixes are applied`() { + + settings.checks.depths = true + settings.reports.depths.enabled = true + + val runner = runner( + autoCorrect = true, + findingFactory = MultiRuleFindingFactory( + settings, + ruleFactory.create(settings) + ) + ) + + val lib1 = project(":lib1") { + + addSource( + "com/modulecheck/lib1/Lib1Class.kt", + """ + package com.modulecheck.lib1 + + class Lib1Class + """.trimIndent() + ) + } + + val lib2 = project(":lib2") { + addDependency(ConfigurationName.implementation, lib1) + + addSource( + "com/modulecheck/lib2/Lib2Class.kt", + """ + package com.modulecheck.lib2 + + class Lib2Class + """.trimIndent() + ) + + buildFile.writeText( + """ + plugins { + kotlin("jvm") + } + + dependencies { + implementation(project(path = ":lib1")) + } + """.trimIndent() + ) + } + + project(":app") { + addDependency(ConfigurationName.implementation, lib1) + addDependency(ConfigurationName.implementation, lib2) + + addSource( + "com/modulecheck/app/App.kt", + """ + package com.modulecheck.app + + import com.modulecheck.lib1.Lib1Class + import com.modulecheck.lib2.Lib2Class + + class App { + private val lib1Class = Lib1Class() + private val lib2Class = Lib2Class() + } + """.trimIndent() + ) + } + + runner.run(allProjects()).isSuccess shouldBe true + + logger.collectReport() + .joinToString() + .clean() shouldBe """ + :lib2 + dependency name source build file + ✔ :lib1 unusedDependency /lib2/build.gradle.kts: (6, 3): + + -- ModuleCheck main source set depth results -- + depth modules + 0 [:lib1, :lib2] + 1 [:app] + + ModuleCheck found 1 issue + """ + + outputFile.readText() shouldBe """ + -- ModuleCheck Depth results -- + + :app + source set depth most expensive dependencies + main 1 [:lib1, :lib2] + + :lib1 + source set depth most expensive dependencies + main 0 [] + + :lib2 + source set depth most expensive dependencies + main 0 [] + + """ + } + @Test fun `debug source set depth should be reported`() { diff --git a/modulecheck-parsing/core/src/main/kotlin/modulecheck/parsing/ProjectProvider.kt b/modulecheck-parsing/core/src/main/kotlin/modulecheck/parsing/ProjectProvider.kt index 9ee7bd7326..de83ad91dd 100644 --- a/modulecheck-parsing/core/src/main/kotlin/modulecheck/parsing/ProjectProvider.kt +++ b/modulecheck-parsing/core/src/main/kotlin/modulecheck/parsing/ProjectProvider.kt @@ -21,4 +21,8 @@ import modulecheck.project.McProject interface ProjectProvider : HasProjectCache { fun get(path: String): McProject + + fun getAll(): List + + fun clearCaches() } diff --git a/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/GradleProjectProvider.kt b/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/GradleProjectProvider.kt index e507d6bcc9..359890c92a 100644 --- a/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/GradleProjectProvider.kt +++ b/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/GradleProjectProvider.kt @@ -29,6 +29,7 @@ import com.squareup.anvil.plugin.AnvilExtension import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject +import modulecheck.api.settings.ModuleCheckSettings import modulecheck.core.parse import modulecheck.core.rule.KAPT_PLUGIN_ID import modulecheck.gradle.internal.androidManifests @@ -68,7 +69,8 @@ import kotlin.LazyThreadSafetyMode.NONE class GradleProjectProvider @AssistedInject constructor( @Assisted - rootGradleProject: GradleProject, + private val rootGradleProject: GradleProject, + private val settings: ModuleCheckSettings, override val projectCache: ProjectCache, private val gradleLogger: GradleLogger ) : ProjectProvider { @@ -82,6 +84,17 @@ class GradleProjectProvider @AssistedInject constructor( } } + override fun getAll(): List { + return rootGradleProject.allprojects + .filter { it.buildFile.exists() } + .filterNot { it.path in settings.doNotCheck } + .map { get(it.path) } + } + + override fun clearCaches() { + projectCache.clearContexts() + } + @Suppress("UnstableApiUsage") private fun createProject(path: String): McProject { val gradleProject = gradleProjects.getValue(path) diff --git a/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/task/ModuleCheckTask.kt b/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/task/ModuleCheckTask.kt index f27c8785ca..31b2c476b0 100644 --- a/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/task/ModuleCheckTask.kt +++ b/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/task/ModuleCheckTask.kt @@ -56,14 +56,10 @@ open class ModuleCheckTask @Inject constructor( Components.add(component) - val runner = component.runnerFactory.create(autoCorrect) val projectProvider = component.gradleProjectProvider.create(project) + val runner = component.runnerFactory.create(projectProvider, autoCorrect) - val projects = project - .allprojects - .filter { it.buildFile.exists() } - .filterNot { it.path in settings.doNotCheck } - .map { projectProvider.get(it.path) } + val projects = projectProvider.getAll() val result = runner.run(projects) diff --git a/modulecheck-project/api/src/main/kotlin/modulecheck/project/McProject.kt b/modulecheck-project/api/src/main/kotlin/modulecheck/project/McProject.kt index 3e2596a5f9..7999ef6030 100644 --- a/modulecheck-project/api/src/main/kotlin/modulecheck/project/McProject.kt +++ b/modulecheck-project/api/src/main/kotlin/modulecheck/project/McProject.kt @@ -15,14 +15,9 @@ package modulecheck.project -import modulecheck.dagger.AppScope -import modulecheck.dagger.SingleIn import modulecheck.project.temp.AnvilGradlePlugin import org.jetbrains.kotlin.name.FqName import java.io.File -import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.ConcurrentMap -import javax.inject.Inject import kotlin.contracts.contract @Suppress("TooManyFunctions") @@ -86,6 +81,3 @@ interface AndroidMcProject : McProject { val androidRFqNameOrNull: String? val manifests: Map } - -@SingleIn(AppScope::class) -class ProjectCache @Inject constructor() : ConcurrentMap by ConcurrentHashMap() diff --git a/modulecheck-project/api/src/main/kotlin/modulecheck/project/ProjectCache.kt b/modulecheck-project/api/src/main/kotlin/modulecheck/project/ProjectCache.kt new file mode 100644 index 0000000000..0e3677ae68 --- /dev/null +++ b/modulecheck-project/api/src/main/kotlin/modulecheck/project/ProjectCache.kt @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2021 Rick Busarow + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package modulecheck.project + +import modulecheck.dagger.AppScope +import modulecheck.dagger.SingleIn +import java.util.concurrent.ConcurrentHashMap +import javax.inject.Inject + +@SingleIn(AppScope::class) +class ProjectCache @Inject constructor() { + private val delegate = ConcurrentHashMap() + + val values: MutableCollection get() = delegate.values + + fun getOrPut(path: String, defaultValue: () -> McProject): McProject { + return delegate.getOrPut(path, defaultValue) + } + + fun getValue(path: String): McProject { + return delegate.getValue(path) + } + + operator fun set(path: String, project: McProject): McProject? { + return delegate.put(path, project) + } + + fun clearContexts() { + delegate.values.forEach { it.clearContext() } + } +} diff --git a/modulecheck-project/api/src/main/kotlin/modulecheck/project/ProjectContext.kt b/modulecheck-project/api/src/main/kotlin/modulecheck/project/ProjectContext.kt index 847611eb2a..5302233aeb 100644 --- a/modulecheck-project/api/src/main/kotlin/modulecheck/project/ProjectContext.kt +++ b/modulecheck-project/api/src/main/kotlin/modulecheck/project/ProjectContext.kt @@ -22,6 +22,8 @@ import modulecheck.utils.SafeCache interface ProjectContext { suspend fun get(key: Key): E + fun clearContext() + interface Key { suspend operator fun invoke(project: McProject): E } @@ -29,15 +31,23 @@ interface ProjectContext { interface Element { val key: Key<*> } + + companion object { + operator fun invoke(project: McProject): ProjectContext = RealProjectContext(project) + } } -class RealProjectContext(val project: McProject) : ProjectContext { +internal class RealProjectContext(val project: McProject) : ProjectContext { - private val cache = SafeCache, Element>() + private var cache = SafeCache, Element>() override suspend fun get(key: Key): E { @Suppress("UNCHECKED_CAST") return cache.getOrPut(key) { key.invoke(project) } as E } + + override fun clearContext() { + cache = SafeCache() + } } diff --git a/modulecheck-project/api/src/testFixtures/kotlin/modulecheck/project/test/ProjectTest.kt b/modulecheck-project/api/src/testFixtures/kotlin/modulecheck/project/test/ProjectTest.kt index ca084f9f7e..52ccd62fff 100644 --- a/modulecheck-project/api/src/testFixtures/kotlin/modulecheck/project/test/ProjectTest.kt +++ b/modulecheck-project/api/src/testFixtures/kotlin/modulecheck/project/test/ProjectTest.kt @@ -15,12 +15,12 @@ package modulecheck.project.test +import modulecheck.parsing.ProjectProvider import modulecheck.project.AndroidMcProject import modulecheck.project.ConfigurationName import modulecheck.project.ConfiguredProjectDependency import modulecheck.project.McProject import modulecheck.project.ProjectCache -import modulecheck.project.ProjectProvider import modulecheck.testing.BaseTest import java.io.File import java.nio.charset.Charset @@ -38,6 +38,12 @@ abstract class ProjectTest : BaseTest() { override fun get(path: String): McProject { return projectCache.getValue(path) } + + override fun getAll(): List = allProjects() + + override fun clearCaches() { + allProjects().forEach { it.clearContext() } + } } } diff --git a/modulecheck-project/impl/src/main/kotlin/modulecheck/project/impl/RealAndroidMcProject.kt b/modulecheck-project/impl/src/main/kotlin/modulecheck/project/impl/RealAndroidMcProject.kt index 12e578b42c..a5ebe6c4ef 100644 --- a/modulecheck-project/impl/src/main/kotlin/modulecheck/project/impl/RealAndroidMcProject.kt +++ b/modulecheck-project/impl/src/main/kotlin/modulecheck/project/impl/RealAndroidMcProject.kt @@ -25,7 +25,6 @@ import modulecheck.project.McProject import modulecheck.project.ProjectCache import modulecheck.project.ProjectContext import modulecheck.project.ProjectDependencies -import modulecheck.project.RealProjectContext import modulecheck.project.SourceSetName import modulecheck.project.SourceSets import modulecheck.project.temp.AnvilGradlePlugin @@ -58,7 +57,11 @@ class RealAndroidMcProject( androidPackageOrNull?.let { "$it.R" } } - private val context = RealProjectContext(this) + private val context = ProjectContext(this) + + override fun clearContext() { + context.clearContext() + } override suspend fun get(key: ProjectContext.Key): E { return context.get(key) diff --git a/modulecheck-project/impl/src/main/kotlin/modulecheck/project/impl/RealMcProject.kt b/modulecheck-project/impl/src/main/kotlin/modulecheck/project/impl/RealMcProject.kt index 538117fe6e..39f6b740e6 100644 --- a/modulecheck-project/impl/src/main/kotlin/modulecheck/project/impl/RealMcProject.kt +++ b/modulecheck-project/impl/src/main/kotlin/modulecheck/project/impl/RealMcProject.kt @@ -24,7 +24,6 @@ import modulecheck.project.McProject import modulecheck.project.ProjectCache import modulecheck.project.ProjectContext import modulecheck.project.ProjectDependencies -import modulecheck.project.RealProjectContext import modulecheck.project.SourceSetName import modulecheck.project.SourceSets import modulecheck.project.temp.AnvilGradlePlugin @@ -50,7 +49,11 @@ class RealMcProject( override val externalDependencies: ExternalDependencies by externalDependencies - private val context = RealProjectContext(this) + private val context = ProjectContext(this) + + override fun clearContext() { + context.clearContext() + } override suspend fun get(key: ProjectContext.Key): E { return context.get(key) diff --git a/modulecheck-runtime/src/main/kotlin/modulecheck/runtime/ModuleCheckRunner.kt b/modulecheck-runtime/src/main/kotlin/modulecheck/runtime/ModuleCheckRunner.kt index 61cd3f3de6..c0bd31d6b8 100644 --- a/modulecheck-runtime/src/main/kotlin/modulecheck/runtime/ModuleCheckRunner.kt +++ b/modulecheck-runtime/src/main/kotlin/modulecheck/runtime/ModuleCheckRunner.kt @@ -22,10 +22,12 @@ import dispatch.core.DispatcherProvider import kotlinx.coroutines.runBlocking import modulecheck.api.DepthFinding import modulecheck.api.finding.Finding +import modulecheck.api.finding.Finding.FindingResult import modulecheck.api.finding.FindingFactory import modulecheck.api.finding.FindingResultFactory import modulecheck.api.finding.Problem import modulecheck.api.settings.ModuleCheckSettings +import modulecheck.parsing.ProjectProvider import modulecheck.project.Logger import modulecheck.project.McProject import modulecheck.reporting.checkstyle.CheckstyleReporter @@ -34,7 +36,6 @@ import modulecheck.reporting.console.DepthReportFactory import modulecheck.reporting.console.ReportFactory import modulecheck.reporting.graphviz.GraphvizFileWriter import java.io.File -import kotlin.properties.Delegates import kotlin.system.measureTimeMillis /** @@ -57,6 +58,8 @@ data class ModuleCheckRunner @AssistedInject constructor( val graphvizFileWriter: GraphvizFileWriter, val dispatcherProvider: DispatcherProvider, @Assisted + val projectProvider: ProjectProvider, + @Assisted val autoCorrect: Boolean ) { @@ -64,21 +67,46 @@ data class ModuleCheckRunner @AssistedInject constructor( // total findings, whether they're fixed or not var totalFindings = 0 - var allFindings by Delegates.notNull>() + val allFindings = mutableListOf() // number of findings which couldn't be fixed // time does not include initial parsing from GradleProjectProvider, // but does include all source file parsing and the amount of time spent applying fixes - val unfixedCountWithTime = measured { - allFindings = findingFactory.evaluate(projects).distinct() + val resultsWithTime = measured { + val fixableFindings = findingFactory.evaluateFixable(projects).distinct() + + val fixableResults = fixableFindings.filterIsInstance() + .filterNot { it.shouldSkip() } + .also { totalFindings += it.size } + .let { processFindings(it) } + + projectProvider.clearCaches() - allFindings.filterIsInstance() + val sortFindings = findingFactory.evaluateSorts(projectProvider.getAll()) + .distinct() + + val sortsResults = sortFindings.filterIsInstance() .filterNot { it.shouldSkip() } - .also { totalFindings = it.size } + .also { totalFindings += it.size } .let { processFindings(it) } + + projectProvider.clearCaches() + + val reportOnlyFindings = findingFactory.evaluateReports(projectProvider.getAll()) + .distinct() + + processFindings(reportOnlyFindings) + + allFindings += (fixableFindings + sortFindings + reportOnlyFindings) + + fixableResults + sortsResults // + reportResults } - val totalUnfixedIssues = unfixedCountWithTime.data + val allResults = resultsWithTime.data + + reportResults(allResults) + + val totalUnfixedIssues = allResults.count { !it.fixed } val depths = allFindings.filterIsInstance() maybeLogDepths(depths) @@ -87,7 +115,7 @@ data class ModuleCheckRunner @AssistedInject constructor( // Replace this with kotlinx Duration APIs as soon as it's stable @Suppress("MagicNumber") - val secondsDouble = unfixedCountWithTime.timeMillis / 1000.0 + val secondsDouble = resultsWithTime.timeMillis / 1000.0 val issuePlural = if (totalFindings == 1) "issue" else "issues" @@ -120,7 +148,7 @@ data class ModuleCheckRunner @AssistedInject constructor( /** * Tries to fix all findings one project at a time, then reports the results. */ - private fun processFindings(findings: List): Int { + private fun processFindings(findings: List): List { // TODO - The order of applying fixes is stable, which may be important in troubleshooting, but // it's probably not perfect. There is a chance that up-stream changes to a dependency can @@ -144,6 +172,14 @@ data class ModuleCheckRunner @AssistedInject constructor( ) } + return results + } + + /** + * Creates any applicable reports. + */ + private fun reportResults(results: List) { + val textReport = reportFactory.create(results) if (results.isNotEmpty()) { @@ -165,8 +201,6 @@ data class ModuleCheckRunner @AssistedInject constructor( .also { it.parentFile.mkdirs() } .writeText(checkstyleReporter.createXml(results)) } - - return results.count { !it.fixed } } private fun maybeLogDepths(depths: List) { @@ -209,7 +243,7 @@ data class ModuleCheckRunner @AssistedInject constructor( @AssistedFactory interface Factory { - fun create(autoCorrect: Boolean): ModuleCheckRunner + fun create(projectProvider: ProjectProvider, autoCorrect: Boolean): ModuleCheckRunner } } diff --git a/modulecheck-runtime/src/testFixtures/kotlin/modulecheck/runtime/test/RunnerTest.kt b/modulecheck-runtime/src/testFixtures/kotlin/modulecheck/runtime/test/RunnerTest.kt index 533e3db654..bdf2f1ee48 100644 --- a/modulecheck-runtime/src/testFixtures/kotlin/modulecheck/runtime/test/RunnerTest.kt +++ b/modulecheck-runtime/src/testFixtures/kotlin/modulecheck/runtime/test/RunnerTest.kt @@ -23,7 +23,9 @@ import modulecheck.api.finding.RealFindingResultFactory import modulecheck.api.settings.ModuleCheckSettings import modulecheck.api.test.ReportingLogger import modulecheck.api.test.TestSettings +import modulecheck.parsing.ProjectProvider import modulecheck.project.Logger +import modulecheck.project.McProject import modulecheck.project.test.ProjectTest import modulecheck.reporting.checkstyle.CheckstyleReporter import modulecheck.reporting.console.ReportFactory @@ -42,6 +44,7 @@ abstract class RunnerTest : ProjectTest() { findingFactory: FindingFactory, settings: ModuleCheckSettings = this.settings, logger: Logger = this.logger, + projectProvider: ProjectProvider = this.projectProvider, findingResultFactory: FindingResultFactory = RealFindingResultFactory(), reportFactory: ReportFactory = ReportFactory(), checkstyleReporter: CheckstyleReporter = CheckstyleReporter(), @@ -56,20 +59,17 @@ abstract class RunnerTest : ProjectTest() { reportFactory = reportFactory, checkstyleReporter = checkstyleReporter, graphvizFileWriter = graphvizFileWriter, - dispatcherProvider = dispatcherProvider + dispatcherProvider = dispatcherProvider, + projectProvider = projectProvider ) fun findingFactory( - findings: List = emptyList() - ): FindingFactory<*> = FindingFactory { findings } - - // fun findingFactory( - // fixable: List = emptyList(), - // sorts: List = emptyList(), - // reports: List = emptyList() - // ): FindingFactory<*> = object : FindingFactory { - // override suspend fun evaluateFixable(projects: List): List = fixable - // override suspend fun evaluateSorts(projects: List): List = sorts - // override suspend fun evaluateReports(projects: List): List = reports - // } + fixable: List = emptyList(), + sorts: List = emptyList(), + reports: List = emptyList() + ): FindingFactory<*> = object : FindingFactory { + override suspend fun evaluateFixable(projects: List): List = fixable + override suspend fun evaluateSorts(projects: List): List = sorts + override suspend fun evaluateReports(projects: List): List = reports + } } diff --git a/modulecheck-utils/src/main/kotlin/modulecheck/utils/SafeCache.kt b/modulecheck-utils/src/main/kotlin/modulecheck/utils/SafeCache.kt index deed785559..44a662bb3a 100644 --- a/modulecheck-utils/src/main/kotlin/modulecheck/utils/SafeCache.kt +++ b/modulecheck-utils/src/main/kotlin/modulecheck/utils/SafeCache.kt @@ -36,7 +36,7 @@ interface SafeCache { } } -class RealSafeCache( +internal class RealSafeCache( delegate: ConcurrentHashMap = ConcurrentHashMap(), private val lockCache: ConcurrentHashMap = ConcurrentHashMap() ) : SafeCache {