[CALCITE-4830] Remove remaining uses of ImmutableBeans and deprecate

- Supersede and deprecate two-level nested Config classes to ensure all immutables are hidden
  and carry consistene style.
- Remove the use of @value.Style in non-test code to ensure consistent styling.
- Add annotation processing to all modules implementing new RelRule.Config subclasses
- Deprecate public methods and interfaces of ImmutableBeans
- Deprecate RelRule.Config.EMPTY
- Update javadocs for ImmutableBeans, RelRule, RelRule.CONFIG
- Add annotation processing to the core/test module
- Add compilation dependency on jsr305 to work around an Immutables bug when compiling against old Guava versions.

Note that during the development of this patch, kapt was evaluated extensively and ultimately ruled
out for use. The reasons included:
- Kapt is not compatible with JDK17: https://youtrack.jetbrains.com/issue/KT-45545
- Kapt causes deadlocks: https://youtrack.jetbrains.com/issue/KT-47853

Workarounds to those issues could not be made reliable.

bom/build.gradle.kts

@@ -155,5 +155,6 @@ dependencies {
         apiv("org.openjdk.jmh:jmh-generator-annprocess", "jmh")
         runtimev("xalan:xalan")
         runtimev("xerces:xercesImpl")
+        apiv("com.google.code.findbugs:jsr305")
     }
 }

build.gradle.kts

