Skip to content
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.

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

Add default classpath to enable type resolution #154

Merged
merged 2 commits into from
Aug 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
43 changes: 0 additions & 43 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,49 +187,6 @@ This will generate a baseline file for each module named as `baseline-<module-na
</build>
```

## Using Type Resolution

See [Issue #144](https://github.com/Ozsie/detekt-maven-plugin/issues/144) for an explanation.

```xml
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<version>3.2.0</version>
<executions>
<execution>
<id>generate-classpath-var</id>
<phase>package</phase>
<goals><goal>build-classpath</goal></goals>
<configuration>
<outputProperty>generated.classpath</outputProperty>
<silent>true</silent>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.github.ozsie</groupId>
<artifactId>detekt-maven-plugin</artifactId>
<version>1.21.0</version>
<configuration>
<baseline>baseline.xml</baseline>
<classPath>${generated.classpath}</classPath>
<jvmTarget>17</jvmTarget>
</configuration>
<executions>
<execution>
<phase>verify</phase>
<goals><goal>check</goal></goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
```

### Goals
#### check

Expand Down
2 changes: 1 addition & 1 deletion baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<SmellBaseline>
<ManuallySuppressedIssues></ManuallySuppressedIssues>
<CurrentIssues>
<ID>InvalidPackageDeclaration:WildcardImportViolated.kt$package `code-samples`.`invalid-package-naming`</ID>
<ID>PackageNaming:ValidFile.kt$package `code-samples`.`valid`</ID>
<ID>PackageNaming:WildcardImportViolated.kt$package `code-samples`.`invalid-package-naming`</ID>
</CurrentIssues>
</SmellBaseline>
18 changes: 15 additions & 3 deletions src/main/java/com/github/ozsie/CheckMojo.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.github.ozsie

import io.github.detekt.tooling.api.MaxIssuesReached
import io.gitlab.arturbosch.detekt.cli.CliArgs
import io.gitlab.arturbosch.detekt.cli.parseArguments
import io.gitlab.arturbosch.detekt.cli.runners.Runner
import org.apache.maven.plugins.annotations.LifecyclePhase
Expand All @@ -15,18 +16,29 @@ import java.nio.file.Paths
threadSafe = true,
requiresDependencyCollection = ResolutionScope.TEST)
class CheckMojo : DetektMojo() {
lateinit var cliArgs: CliArgs
Copy link
Author

Choose a reason for hiding this comment

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

set to a field for testing purposes. Maybe there is another smart idea of how we can assert the expected outcome.


override fun execute() {
getCliSting().forEach {
log.debug("Applying $it")
}
val cliArgs = parseArguments(getCliSting().log().toTypedArray())
this.cliArgs = parseArguments(getCliSting().log().toTypedArray())

val foundInputDir = Files.isDirectory(Paths.get(input))
if (!skip && foundInputDir)
if (!skip && foundInputDir) {
setDefaultClasspathIfNotSet(cliArgs)
failBuildIfNeeded { Runner(cliArgs, System.out, System.err).execute() }
else
} else
inputSkipLog(skip)
}

private fun setDefaultClasspathIfNotSet(cliArgs: CliArgs) {
if (cliArgs.classpath.isNullOrBlank()) {
cliArgs.classpath = mavenProject?.compileClasspathElements?.joinToString(
java.io.File.pathSeparatorChar.toString())
}
}

private fun failBuildIfNeeded(block: () -> Unit) {
try {
block()
Expand Down
49 changes: 34 additions & 15 deletions src/test/java/com/github/ozsie/CheckMojoSpec.kt
Original file line number Diff line number Diff line change
@@ -1,22 +1,14 @@
package com.github.ozsie

import com.github.ozsie.test.CheckMojoTestFactory
import io.github.detekt.tooling.api.MaxIssuesReached
import org.jetbrains.spek.api.Spek
import org.jetbrains.spek.api.dsl.given
import org.jetbrains.spek.api.dsl.on
import java.net.URI
import java.nio.file.Paths
import kotlin.test.assertFailsWith
import kotlin.test.expect


class CheckMojoSpec : Spek({
val invalidPackageNamingDirectoryPath by lazy {
Copy link
Author

Choose a reason for hiding this comment

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

Extracted a test factors to separate concerns, reduce duplication and improve readability

val uri = CheckMojoSpec::class.java.classLoader
.getResource("code-samples/invalid-package-naming")!!.toURI()
Paths.get(uri).toString()
}

given("a CheckMojo and 'skip' is true") {
val checkMojo = CheckMojo()
checkMojo.skip = true
Expand All @@ -42,11 +34,11 @@ class CheckMojoSpec : Spek({
}

given("a CheckMojo and 'failBuildOnMaxIssuesReached' is false") {
val checkMojo = CheckMojo().apply {
input = invalidPackageNamingDirectoryPath
val checkMojo = CheckMojoTestFactory.createWithInvalidPackageNamingStructure {
failBuildOnMaxIssuesReached = false
failFast = true // fail on any issue
failFast = true
}

on("checkMojo.execute()") {
test("Unit is expected") {
expect(Unit) {
Expand All @@ -57,10 +49,9 @@ class CheckMojoSpec : Spek({
}

given("a CheckMojo and 'failBuildOnMaxIssuesReached' is true") {
val checkMojo = CheckMojo().apply {
input = invalidPackageNamingDirectoryPath
val checkMojo = CheckMojoTestFactory.createWithInvalidPackageNamingStructure {
failBuildOnMaxIssuesReached = true
failFast = true // fail on any issue
failFast = true
}
on("checkMojo.execute()") {
test("Unit is expected") {
Expand All @@ -70,4 +61,32 @@ class CheckMojoSpec : Spek({
}
}
}

given("classpath parameter") {
val checkMojo = CheckMojoTestFactory.createWithNoRuleExecution {
classPath = "/tmp/provided"
}

on("checkMojo.execute()") {
test("uses provider value") {
expect("/tmp/provided") {
checkMojo.execute()
checkMojo.cliArgs.classpath
}
}
}
}

given("no classpath parameter") {
val checkMojo = CheckMojoTestFactory.createWithNoRuleExecution()

on("checkMojo.execute()") {
test("uses default compileClasspathElements") {
expect("/tmp/default${java.io.File.pathSeparatorChar}/tmp/default2") {
checkMojo.execute()
checkMojo.cliArgs.classpath
}
}
}
}
})
69 changes: 69 additions & 0 deletions src/test/java/com/github/ozsie/test/CheckMojoTestFactory.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.github.ozsie.test

import com.github.ozsie.CheckMojo
import com.github.ozsie.CheckMojoSpec
import com.nhaarman.mockito_kotlin.any
import com.nhaarman.mockito_kotlin.doReturn
import com.nhaarman.mockito_kotlin.mock
import org.apache.maven.model.Dependency
import org.apache.maven.model.Plugin
import org.apache.maven.project.MavenProject
import java.nio.file.Paths

object CheckMojoTestFactory {

private val invalidPackageNamingDirectoryPath by lazy {
val uri = CheckMojoSpec::class.java.classLoader
.getResource("code-samples/invalid-package-naming")!!.toURI()
Paths.get(uri).toString()
}

private val validPackageNamingDirectoryPath by lazy {
val uri = CheckMojoSpec::class.java.classLoader
.getResource("code-samples/valid")!!.toURI()
Paths.get(uri).toString()
}

fun create(block: CheckMojo.() -> Unit = {}): CheckMojo {
return CheckMojo().apply {
input = validPackageNamingDirectoryPath
mavenProject = createMockMavenProject()
block(this)
}
}

fun createWithInvalidPackageNamingStructure(block: CheckMojo.() -> Unit): CheckMojo {
return CheckMojo().apply {
input = invalidPackageNamingDirectoryPath
block(this)
}
}

fun createWithNoRuleExecution(block: CheckMojo.() -> Unit = {}): CheckMojo {
return create {
disableDefaultRuleSets = true
block(this)
}
}

private fun createMockMavenProject(): MavenProject {
return mock {
on {
compileClasspathElements
} doReturn listOf("/tmp/default", "/tmp/default2")
on {
getPlugin(any())
} doReturn (
Plugin().apply {
dependencies = mutableListOf(
Dependency().apply {
groupId = "a.b"
artifactId = "b"
version = "1"
}
)
}
)
}
}
}
1 change: 1 addition & 0 deletions src/test/resources/code-samples/valid/ValidFile.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package `code-samples`.`valid`
Copy link
Author

Choose a reason for hiding this comment

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

Created a valid input to execute a full testrun. Maybe this is not necessary - not sure.