IGNITE-15370 [Ignite 3] Add style check to the build lifecycle (#303)
diff --git a/DEVNOTES.md b/DEVNOTES.md
index cc02fd3..197e956 100644
--- a/DEVNOTES.md
+++ b/DEVNOTES.md
@@ -30,9 +30,16 @@
* [Checkstyle suppressions](check-rules/checkstyle-suppressions.xml)
* [Checkstyle rules for javadocs](https://checkstyle.sourceforge.io/config_javadoc.html)
+It is enabled by default and is bound to `compile` phase.
+
+Build project without code style check:
+```
+mvn clean <compile|package|install|deploy> -Dcheckstyle.skip
+```
+
Run code style checks only:
```
-mvn clean checkstyle:checkstyle-aggregate
+mvn clean validate -Pcheckstyle -Dmaven.all-checks.skip
```
Run javadoc style checks for public api only:
@@ -41,7 +48,9 @@
```
>ℹ `javadoc-public-api` profile is required for enabling checkstyle rules for public API javadoc.
-Code style check result is generated at `target/site/checkstyle-aggregate.html`
+Code style check results are generated at:
+* `target/site/checkstyle-aggregate.html`
+* `target/checkstyle.xml`
### License headers
Project files license headers match with required template is checked with [Apache Rat Maven Plugin](https://creadur.apache.org/rat/apache-rat-plugin/).
diff --git a/check-rules/checkstyle-suppressions.xml b/check-rules/checkstyle-suppressions.xml
index 3273169..dbaf1af 100644
--- a/check-rules/checkstyle-suppressions.xml
+++ b/check-rules/checkstyle-suppressions.xml
@@ -21,4 +21,5 @@
"-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
"https://checkstyle.org/dtds/suppressions_1_2.dtd">
<suppressions>
+ <suppress files="[/\\]target[/\\]" checks=".*" />
</suppressions>
diff --git a/modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java b/modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
index c67f78a..2868895 100644
--- a/modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
+++ b/modules/metastorage-client/src/integrationTest/java/org/apache/ignite/internal/metastorage/client/ITMetaStorageServiceTest.java
@@ -1038,7 +1038,7 @@
200
).get(3, TimeUnit.SECONDS);
- MetaStorageService metaStorageSvc2 = new MetaStorageServiceImpl(metaStorageRaftGrpSvc, NODE_ID_1);
+ MetaStorageService metaStorageSvc2 = new MetaStorageServiceImpl(metaStorageRaftGrpSvc, NODE_ID_1);
Cursor<Entry> cursorNode0 = metaStorageSvc.range(EXPECTED_RESULT_ENTRY.key(), null);
diff --git a/modules/network-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/network/processor/ITTransferableObjectProcessorTest.java b/modules/network-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/network/processor/ITTransferableObjectProcessorTest.java
index 54f37ad..f9dd86b 100644
--- a/modules/network-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/network/processor/ITTransferableObjectProcessorTest.java
+++ b/modules/network-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/network/processor/ITTransferableObjectProcessorTest.java
@@ -25,8 +25,8 @@
import com.google.testing.compile.Compiler;
import com.google.testing.compile.JavaFileObjects;
import org.apache.ignite.network.NetworkMessage;
-import org.apache.ignite.network.annotations.Transferable;
import org.apache.ignite.network.annotations.MessageGroup;
+import org.apache.ignite.network.annotations.Transferable;
import org.junit.jupiter.api.Test;
import static com.google.testing.compile.CompilationSubject.assertThat;
diff --git a/modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/ITJRaftCounterServerTest.java b/modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/ITJRaftCounterServerTest.java
index fb82666..1d1631e 100644
--- a/modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/ITJRaftCounterServerTest.java
+++ b/modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/ITJRaftCounterServerTest.java
@@ -140,7 +140,7 @@
Iterator<RaftGroupService> iterClients = clients.iterator();
while (iterClients.hasNext()) {
- RaftGroupService client = iterClients.next();
+ RaftGroupService client = iterClients.next();
iterClients.remove();
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/LiveSchemaChangeKVViewTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/LiveSchemaChangeKVViewTest.java
index 790fa6d..034a695 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/LiveSchemaChangeKVViewTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/LiveSchemaChangeKVViewTest.java
@@ -101,7 +101,6 @@
assertEquals("111", res.value("valStrNew"));
assertEquals(Integer.valueOf(333), res.value("valIntNew"));
-
((TableImpl)tbl).schemaType(SchemaMode.STRICT_SCHEMA);
Tuple anotherKey = Tuple.create().set("key", 2L);
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/LiveSchemaChangeTableTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/LiveSchemaChangeTableTest.java
index bcf86ef..113efcd 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/LiveSchemaChangeTableTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/LiveSchemaChangeTableTest.java
@@ -158,7 +158,6 @@
assertEquals("111", res.value("valStrNew"));
assertEquals(Integer.valueOf(333), res.value("valIntNew"));
-
((TableImpl)tbl).schemaType(SchemaMode.STRICT_SCHEMA);
Tuple anotherKey = Tuple.create().set("key", 2L).set("valStrNew", "111").set("valIntNew", 333);
diff --git a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ITDistributedTableTest.java b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ITDistributedTableTest.java
index e7e1603..9cad5f9 100644
--- a/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ITDistributedTableTest.java
+++ b/modules/table/src/integrationTest/java/org/apache/ignite/distributed/ITDistributedTableTest.java
@@ -435,7 +435,6 @@
for (int i = 0; i < keysCnt; i++)
keys.add(Tuple.create().set("key", Long.valueOf(i)));
-
Map<Tuple, Tuple> entries = view.getAll(keys);
assertEquals(keysCnt, entries.size());
diff --git a/parent/pom.xml b/parent/pom.xml
index e309524..dd770fd 100644
--- a/parent/pom.xml
+++ b/parent/pom.xml
@@ -782,11 +782,7 @@
Profile to add generated sources to the list of paths for the PMD plugin.
-->
<profile>
- <activation>
- <property>
- <name>pmd.check-generated-sources</name>
- </property>
- </activation>
+ <id>pmd</id>
<build>
<plugins>
<plugin>
@@ -824,6 +820,53 @@
</plugins>
</build>
</profile>
+
+ <profile>
+ <id>error-prone</id>
+ <activation>
+ <property>
+ <name>!maven.all-checks.skip</name>
+ </property>
+ </activation>
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>${maven.compiler.plugin.version}</version>
+ <configuration>
+ <compilerArgs>
+ <arg>-XDcompilePolicy=simple</arg>
+ <arg>
+ -Xplugin:ErrorProne
+ -XepDisableWarningsInGeneratedCode
+ -XepExcludedPaths:(.*/src/integrationTest/.*|.*/src/test/.*|.*/modules/(raft|metastorage-server|schema|table)/.*)
+ </arg>
+ <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
+ <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
+ <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
+ <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
+ <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
+ <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
+ <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
+ <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
+ <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
+ <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
+ </compilerArgs>
+ <annotationProcessorPaths>
+ <path>
+ <groupId>com.google.errorprone</groupId>
+ <artifactId>error_prone_core</artifactId>
+ <version>${errorprone-annotation-processor.version}</version>
+ </path>
+ </annotationProcessorPaths>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ </build>
+ </profile>
</profiles>
<build>
@@ -831,39 +874,6 @@
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
- <artifactId>maven-compiler-plugin</artifactId>
- <version>${maven.compiler.plugin.version}</version>
- <configuration>
- <compilerArgs>
- <arg>-XDcompilePolicy=simple</arg>
- <arg>
- -Xplugin:ErrorProne
- -XepDisableWarningsInGeneratedCode
- -XepExcludedPaths:(.*/src/integrationTest/.*|.*/src/test/.*|.*/modules/(raft|metastorage-server|schema|table)/.*)
- </arg>
- <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
- <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
- <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
- <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
- <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
- <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
- <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
- <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
- <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
- <arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
- </compilerArgs>
- <annotationProcessorPaths>
- <path>
- <groupId>com.google.errorprone</groupId>
- <artifactId>error_prone_core</artifactId>
- <version>${errorprone-annotation-processor.version}</version>
- </path>
- </annotationProcessorPaths>
- </configuration>
- </plugin>
-
- <plugin>
- <groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${maven.surefire.plugin.version}</version>
</plugin>
@@ -990,7 +1000,7 @@
<executions>
<execution>
<id>add-test-source</id>
- <phase>process-resources</phase>
+ <phase>validate</phase>
<goals>
<goal>add-test-source</goal>
</goals>
@@ -1002,7 +1012,7 @@
</execution>
<execution>
<id>add-test-resource</id>
- <phase>process-resources</phase>
+ <phase>validate</phase>
<goals>
<goal>add-test-resource</goal>
</goals>
@@ -1086,20 +1096,6 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
- <configuration>
- <sourceDirectories>
- <sourceDirectory>${project.build.sourceDirectory}</sourceDirectory>
- <sourceDirectory>${project.build.testSourceDirectory}</sourceDirectory>
- </sourceDirectories>
- <consoleOutput>true</consoleOutput>
- <logViolationsToConsole>true</logViolationsToConsole>
- <failOnViolation>true</failOnViolation>
- <outputFile>${project.build.directory}/checkstyle.xml</outputFile>
- <configLocation>${project.basedir}/check-rules/checkstyle-rules.xml</configLocation>
- <suppressionsLocation>${project.basedir}/check-rules/checkstyle-suppressions.xml</suppressionsLocation>
- <includeTestSourceDirectory>true</includeTestSourceDirectory>
- <excludes>generated/**/*,com/facebook/presto/bytecode/**/*,org/apache/ignite/raft/jraft/**/*</excludes>
- </configuration>
<dependencies>
<dependency>
<groupId>com.puppycrawl.tools</groupId>
@@ -1107,6 +1103,17 @@
<version>${checkstyle.puppycrawl.version}</version>
</dependency>
</dependencies>
+ <configuration>
+ <outputFile>${project.build.directory}/checkstyle.xml</outputFile>
+ <configLocation>${project.basedir}/check-rules/checkstyle-rules.xml</configLocation>
+ <suppressionsLocation>${project.basedir}/check-rules/checkstyle-suppressions.xml</suppressionsLocation>
+ <includeTestSourceDirectory>true</includeTestSourceDirectory>
+ <excludes>com/facebook/presto/bytecode/**/*,org/apache/ignite/raft/jraft/**/*</excludes>
+ <linkXRef>false</linkXRef>
+
+ <includeTestSourceDirectory>true</includeTestSourceDirectory>
+ <testSourceDirectories>src/integrationTest/java</testSourceDirectories>
+ </configuration>
</plugin>
<!--
diff --git a/pom.xml b/pom.xml
index aa83c61..100f3cb 100644
--- a/pom.xml
+++ b/pom.xml
@@ -69,6 +69,55 @@
<module>modules/vault</module>
</modules>
+ <profiles>
+ <!--
+ This profile is used to check the code for coding guidelines.
+
+ It runs by default and can be skipped with either
+ -Dmaven.chechstyle.skip or -Dcheckstyle.skip flag.
+
+ It executes 2 times - first for generating report and second
+ for failing build if errors found.
+ -->
+ <profile>
+ <id>checkstyle</id>
+ <activation>
+ <property>
+ <name>!maven.all-checks.skip</name>
+ </property>
+ </activation>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-checkstyle-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>generate-report</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>checkstyle-aggregate</goal>
+ </goals>
+ </execution>
+ <execution>
+ <id>fail-if-error</id>
+ <phase>validate</phase>
+ <configuration>
+ <failsOnError>true</failsOnError>
+ <consoleOutput>true</consoleOutput>
+ <logViolationsToConsole>true</logViolationsToConsole>
+ </configuration>
+ <goals>
+ <goal>checkstyle-aggregate</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
+ </profiles>
+
<build>
<plugins>
<plugin>