@@ -492,7 +492,9 @@ allprojects {
         if (!skipAutostyle) {
             autostyle {
                 java {
-                    filter.exclude(*javaccGeneratedPatterns + "**/test/java/*.java")
+                    filter.exclude(*javaccGeneratedPatterns +
+                            "**/test/java/*.java" +
+                            "**/RelRule.java" /** remove as part of CALCITE-4831 **/)
                     license()
                     if (!project.props.bool("junit4", default = false)) {
                         replace("junit5: Test", "org.junit.Test", "org.junit.jupiter.api.Test")

cassandra/build.gradle.kts

@@ -14,6 +14,13 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import com.github.vlsi.gradle.ide.dsl.settings
+import com.github.vlsi.gradle.ide.dsl.taskTriggers
+
+plugins {
+    id("com.github.vlsi.ide")
+}
+
 dependencies {
     api(project(":core"))
     api(project(":linq4j"))
@@ -31,4 +38,41 @@ dependencies {
     }
     testImplementation("org.cassandraunit:cassandra-unit")
     testRuntimeOnly("net.java.dev.jna:jna")
+
+    annotationProcessor("org.immutables:value")
+    compileOnly("org.immutables:value-annotations")
+    compileOnly("com.google.code.findbugs:jsr305")
+}
+
+fun JavaCompile.configureAnnotationSet(sourceSet: SourceSet) {
+    source = sourceSet.java
+    classpath = sourceSet.compileClasspath
+    options.compilerArgs.add("-proc:only")
+    org.gradle.api.plugins.internal.JvmPluginsHelper.configureAnnotationProcessorPath(sourceSet, sourceSet.java, options, project)
+    destinationDirectory.set(temporaryDir)
+
+    // only if we aren't running compileJava, since doing twice fails (in some places)
+    onlyIf { !project.gradle.taskGraph.hasTask(sourceSet.getCompileTaskName("java")) }
+}
+
+val annotationProcessorMain by tasks.registering(JavaCompile::class) {
+    configureAnnotationSet(sourceSets.main.get())
+}
+
+ide {
+    // generate annotation processed files on project import/sync.
+    // adds to idea path but skip don't add to SourceSet since that triggers checkstyle
+    fun generatedSource(compile: TaskProvider<JavaCompile>) {
+        project.rootProject.configure<org.gradle.plugins.ide.idea.model.IdeaModel> {
+            project {
+                settings {
+                    taskTriggers {
+                        afterSync(compile.get())
+                    }
+                }
+            }
+        }
+    }
+
+    generatedSource(annotationProcessorMain)
 }

cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraRules.java

@@ -41,6 +41,8 @@
 import org.apache.calcite.sql.validate.SqlValidatorUtil;
 import org.apache.calcite.util.Pair;
 
+import org.immutables.value.Value;
+
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -113,9 +115,9 @@ abstract static class CassandraConverterRule extends ConverterRule {
    * @see #FILTER
    */
   public static class CassandraFilterRule
-      extends RelRule<CassandraFilterRule.Config> {
+      extends RelRule<CassandraFilterRule.CassandraFilterRuleConfig> {
     /** Creates a CassandraFilterRule. */
-    protected CassandraFilterRule(Config config) {
+    protected CassandraFilterRule(CassandraFilterRuleConfig config) {
       super(config);
     }
 
@@ -222,19 +224,25 @@ RelNode convert(LogicalFilter filter, CassandraTableScan scan) {
           scan.cassandraTable.getClusteringOrder());
     }
 
+    /** Deprecated in favor of CassandraFilterRuleConfig. **/
+    @Deprecated
+    public interface Config extends CassandraFilterRuleConfig { }
+
     /** Rule configuration. */
-    public interface Config extends RelRule.Config {
-      Config DEFAULT = EMPTY
+    @Value.Immutable
+    public interface CassandraFilterRuleConfig extends RelRule.Config {
+      CassandraFilterRuleConfig DEFAULT = ImmutableCassandraFilterRuleConfig.builder()
           .withOperandSupplier(b0 ->
               b0.operand(LogicalFilter.class)
                   .oneInput(b1 -> b1.operand(CassandraTableScan.class)
                       .noInputs()))
-          .as(Config.class);
+          .build();
 
       @Override default CassandraFilterRule toRule() {
         return new CassandraFilterRule(this);
       }
     }
+
   }
 
   /**
@@ -281,9 +289,9 @@ protected CassandraProjectRule(Config config) {
    * @see #SORT
    */
   public static class CassandraSortRule
-      extends RelRule<CassandraSortRule.Config> {
+      extends RelRule<CassandraSortRule.CassandraSortRuleConfig> {
     /** Creates a CassandraSortRule. */
-    protected CassandraSortRule(Config config) {
+    protected CassandraSortRule(CassandraSortRuleConfig config) {
       super(config);
     }
 
@@ -354,9 +362,14 @@ private static boolean collationsCompatible(RelCollation sortCollation,
       }
     }
 
+    /** Deprecated in favor of CassandraSortRuleConfig. **/
+    @Deprecated
+    public interface Config extends CassandraSortRuleConfig { }
+
     /** Rule configuration. */
-    public interface Config extends RelRule.Config {
-      Config DEFAULT = EMPTY
+    @Value.Immutable
+    public interface CassandraSortRuleConfig extends RelRule.Config {
+      CassandraSortRuleConfig DEFAULT = ImmutableCassandraSortRuleConfig.builder()
           .withOperandSupplier(b0 ->
               b0.operand(Sort.class)
                   // Limits are handled by CassandraLimit
@@ -370,8 +383,7 @@ public interface Config extends RelRule.Config {
                                   // single partition
                                   .predicate(
                                       CassandraFilter::isSinglePartition)
-                                  .anyInputs())))
-          .as(Config.class);
+                                  .anyInputs()))).build();
 
       @Override default CassandraSortRule toRule() {
         return new CassandraSortRule(this);
@@ -387,9 +399,9 @@ public interface Config extends RelRule.Config {
    * @see #LIMIT
    */
   public static class CassandraLimitRule
-      extends RelRule<CassandraLimitRule.Config> {
+      extends RelRule<CassandraLimitRule.CassandraLimitRuleConfig> {
     /** Creates a CassandraLimitRule. */
-    protected CassandraLimitRule(Config config) {
+    protected CassandraLimitRule(CassandraLimitRuleConfig config) {
       super(config);
     }
 
@@ -408,15 +420,19 @@ public RelNode convert(EnumerableLimit limit) {
       }
     }
 
+    /** Deprecated in favor of CassandraLimitRuleConfig. **/
+    @Deprecated
+    public interface Config extends CassandraLimitRuleConfig { }
+
     /** Rule configuration. */
-    public interface Config extends RelRule.Config {
-      Config DEFAULT = EMPTY
+    @Value.Immutable
+    public interface CassandraLimitRuleConfig extends RelRule.Config {
+      CassandraLimitRuleConfig DEFAULT = ImmutableCassandraLimitRuleConfig.builder()
           .withOperandSupplier(b0 ->
               b0.operand(EnumerableLimit.class)
                   .oneInput(b1 ->
                       b1.operand(CassandraToEnumerableConverter.class)
-                          .anyInputs()))
-          .as(Config.class);
+                          .anyInputs())).build();
 
       @Override default CassandraLimitRule toRule() {
         return new CassandraLimitRule(this);

core/build.gradle.kts

@@ -65,6 +65,10 @@ dependencies {
     implementation("org.codehaus.janino:janino")
     annotationProcessor("org.immutables:value")
     compileOnly("org.immutables:value-annotations")
+    compileOnly("com.google.code.findbugs:jsr305")
+    testAnnotationProcessor("org.immutables:value")
+    testCompileOnly("org.immutables:value-annotations")
+    testCompileOnly("com.google.code.findbugs:jsr305")
 
     testH2("com.h2database:h2")
     testMysql("mysql:mysql-connector-java")
@@ -194,19 +198,30 @@ fun JavaCompile.configureAnnotationSet(sourceSet: SourceSet) {
     org.gradle.api.plugins.internal.JvmPluginsHelper.configureAnnotationProcessorPath(sourceSet, sourceSet.java, options, project)
     destinationDirectory.set(temporaryDir)
 
-    // only if we aren't running compileJava, since doing twice fails (in some places)
+    // only if we aren't running java compilation, since doing twice fails (in some places)
     onlyIf { !project.gradle.taskGraph.hasTask(sourceSet.getCompileTaskName("java")) }
 }
 
 val annotationProcessorMain by tasks.registering(JavaCompile::class) {
-    dependsOn(javaCCMain)
     configureAnnotationSet(sourceSets.main.get())
 }
 
+val annotationProcessorTest by tasks.registering(JavaCompile::class) {
+    val kotlinTestCompile = tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompile>()
+        .getByName("compileTestKotlin")
+
+    dependsOn(javaCCTest, kotlinTestCompile)
+
+    configureAnnotationSet(sourceSets.test.get())
+    classpath += files(kotlinTestCompile.destinationDirectory.get())
+
+    // only if we aren't running compileJavaTest, since doing twice fails.
+    onlyIf { tasks.findByPath("compileTestJava")?.enabled != true }
+}
+
 ide {
     // generate annotation processed files on project import/sync.
-    // adds to idea path but skip don't add to SourceSet since that triggers checkstyle
-    fun generatedSource(compile: TaskProvider<JavaCompile>, sourceSetName: String) {
+    fun addSync(compile: TaskProvider<JavaCompile>) {
         project.rootProject.configure<org.gradle.plugins.ide.idea.model.IdeaModel> {
             project {
                 settings {
@@ -218,7 +233,8 @@ ide {
         }
     }
 
-    generatedSource(annotationProcessorMain, "main")
+    addSync(annotationProcessorMain)
+    addSync(annotationProcessorTest)
 }
 
 val integTestAll by tasks.registering() {

core/src/main/java/org/apache/calcite/CalciteImmutable.java

@@ -30,6 +30,7 @@
     visibility = Value.Style.ImplementationVisibility.PACKAGE,
     defaults = @Value.Immutable(builder = true, singleton = true),
     get = {"is*", "get*"},
-    init = "with*"
+    init = "with*",
+    passAnnotations = SuppressWarnings.class
 )
 public @interface CalciteImmutable { }

core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableBatchNestedLoopJoinRule.java

@@ -166,7 +166,7 @@ public interface Config extends RelRule.Config {
      *
      * <p>Warning: if the batch size is around or bigger than 1000 there
      * can be an error because the generated code exceeds the size limit. */
-    @ImmutableBeans.Property
+    @SuppressWarnings("deprecation") @ImmutableBeans.Property
     @ImmutableBeans.IntDefault(100)
     @Value.Default default int batchSize() {
       return 100;

core/src/main/java/org/apache/calcite/plan/RelRule.java

@@ -55,7 +55,14 @@
  *
  * <p>2. If your rule is not a sub-class of
  * {@link org.apache.calcite.rel.convert.ConverterRule},
- * create an inner {@code interface Config extends RelRule.Config}.
+ * create an inner {@code interface Config extends RelRule.Config} and
+ * annotate it with {@code @Value.Immutable}. Note, if your inner class
+ * is two levels deep (e.g. top-level Rule with Config inside), we recommend
+ * you annotate the outer class with {@code @Value.Enclosing} which will
+ * instruct the annotation processor to put your generated value class
+ * inside a new Immutable outer class. If your rule is three levels deep,
+ * the best thing to do is give your class a unique name to avoid any
+ * generated code class name overlaps.
  * Implement {@link Config#toRule() toRule} using a {@code default} method:
  *
  * <blockquote>
@@ -72,23 +79,24 @@
  *
  * <blockquote><pre><code>
  * &#x2f;** Returns foo. *&#x2f;
- * &#x40;ImmutableBeans.Property
  * int foo();
  *
  * &#x2f;** Sets {&#x40;link #foo}. *&#x2f;
  * Config withFoo(int x);
  * </code></pre></blockquote>
  *
  * <p>4. In your {@code Config} interface, create a {@code DEFAULT} constant
- * that represents the most typical configuration of your rule. For example,
+ * that represents the most typical configuration of your rule. This default
+ * will leverage the Immutables class generated by the Annotation Processor
+ * based on the annotation you provided above. For example,
  * {@code CsvProjectTableScanRule.Config} has the following:
  *
  * <blockquote><pre><code>
- * Config DEFAULT = EMPTY
+ * Config DEFAULT = ImmutableCsvProjectTableScanRule.Config.builder()
  *     .withOperandSupplier(b0 -&gt;
  *         b0.operand(LogicalProject.class).oneInput(b1 -&gt;
  *             b1.operand(CsvTableScan.class).noInputs()))
- *      .as(Config.class);
+ *      .build();
  * </code></pre></blockquote>
  *
  * <p>5. Do not create an {@code INSTANCE} constant inside your rule.
@@ -120,8 +128,17 @@ protected RelRule(C config) {
   }
 
   /** Rule configuration. */
+  @SuppressWarnings("deprecation")
   public interface Config {
-    /** Empty configuration. */
+    /**
+     * Empty configuration.
+     *
+     * This is based on ImmutableBeans and dynamic proxies and has been replaced
+     * by the use of the Immutables annotation processor to pre-generate values.
+     *
+     * This field will be removed in a subsequent release.
+     * */
+    @Deprecated
     RelRule.Config EMPTY = ImmutableBeans.create(Config.class)
         .withRelBuilderFactory(RelFactories.LOGICAL_BUILDER)
         .withOperandSupplier(b -> {
@@ -149,7 +166,7 @@ default <T extends Object> T as(Class<T> class_) {
 
     /** The factory that is used to create a
      * {@link org.apache.calcite.tools.RelBuilder} during rule invocations. */
-    @ImmutableBeans.Property
+    @SuppressWarnings("deprecation") @ImmutableBeans.Property
     @Value.Default default RelBuilderFactory relBuilderFactory() {
       return RelFactories.LOGICAL_BUILDER;
     }
@@ -158,14 +175,15 @@ default <T extends Object> T as(Class<T> class_) {
     Config withRelBuilderFactory(RelBuilderFactory factory);
 
     /** Description of the rule instance. */
-    @ImmutableBeans.Property
-    @Nullable String description();
+    // CALCITE-4831: remove the second nullable annotation once immutables/#1261 is fixed
+    @SuppressWarnings("deprecation") @ImmutableBeans.Property
+    @javax.annotation.Nullable @Nullable String description();
 
     /** Sets {@link #description()}. */
     Config withDescription(@Nullable String description);
 
     /** Creates the operands for the rule instance. */
-    @ImmutableBeans.Property
+    @SuppressWarnings("deprecation") @ImmutableBeans.Property
     @Value.Default default OperandTransform operandSupplier() {
       return s -> {
         throw new IllegalArgumentException("Rules must have at least one "

core/src/main/java/org/apache/calcite/rel/convert/ConverterRule.java

@@ -191,19 +191,19 @@ public interface Config extends RelRule.Config {
           }
         }).build();
 
-    @ImmutableBeans.Property
+    @SuppressWarnings("deprecation") @ImmutableBeans.Property
     RelTrait inTrait();
 
     /** Sets {@link #inTrait}. */
     Config withInTrait(RelTrait trait);
 
-    @ImmutableBeans.Property
+    @SuppressWarnings("deprecation") @ImmutableBeans.Property
     RelTrait outTrait();
 
     /** Sets {@link #outTrait}. */
     Config withOutTrait(RelTrait trait);
 
-    @ImmutableBeans.Property
+    @SuppressWarnings("deprecation") @ImmutableBeans.Property
     Function<Config, ConverterRule> ruleFactory();
 
     /** Sets {@link #outTrait}. */

core/src/main/java/org/apache/calcite/rel/convert/TraitMatchingRule.java

@@ -98,7 +98,7 @@ public interface Config extends RelRule.Config {
 
     /** Returns the rule to be restricted; rule must take a single
      * operand expecting a single input. */
-    @ImmutableBeans.Property
+    @SuppressWarnings("deprecation") @ImmutableBeans.Property
     ConverterRule converterRule();
 
     /** Sets {@link #converterRule()}. */