completely remove checkstyle and replace it with a simple regex check

This is ~ 2 orders of magnitude faster on my machine, so it can be
executed as part of the compileJava task. Also, it actually logs the
failures, instead of making me go hunt for them in some generated
HTML-based report.
This commit is contained in:
Gavin King 2024-09-27 09:48:08 +02:00 committed by Steve Ebersole
parent 4f9035e9f8
commit 054aeff78b
12 changed files with 36 additions and 336 deletions

View File

@ -113,6 +113,5 @@ jobs:
name: test-reports-java11-${{ matrix.rdbms }} name: test-reports-java11-${{ matrix.rdbms }}
path: | path: |
./**/target/reports/tests/ ./**/target/reports/tests/
./**/target/reports/checkstyle/
- name: Omit produced artifacts from build cache - name: Omit produced artifacts from build cache
run: ./ci/before-cache.sh run: ./ci/before-cache.sh

View File

@ -121,6 +121,5 @@ jobs:
name: test-reports-java11-${{ matrix.rdbms }} name: test-reports-java11-${{ matrix.rdbms }}
path: | path: |
./**/target/reports/tests/ ./**/target/reports/tests/
./**/target/reports/checkstyle/
- name: Omit produced artifacts from build cache - name: Omit produced artifacts from build cache
run: ./ci/before-cache.sh run: ./ci/before-cache.sh

View File

@ -3,12 +3,12 @@
<option name="myName" value="Project Default" /> <option name="myName" value="Project Default" />
<inspection_tool class="BooleanMethodIsAlwaysInverted" enabled="false" level="WARNING" enabled_by_default="false" /> <inspection_tool class="BooleanMethodIsAlwaysInverted" enabled="false" level="WARNING" enabled_by_default="false" />
<inspection_tool class="DeprecatedIsStillUsed" enabled="true" level="WEAK WARNING" enabled_by_default="true" editorAttributes="INFO_ATTRIBUTES" /> <inspection_tool class="DeprecatedIsStillUsed" enabled="true" level="WEAK WARNING" enabled_by_default="true" editorAttributes="INFO_ATTRIBUTES" />
<inspection_tool class="MarkedForRemoval" enabled="true" level="WEAK WARNING" enabled_by_default="true" editorAttributes="INFO_ATTRIBUTES" />
<inspection_tool class="JUnitMalformedDeclaration" enabled="false" level="ERROR" enabled_by_default="false" /> <inspection_tool class="JUnitMalformedDeclaration" enabled="false" level="ERROR" enabled_by_default="false" />
<inspection_tool class="StringBufferReplaceableByString" enabled="false" level="WARNING" enabled_by_default="false" />
<inspection_tool class="JavadocBlankLines" enabled="true" level="WEAK WARNING" enabled_by_default="true" editorAttributes="INFO_ATTRIBUTES" /> <inspection_tool class="JavadocBlankLines" enabled="true" level="WEAK WARNING" enabled_by_default="true" editorAttributes="INFO_ATTRIBUTES" />
<inspection_tool class="JavadocDeclaration" enabled="true" level="WARNING" enabled_by_default="true"> <inspection_tool class="JavadocDeclaration" enabled="true" level="WARNING" enabled_by_default="true">
<option name="ADDITIONAL_TAGS" value="todo,settingDefault,remove" /> <option name="ADDITIONAL_TAGS" value="todo,settingDefault,remove" />
</inspection_tool> </inspection_tool>
<inspection_tool class="MarkedForRemoval" enabled="true" level="WEAK WARNING" enabled_by_default="true" editorAttributes="INFO_ATTRIBUTES" />
<inspection_tool class="StringBufferReplaceableByString" enabled="false" level="WARNING" enabled_by_default="false" />
</profile> </profile>
</component> </component>

View File

@ -83,7 +83,6 @@ Do your thing!
up the related commits and display them on the JIRA issue up the related commits and display them on the JIRA issue
* Make sure you have added the necessary tests for your changes * Make sure you have added the necessary tests for your changes
* Run _all_ the tests to ensure nothing else was accidentally broken * Run _all_ the tests to ensure nothing else was accidentally broken
* Make sure your source does not violate the _checkstyles_
_Before committing, if you want to pull in the latest upstream changes (highly _Before committing, if you want to pull in the latest upstream changes (highly
appreciated btw), please use rebasing rather than merging. Merging creates appreciated btw), please use rebasing rather than merging. Merging creates

View File

