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

Apply Checkstyle, SpotBugs, Jacoco plugins #220

Merged
merged 4 commits into from
Mar 6, 2023
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
2 changes: 2 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ repositories {
maven {
url = uri("https://repo.jenkins-ci.org/public")
}
gradlePluginPortal()
}

java {
Expand All @@ -46,6 +47,7 @@ dependencies {
isTransitive = false
}
compileOnly("org.jvnet.localizer:maven-localizer-plugin:1.24")
implementation("com.github.spotbugs.snom:spotbugs-gradle-plugin:5.0.13")
implementation(sezpoz)
implementation(localGroovy())
testAnnotationProcessor(sezpoz)
Expand Down
8 changes: 8 additions & 0 deletions gradle/verification-metadata.xml
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,14 @@
<sha256 value="f96ca81b6146cc83a6e17e9152c33d58c873f0472b4c284da1515dd713855667" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="com.github.spotbugs.snom" name="spotbugs-gradle-plugin" version="5.0.13">
<artifact name="spotbugs-gradle-plugin-5.0.13.jar">
<sha256 value="647526529134f4be34161d80ddd7b501a9e084fb43ed5f3f1181a2ff3baac707" origin="Generated by Gradle"/>
</artifact>
<artifact name="spotbugs-gradle-plugin-5.0.13.module">
<sha256 value="d69bc95ca3021fff82c3945b3c9245694e7edcdd6a59d5dfeb8cab93ad2774ee" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="com.google" name="google" version="5">
<artifact name="google-5.pom">
<sha256 value="e09d345e73ca3fbca7f3e05f30deb74e9d39dd6b79a93fee8c511f23417b6828" origin="Generated by Gradle"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class JpiExtension implements JpiExtensionBridge {
private final SetProperty<String> maskedClassesFromCore
private final ListProperty<PluginDeveloper> pluginDevelopers
private final Property<String> incrementalsRepoUrl
private final Property<Boolean> checkstyleEnabled
private final Property<Boolean> jacocoEnabled
private final Property<Boolean> spotBugsEnabled

@SuppressWarnings('UnnecessarySetter')
JpiExtension(Project project) {
Expand All @@ -95,6 +98,9 @@ class JpiExtension implements JpiExtensionBridge {
this.requireEscapeByDefaultInJelly = project.objects.property(Boolean).convention(true)
this.generatedTestClassName = project.objects.property(String).convention('InjectedTest')
this.incrementalsRepoUrl = project.objects.property(String).convention(JENKINS_INCREMENTALS_REPO)
this.checkstyleEnabled = project.objects.property(Boolean).convention(false)
this.jacocoEnabled = project.objects.property(Boolean).convention(false)
this.spotBugsEnabled = project.objects.property(Boolean).convention(false)
Copy link

Choose a reason for hiding this comment

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

Do you think it makes sense to have a higher level property to allow opting into all the OSS recommendations? It's really nice to have this level of granularity, but I'm wondering if folks will basically want two modes most of the time - one for internal plugins, and one for oss plugins.

Copy link
Author

@alextu alextu Mar 6, 2023

Choose a reason for hiding this comment

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

That's a good question, how about having one single flag then configureQualityChecks (false by default) that OSS plugins devs would set to true. Internal plugin devs would apply and configure quality plugins "manually" as for any Gradle projects.
The only limitation I see is that it would require conditionnal applying plugins in afterEvaluate. That will not play well with configureQualityChecks = true and customizing a plugin with an extension, since the plugin is not applied yet when Gradle evaluates the extension. In that case the user would have to fallback to configureQualityChecks = false and configure manually as for internal plugins.
What do you think ?

Copy link

Choose a reason for hiding this comment

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

That's a really good point. Maybe it eventually makes sense to split into two plugins (oss, internal) for the different use-cases, but this is a good starting point and we can see where it evolves to. Happy to move forward with this.

}

/**
Expand Down Expand Up @@ -475,6 +481,18 @@ class JpiExtension implements JpiExtensionBridge {
incrementalsRepoUrl
}

Property<Boolean> getCheckstyleEnabled() {
checkstyleEnabled
}

Property<Boolean> getJacocoEnabled() {
jacocoEnabled
}

Property<Boolean> getSpotBugsEnabled() {
spotBugsEnabled
}

/**
* @see PluginDeveloper
* @deprecated To be removed in 1.0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package org.jenkinsci.gradle.plugins.jpi

import com.github.spotbugs.snom.SpotBugsPlugin
import com.github.spotbugs.snom.SpotBugsTask
import org.gradle.api.Action
import org.gradle.api.GradleException
import org.gradle.api.NamedDomainObjectProvider
Expand All @@ -39,6 +41,9 @@ import org.gradle.api.plugins.JavaLibraryPlugin
import org.gradle.api.plugins.JavaPlugin
import org.gradle.api.plugins.JavaPluginConvention
import org.gradle.api.plugins.JavaPluginExtension
import org.gradle.api.plugins.quality.Checkstyle
import org.gradle.api.plugins.quality.CheckstyleExtension
import org.gradle.api.plugins.quality.CheckstylePlugin
import org.gradle.api.publish.PublishingExtension
import org.gradle.api.publish.maven.MavenPublication
import org.gradle.api.publish.maven.plugins.MavenPublishPlugin
Expand All @@ -49,6 +54,8 @@ import org.gradle.api.tasks.compile.GroovyCompile
import org.gradle.api.tasks.compile.JavaCompile
import org.gradle.api.tasks.testing.Test
import org.gradle.language.base.plugins.LifecycleBasePlugin
import org.gradle.testing.jacoco.plugins.JacocoPlugin
import org.gradle.testing.jacoco.tasks.JacocoReport
import org.gradle.util.GradleVersion
import org.jenkinsci.gradle.plugins.jpi.internal.DependenciesPlugin
import org.jenkinsci.gradle.plugins.jpi.internal.PluginDependencyProvider
Expand All @@ -59,7 +66,6 @@ import org.jenkinsci.gradle.plugins.jpi.server.GenerateHplTask
import org.jenkinsci.gradle.plugins.jpi.server.InstallJenkinsServerPluginsTask
import org.jenkinsci.gradle.plugins.jpi.server.JenkinsServerTask
import org.jenkinsci.gradle.plugins.jpi.verification.CheckOverlappingSourcesTask

import java.util.concurrent.Callable

import static org.gradle.api.logging.LogLevel.INFO
Expand Down Expand Up @@ -123,6 +129,8 @@ class JpiPlugin implements Plugin<Project>, PluginDependencyProvider {

gradleProject.plugins.apply(JavaLibraryPlugin)
gradleProject.plugins.apply(GroovyPlugin)
gradleProject.plugins.apply(JacocoPlugin)
gradleProject.plugins.apply(CheckstylePlugin)
gradleProject.plugins.apply(kotlinPlugin('org.jenkinsci.gradle.plugins.accmod.AccessModifierPlugin'))
gradleProject.plugins.apply(kotlinPlugin('org.jenkinsci.gradle.plugins.manifest.JenkinsManifestPlugin'))
gradleProject.plugins.apply(kotlinPlugin('org.jenkinsci.gradle.plugins.testing.JpiTestingPlugin'))
Expand Down Expand Up @@ -236,6 +244,9 @@ class JpiPlugin implements Plugin<Project>, PluginDependencyProvider {
configureTestDependencies(gradleProject)
configurePublishing(gradleProject)
configureTestHpl(gradleProject)
configureCheckstyle(gradleProject)
configureJacoco(gradleProject)
configureSpotbugs(gradleProject)
gradleProject.afterEvaluate {
gradleProject.setProperty('archivesBaseName', ext.shortName)
}
Expand Down Expand Up @@ -578,6 +589,51 @@ class JpiPlugin implements Plugin<Project>, PluginDependencyProvider {
}
}

private configureCheckstyle(Project project) {
def checkstyle = project.extensions.getByType(CheckstyleExtension)
checkstyle.config = project.resources.text.fromUri(JpiPlugin.getResource('/sun_checks.xml').toURI())
project.tasks.withType(Checkstyle).configureEach {
it.reports {
xml.required = true
html.required = false
}
it.onlyIf {
project.extensions.getByType(JpiExtension).checkstyleEnabled.get()
}
}
}

private configureJacoco(Project project) {
project.tasks.withType(JacocoReport).configureEach {
it.reports {
xml.required = true
html.required = false
}
it.onlyIf {
project.extensions.getByType(JpiExtension).jacocoEnabled.get()
}
}
project.tasks.withType(Test).configureEach {
it.finalizedBy(project.tasks.named('jacocoTestReport'))
}
}

private configureSpotbugs(Project project) {
if (GradleVersion.current() < GradleVersion.version('7.0')) {
return
}
project.plugins.apply(SpotBugsPlugin)
project.tasks.withType(SpotBugsTask).configureEach {
it.reports {
xml.required = true
html.required = false
}
it.onlyIf {
project.extensions.getByType(JpiExtension).spotBugsEnabled.get()
}
}
}

@Override
String pluginDependencies() {
dependencyAnalysis.analyse().manifestPluginDependencies
Expand Down
198 changes: 198 additions & 0 deletions src/main/resources/sun_checks.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">

<!--

Checkstyle configuration that checks the sun coding conventions from:

- the Java Language Specification at
https://docs.oracle.com/javase/specs/jls/se11/html/index.html

- the Sun Code Conventions at https://www.oracle.com/java/technologies/javase/codeconventions-contents.html

- the Javadoc guidelines at
https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html

- the JDK Api documentation https://docs.oracle.com/en/java/javase/11/

- some best practices

Checkstyle is very configurable. Be sure to read the documentation at
https://checkstyle.org (or in your downloaded distribution).

Most Checks are configurable, be sure to consult the documentation.

To completely disable a check, just comment it out or delete it from the file.
To suppress certain violations please review suppression filters.

Finally, it is worth reading the documentation.

-->

<module name="Checker">
<!--
If you set the basedir property below, then all reported file
names will be relative to the specified directory. See
https://checkstyle.org/config.html#Checker

<property name="basedir" value="${basedir}"/>
-->
<property name="severity" value="error"/>

<property name="fileExtensions" value="java, properties, xml"/>

<!-- Excludes all 'module-info.java' files -->
<!-- See https://checkstyle.org/config_filefilters.html -->
<module name="BeforeExecutionExclusionFileFilter">
<property name="fileNamePattern" value="module\-info\.java$"/>
</module>

<!-- https://checkstyle.org/config_filters.html#SuppressionFilter -->
<module name="SuppressionFilter">
<property name="file" value="${org.checkstyle.sun.suppressionfilter.config}"
default="checkstyle-suppressions.xml" />
<property name="optional" value="true"/>
</module>

<!-- Checks that a package-info.java file exists for each package. -->
<!-- See https://checkstyle.org/config_javadoc.html#JavadocPackage -->
<module name="JavadocPackage"/>

<!-- Checks whether files end with a new line. -->
<!-- See https://checkstyle.org/config_misc.html#NewlineAtEndOfFile -->
<module name="NewlineAtEndOfFile"/>

<!-- Checks that property files contain the same keys. -->
<!-- See https://checkstyle.org/config_misc.html#Translation -->
<module name="Translation"/>

<!-- Checks for Size Violations. -->
<!-- See https://checkstyle.org/config_sizes.html -->
<module name="FileLength"/>
<module name="LineLength">
<property name="fileExtensions" value="java"/>
</module>

<!-- Checks for whitespace -->
<!-- See https://checkstyle.org/config_whitespace.html -->
<module name="FileTabCharacter"/>

<!-- Miscellaneous other checks. -->
<!-- See https://checkstyle.org/config_misc.html -->
<module name="RegexpSingleline">
<property name="format" value="\s+$"/>
<property name="minimum" value="0"/>
<property name="maximum" value="0"/>
<property name="message" value="Line has trailing spaces."/>
</module>

<!-- Checks for Headers -->
<!-- See https://checkstyle.org/config_header.html -->
<!-- <module name="Header"> -->
<!-- <property name="headerFile" value="${checkstyle.header.file}"/> -->
<!-- <property name="fileExtensions" value="java"/> -->
<!-- </module> -->

<module name="TreeWalker">

<!-- Checks for Javadoc comments. -->
<!-- See https://checkstyle.org/config_javadoc.html -->
<module name="InvalidJavadocPosition"/>
<module name="JavadocMethod"/>
<module name="JavadocType"/>
<module name="JavadocVariable"/>
<module name="JavadocStyle"/>
<module name="MissingJavadocMethod"/>

<!-- Checks for Naming Conventions. -->
<!-- See https://checkstyle.org/config_naming.html -->
<module name="ConstantName"/>
<module name="LocalFinalVariableName"/>
<module name="LocalVariableName"/>
<module name="MemberName"/>
<module name="MethodName"/>
<module name="PackageName"/>
<module name="ParameterName"/>
<module name="StaticVariableName"/>
<module name="TypeName"/>

<!-- Checks for imports -->
<!-- See https://checkstyle.org/config_imports.html -->
<module name="AvoidStarImport"/>
<module name="IllegalImport"/> <!-- defaults to sun.* packages -->
<module name="RedundantImport"/>
<module name="UnusedImports">
<property name="processJavadoc" value="false"/>
</module>

<!-- Checks for Size Violations. -->
<!-- See https://checkstyle.org/config_sizes.html -->
<module name="MethodLength"/>
<module name="ParameterNumber"/>

<!-- Checks for whitespace -->
<!-- See https://checkstyle.org/config_whitespace.html -->
<module name="EmptyForIteratorPad"/>
<module name="GenericWhitespace"/>
<module name="MethodParamPad"/>
<module name="NoWhitespaceAfter"/>
<module name="NoWhitespaceBefore"/>
<module name="OperatorWrap"/>
<module name="ParenPad"/>
<module name="TypecastParenPad"/>
<module name="WhitespaceAfter"/>
<module name="WhitespaceAround"/>

<!-- Modifier Checks -->
<!-- See https://checkstyle.org/config_modifier.html -->
<module name="ModifierOrder"/>
<module name="RedundantModifier"/>

<!-- Checks for blocks. You know, those {}'s -->
<!-- See https://checkstyle.org/config_blocks.html -->
<module name="AvoidNestedBlocks"/>
<module name="EmptyBlock"/>
<module name="LeftCurly"/>
<module name="NeedBraces"/>
<module name="RightCurly"/>

<!-- Checks for common coding problems -->
<!-- See https://checkstyle.org/config_coding.html -->
<module name="EmptyStatement"/>
<module name="EqualsHashCode"/>
<module name="HiddenField"/>
<module name="IllegalInstantiation"/>
<module name="InnerAssignment"/>
<module name="MagicNumber"/>
<module name="MissingSwitchDefault"/>
<module name="MultipleVariableDeclarations"/>
<module name="SimplifyBooleanExpression"/>
<module name="SimplifyBooleanReturn"/>

<!-- Checks for class design -->
<!-- See https://checkstyle.org/config_design.html -->
<module name="DesignForExtension"/>
<module name="FinalClass"/>
<module name="HideUtilityClassConstructor"/>
<module name="InterfaceIsType"/>
<module name="VisibilityModifier"/>

<!-- Miscellaneous other checks. -->
<!-- See https://checkstyle.org/config_misc.html -->
<module name="ArrayTypeStyle"/>
<module name="FinalParameters"/>
<module name="TodoComment"/>
<module name="UpperEll"/>

<!-- https://checkstyle.org/config_filters.html#SuppressionXpathFilter -->
<module name="SuppressionXpathFilter">
<property name="file" value="${org.checkstyle.sun.suppressionxpathfilter.config}"
default="checkstyle-xpath-suppressions.xml" />
<property name="optional" value="true"/>
</module>

</module>

</module>
Loading