@ -76,11 +76,6 @@ elif [ "$RDBMS" == "informix" ]; then
goal="-Pdb=informix" goal="-Pdb=informix"
fi fi
# Disable checkstyle
#if [ -n "$goal" ]; then
goal="$goal -x checkstyleMain -DPOPULATE_REMOTE=true"
#fi
function logAndExec() { function logAndExec() {
echo 1>&2 "Executing:" "${@}" echo 1>&2 "Executing:" "${@}"
exec "${@}" exec "${@}"

View File

@ -36,7 +36,6 @@ apply plugin: 'de.thetaphi.forbiddenapis'
apply plugin: 'com.diffplug.spotless' apply plugin: 'com.diffplug.spotless'
apply plugin: "jacoco" apply plugin: "jacoco"
apply plugin: 'checkstyle'
apply plugin: 'build-dashboard' apply plugin: 'build-dashboard'
apply plugin: 'project-report' apply plugin: 'project-report'
@ -448,26 +447,37 @@ tasks.copyResourcesToIntelliJOutFolder.mustRunAfter processTestResources
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// Report configs // Report configs
checkstyle { task enforceRules {
it.sourceSets = [ project.sourceSets.main ] doLast {
configFile = rootProject.file( 'shared/config/checkstyle/checkstyle.xml' ) def illegalImport = ~/^import (sun|java.awt|org.slf4j)/
showViolations = false def missingNewline = ~/^\s*}\s*(else|catch|finally)/
def lowerEll = ~/\b\d+l\b/
def errors = 0
def tree = fileTree("src/main/java/")
tree.include "**/*.java"
tree.each { file ->
def lineNum = 0
def shortName = file.path.substring(rootDir.path.length())
file.eachLine { line ->
lineNum++
if (line =~ illegalImport) {
errors++
logger.error("Illegal import in ${shortName}\n${lineNum}: ${line}")
}
if (line =~ missingNewline) {
errors++
logger.error("Missing newline in ${shortName}\n${lineNum}: ${line}")
}
if (line =~ lowerEll) {
errors++
logger.error("Lowercase long literal in ${shortName}\n${lineNum}: ${line}")
}
}
}
if ( errors>0 ) {
throw new GradleException("Code rules were violated ($errors problems)")
}
} }
// exclude generated java sources - by explicitly setting the base source dir
tasks.checkstyleMain.source = 'src/main/java'
tasks.checkstyleMain
.exclude('org/hibernate/processor/util/NullnessUtil.java')
.exclude('org/hibernate/internal/util/NullnessUtil.java')
// For some big files we need more heap memory than the default 512m for parsing
tasks.checkstyleMain.maxHeapSize = "640m"
// define a second checkstyle task for checking non-fatal violations
task nonFatalCheckstyle(type:Checkstyle) {
source = project.sourceSets.main.java
classpath = project.configurations.checkstyle
showViolations = false
configFile = rootProject.file( 'shared/config/checkstyle/checkstyle-non-fatal.xml' )
} }
spotless { spotless {
@ -483,6 +493,8 @@ spotless {
} }
} }
tasks.spotlessApply.dependsOn enforceRules
class CompilerStubsArgumentProvider implements CommandLineArgumentProvider { class CompilerStubsArgumentProvider implements CommandLineArgumentProvider {

View File

@ -14,9 +14,6 @@ apply from: rootProject.file( 'gradle/java-module.gradle' )
java.modularity.inferModulePath = true java.modularity.inferModulePath = true
// Checkstyle fails for module-info
checkstyleMain.exclude '**/module-info.java'
dependencies { dependencies {
api project( ':hibernate-core' ) api project( ':hibernate-core' )
api project( ':hibernate-envers' ) api project( ':hibernate-envers' )

View File

@ -49,6 +49,3 @@ dependencies {
annotationProcessor project( ':hibernate-processor' ) annotationProcessor project( ':hibernate-processor' )
} }
tasks.checkstyleMain {
exclude '**/org/hibernate/orm/test/legacy/**'
}

View File

@ -15,7 +15,7 @@ First, a list of resources you will need access to in order to perform a release
== Steps == Steps
1. Perform `./gradlew preVerifyRelease` locally (after pulling all upstream changes). The Jenkins job does only the release steps, and we need to make sure tests and checkstyle especially are ok 1. Perform `./gradlew preVerifyRelease` locally (after pulling all upstream changes). The Jenkins job does only the release steps, and we need to make sure tests are ok
2. Mark the version as released in Jira 2. Mark the version as released in Jira
3. Close all issues associated with the version as closed. Be sure to remove the version from any issues that are not resolved (e.g. rejected) - the Jira "release notes" mechanism includes all issues with that version as the fix-for regardless of the resolution 3. Close all issues associated with the version as closed. Be sure to remove the version from any issues that are not resolved (e.g. rejected) - the Jira "release notes" mechanism includes all issues with that version as the fix-for regardless of the resolution
4. Start the https://ci.hibernate.org/view/Release/job/hibernate-orm-release-jdk17/[Jenkins job]. It is a parameterized build - Jenkins will prompt user for needed information: 4. Start the https://ci.hibernate.org/view/Release/job/hibernate-orm-release-jdk17/[Jenkins job]. It is a parameterized build - Jenkins will prompt user for needed information:

View File

@ -1,186 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Hibernate, Relational Persistence for Idiomatic Java
~
~ License: GNU Lesser General Public License (LGPL), version 2.1 or later
~ See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
-->
<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.1//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
<module name="Checker">
<module name="TreeWalker">
<module name="AvoidStarImport">
<property name="severity" value="warning" />
</module>
<module name="RedundantImport">
<property name="severity" value="warning" />
</module>
<module name="UnusedImports">
<property name="severity" value="warning" />
</module>
<module name="AvoidNestedBlocks">
<property name="allowInSwitchCase" value="true" />
<property name="severity" value="warning" />
</module>
<module name="HideUtilityClassConstructor">
<property name="severity" value="warning" />
</module>
<module name="MutableException">
<property name="severity" value="warning" />
</module>
<module name="EmptyStatement">
<property name="severity" value="warning" />
</module>
<module name="MissingSwitchDefault">
<property name="severity" value="warning" />
</module>
<module name="DefaultComesLast">
<property name="severity" value="warning" />
</module>
<module name="ModifiedControlVariable">
<property name="severity" value="warning" />
</module>
<module name="SimplifyBooleanExpression">
<property name="severity" value="warning" />
</module>
<module name="SimplifyBooleanReturn">
<property name="severity" value="warning" />
</module>
<module name="ExplicitInitialization">
<property name="severity" value="warning" />
</module>
<module name="FallThrough">
<property name="severity" value="warning" />
</module>
<module name="ArrayTypeStyle">
<property name="severity" value="warning" />
</module>
<module name="TrailingComment">
<property name="severity" value="warning" />
</module>
<module name="ModifierOrder">
<property name="severity" value="warning" />
</module>
<module name="AbstractClassName">
<!-- we are just using this to make sure that classes matching the pattern (Abstract*) have the abstract modifier -->
<property name="format" value="^Abstract.*$" />
<property name="ignoreName" value="true" />
<property name="severity" value="warning" />
</module>
<module name="ClassTypeParameterName">
<property name="format" value="^[A-Z][A-Z0-9]*$" />
<property name="severity" value="warning" />
</module>
<module name="ConstantName">
<property name="format" value="^[A-Z](_?[A-Z0-9]+)*$|log" />
<property name="severity" value="warning" />
</module>
<module name="LocalFinalVariableName">
<property name="severity" value="warning" />
</module>
<module name="LocalVariableName">
<property name="severity" value="warning" />
</module>
<module name="MemberName">
<property name="severity" value="warning" />
</module>
<module name="MethodTypeParameterName">
<property name="format" value="^[A-Z][A-Z0-9]*$" />
<property name="severity" value="warning" />
</module>
<module name="PackageName">
<property name="severity" value="warning" />
</module>
<module name="ParameterName">
<property name="severity" value="warning" />
</module>
<module name="StaticVariableName">
<property name="severity" value="warning" />
</module>
<module name="TypeName">
<property name="severity" value="warning" />
</module>
<module name="AbbreviationAsWordInName">
<property name="severity" value="warning" />
</module>
<module name="MethodParamPad">
<property name="severity" value="warning" />
</module>
<module name="TypecastParenPad">
<property name="severity" value="warning" />
</module>
<module name="ParenPad">
<property name="tokens" value="CTOR_CALL, METHOD_CALL, SUPER_CTOR_CALL" />
<property name="option" value="space" />
<property name="severity" value="warning" />
</module>
<!--
Source code comment-based suppressions
-->
<module name="SuppressionCommentFilter">
<!--
Allow a finalize() method within these comments. DriverManagerConnectionProviderImpl e.g.
uses a finalizer to make sure we release all of its cached connections.
-->
<property name="offCommentFormat" value="CHECKSTYLE:START_ALLOW_FINALIZER"/>
<property name="onCommentFormat" value="CHECKSTYLE:END_ALLOW_FINALIZER"/>
<property name="checkFormat" value="NoFinalizer"/>
</module>
<module name="SuppressWithNearbyCommentFilter">
<property name="commentFormat" value="noinspection StatementWithEmptyBody"/>
<property name="checkFormat" value="EmptyStatement"/>
<property name="influenceFormat" value="1"/>
</module>
</module>
<module name="JavadocPackage">
<property name="allowLegacy" value="true" />
<property name="severity" value="warning" />
</module>
<!--
Used to collect "todo" comments into a single location
-->
<module name="TreeWalker">
<module name="TodoComment">
<property name="format" value="[Tt][Oo][Dd][Oo]"/>
<property name="severity" value="info" />
</module>
</module>
</module>

View File

@ -1,104 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Hibernate, Relational Persistence for Idiomatic Java
~
~ License: GNU Lesser General Public License (LGPL), version 2.1 or later.
~ See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
-->
<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.1//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
<module name="Checker">
<!-- Only defines "fatal" violations; there are additional "non-fatal" rules in checkstyle-non-fatal.xml -->
<module name="RegexpHeader">
<property name="header" value=""/>
<property name="fileExtensions" value="\/\*\n * SPDX-License-Identifier: LGPL-2.1-or-later\n \* Copyright Red Hat Inc. and Hibernate Authors\n \*\/java"/>
</module>
<module name="RegexpHeader">
<property name="header" value="&lt;\?xml version\=&quot;1.0&quot; encoding\=&quot;UTF-8&quot;\?&gt;\n&lt;!--\n ~ Hibernate, Relational Persistence for Idiomatic Java\n ~\n ~ License: GNU Lesser General Public License \(LGPL\), version 2\.1 or later[\.]?\n ~ See the lgpl.txt file in the root directory or [&lt;]?http:\/\/www\.gnu\.org\/licenses\/lgpl-2\.1\.html[&gt;]?[.]?\n --&gt;"/>
<property name="fileExtensions" value="xml,xsd"/>
</module>
<module name="TreeWalker">
<!--
High-priority warnings : fail the build...
-->
<module name="RegexpSinglelineJava">
<property name="ignoreComments" value="true" />
<property name="format" value="^\t* +\t*\S" />
<property name="message" value="Line has leading space characters; indentation should be performed with tabs only." />
</module>
<module name="Regexp">
<property name="format" value="/^(master|slave)$/"/>
<property name="illegalPattern" value="true"/>
<property name="message" value="Conscious Language (use of terms 'master' or 'slave' not allowed)"/>
</module>
<module name="MissingDeprecated" />
<module name="MissingOverride" />
<module name="PackageAnnotation" />
<module name="NeedBraces" />
<module name="LeftCurly">
<property name="option" value="eol" />
</module>
<module name="RightCurly">
<property name="option" value="alone" />
</module>
<module name="EqualsHashCode" />
<module name="StringLiteralEquality" />
<module name="NoFinalizer" />
<module name="OneStatementPerLine" />
<module name="UpperEll" />
<module name="IllegalImport">
<property name="illegalPkgs" value="java.awt, sun, org.slf4j"/>
</module>
<!--
Source code comment-based suppressions
-->
<module name="SuppressionCommentFilter">
<!--
Allow a finalize() method within these comments. DriverManagerConnectionProviderImpl e.g.
uses a finalizer to make sure we release all of its cached connections.
-->
<property name="offCommentFormat" value="CHECKSTYLE:START_ALLOW_FINALIZER"/>
<property name="onCommentFormat" value="CHECKSTYLE:END_ALLOW_FINALIZER"/>
<property name="checkFormat" value="NoFinalizer"/>
</module>
<module name="SuppressWithNearbyCommentFilter">
<property name="commentFormat" value="noinspection StatementWithEmptyBody"/>
<property name="checkFormat" value="EmptyStatement"/>
<property name="influenceFormat" value="1"/>
</module>
</module>
<!-- We are not using NewLineAtEndOfFile because the new line chars change
on different operating systems and that rule allows only one type. This rule
is not actually checking for new lines, but it will work if we check that
there are not white spaces at the end of a line with another rule. -->
<module name="RegexpMultiline">
<property name="format" value="\S\z" />
<property name="message" value="Missing new line at the end of file" />
</module>
<module name="RegexpMultiline">
<property name="format" value="\S(r?\n){5,}\z" />
<property name="message" value="Files should end with no more than 5 (empty) new lines" />
</module>
</module>

View File

@ -11,8 +11,6 @@ plugins {
id 'java-gradle-plugin' id 'java-gradle-plugin'
id 'com.gradle.plugin-publish' version '1.2.1' id 'com.gradle.plugin-publish' version '1.2.1'
id 'checkstyle'
// for local publishing // for local publishing
id 'maven-publish' id 'maven-publish'
} }
@ -131,12 +129,6 @@ tasks.named( "javadoc", Javadoc ) {
} }
} }
checkstyle {
sourceSets = [ project.sourceSets.main ]
configFile = rootProject.file( 'shared/config/checkstyle/checkstyle.xml' )
showViolations = false
}
tasks.publish.enabled !project.ormVersion.isSnapshot tasks.publish.enabled !project.ormVersion.isSnapshot
tasks.publishPlugins.enabled !project.ormVersion.isSnapshot tasks.publishPlugins.enabled !project.ormVersion.isSnapshot