From 40df57d8179907dd8d6d137160afc6ed5623135f Mon Sep 17 00:00:00 2001
From: Maksym Tymoshyk <maksym.tymoshyk@gmail.com>
Date: Tue, 1 Sep 2026 14:09:44 +0300
Subject: [PATCH] [Java] Remove MS_EXPOSE_REP from the global SpotBugs filter

The global filter turns off MS_EXPOSE_REP for every module. That hides 34
findings today, and hides any new one automatically.

Remove that line and mark the existing sites one by one with
@SuppressFBWarnings, each carrying the reason it is exempt. Nothing changes
at runtime. What changes is that the next accessor someone writes that
returns a mutable static will fail spotbugsMain instead of passing quietly.

The three Guava sites cannot be configured away. SpotBugs checks its list of
known immutable types by fully qualified name, so it does not recognise
Guava relocated into org.apache.beam.vendor.guava, and the fallback scan
finds setter-named methods on ImmutableCollection. See
https://github.com/spotbugs/spotbugs/issues/1601.

Addresses #35312.
---
 .../beam/runners/spark/metrics/MetricsAccumulator.java   | 5 +++++
 .../structuredstreaming/metrics/MetricsAccumulator.java  | 5 +++++
 .../src/main/resources/beam/spotbugs-filter.xml          | 1 -
 .../main/java/org/apache/beam/sdk/metrics/Lineage.java   | 9 +++++++++
 .../java/org/apache/beam/sdk/metrics/SourceMetrics.java  | 9 +++++++++
 .../apache/beam/sdk/util/construction/ModelCoders.java   | 7 +++++++
 .../sdk/util/construction/PTransformTranslation.java     | 7 +++++++
 .../java/org/apache/beam/sdk/io/thrift/ThriftSchema.java | 5 +++++
 .../org/apache/beam/sdk/testutils/NamedTestResult.java   | 7 +++++++
 .../java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java     | 6 ++++++
 10 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/runners/spark/src/main/java/org/apache/beam/runners/spark/metrics/MetricsAccumulator.java b/runners/spark/src/main/java/org/apache/beam/runners/spark/metrics/MetricsAccumulator.java
index dbdfe11a585c..612d71b1aea1 100644
--- a/runners/spark/src/main/java/org/apache/beam/runners/spark/metrics/MetricsAccumulator.java
+++ b/runners/spark/src/main/java/org/apache/beam/runners/spark/metrics/MetricsAccumulator.java
@@ -17,6 +17,7 @@
  */
 package org.apache.beam.runners.spark.metrics;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import org.apache.beam.runners.core.metrics.MetricsContainerStepMap;
 import org.apache.beam.runners.spark.SparkPipelineOptions;
@@ -82,6 +83,10 @@ public static void init(SparkPipelineOptions opts, JavaSparkContext jsc) {
     }
   }
 
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "Spark merges only the accumulator instance the driver registered. A copy would collect metrics that nothing reports.")
   public static MetricsContainerStepMapAccumulator getInstance() {
     if (instance == null) {
       throw new IllegalStateException("Metrics accumulator has not been instantiated");
diff --git a/runners/spark/src/main/java/org/apache/beam/runners/spark/structuredstreaming/metrics/MetricsAccumulator.java b/runners/spark/src/main/java/org/apache/beam/runners/spark/structuredstreaming/metrics/MetricsAccumulator.java
index 63407b9f14d8..e8cbd895082a 100644
--- a/runners/spark/src/main/java/org/apache/beam/runners/spark/structuredstreaming/metrics/MetricsAccumulator.java
+++ b/runners/spark/src/main/java/org/apache/beam/runners/spark/structuredstreaming/metrics/MetricsAccumulator.java
@@ -17,6 +17,7 @@
  */
 package org.apache.beam.runners.spark.structuredstreaming.metrics;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.beam.runners.core.metrics.MetricsContainerStepMap;
 import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.annotations.VisibleForTesting;
 import org.apache.spark.sql.SparkSession;
@@ -85,6 +86,10 @@ public MetricsContainerStepMap value() {
    * Get the {@link MetricsAccumulator} on this driver. If there's no such accumulator yet, it will
    * be created and registered using the provided {@link SparkSession}.
    */
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "Spark merges only the accumulator instance the driver registered. A copy would collect metrics that nothing reports.")
   public static MetricsAccumulator getInstance(SparkSession session) {
     MetricsAccumulator current = instance;
     if (current != null) {
diff --git a/sdks/java/build-tools/src/main/resources/beam/spotbugs-filter.xml b/sdks/java/build-tools/src/main/resources/beam/spotbugs-filter.xml
index 4393ec6a624e..5f6f368228e5 100644
--- a/sdks/java/build-tools/src/main/resources/beam/spotbugs-filter.xml
+++ b/sdks/java/build-tools/src/main/resources/beam/spotbugs-filter.xml
@@ -57,7 +57,6 @@
 
   <!-- TODO(https://github.com/apache/beam/issues/35312) resolve findings-->
   <Bug pattern="CT_CONSTRUCTOR_THROW"/>
-  <Bug pattern="MS_EXPOSE_REP"/>
 
   <!--
     Many test classes are captured by lambdas and marked `implements Serializable`. They are not
diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/Lineage.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/Lineage.java
index f845d8fde940..681f022c9042 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/Lineage.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/Lineage.java
@@ -19,6 +19,7 @@
 
 import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkNotNull;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -122,6 +123,10 @@ private static Lineage createLineage(PipelineOptions options, LineageDirection d
   }
 
   /** {@link Lineage} representing sources and optionally side inputs. */
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "Every reporter writes into the same metric cell, so all callers need the one shared instance. A copy would drop the lineage it records.")
   public static Lineage getSources() {
     Lineage localSources = sources;
     if (localSources == null) {
@@ -131,6 +136,10 @@ public static Lineage getSources() {
   }
 
   /** {@link Lineage} representing sinks. */
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "Every reporter writes into the same metric cell, so all callers need the one shared instance. A copy would drop the lineage it records.")
   public static Lineage getSinks() {
     Lineage localSinks = sinks;
     if (localSinks == null) {
diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/SourceMetrics.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/SourceMetrics.java
index e9677626f88b..5d033d4a91aa 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/SourceMetrics.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/SourceMetrics.java
@@ -17,6 +17,7 @@
  */
 package org.apache.beam.sdk.metrics;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Joiner;
 
 /** Standard {@link org.apache.beam.sdk.io.Source} Metrics. */
@@ -69,6 +70,10 @@ public static Counter bytesReadBySplit(String splitId) {
   }
 
   /** Gauge for source backlog in bytes. */
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "A Gauge is a handle onto one metric cell, not a value. A copy would send the reading nowhere.")
   public static Gauge backlogBytes() {
     return BACKLOG_BYTES_GAUGE;
   }
@@ -84,6 +89,10 @@ public static Gauge backlogBytesOfSplit(String splitId) {
   }
 
   /** Gauge for source backlog in elements. */
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "A Gauge is a handle onto one metric cell, not a value. A copy would send the reading nowhere.")
   public static Gauge backlogElements() {
     return BACKLOG_ELEMENTS_GAUGE;
   }
diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/ModelCoders.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/ModelCoders.java
index 5059cc1c6b83..f625e376abcc 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/ModelCoders.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/ModelCoders.java
@@ -22,6 +22,7 @@
 import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkState;
 
 import com.google.auto.value.AutoValue;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Set;
 import org.apache.beam.model.pipeline.v1.RunnerApi.Coder;
 import org.apache.beam.model.pipeline.v1.RunnerApi.FunctionSpec;
@@ -97,6 +98,12 @@ private ModelCoders() {}
           SHARDED_KEY_CODER_URN,
           NULLABLE_CODER_URN);
 
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "Returns a Guava ImmutableSet."
+              + " Spotbugs matches its known-immutable list by fully qualified name, so it cannot recognise collections relocated into org.apache.beam.vendor.guava."
+              + " See https://github.com/spotbugs/spotbugs/issues/1601.")
   public static Set<String> urns() {
     return MODEL_CODER_URNS;
   }
diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/PTransformTranslation.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/PTransformTranslation.java
index 5f8782265f17..88bfb8b5b538 100644
--- a/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/PTransformTranslation.java
+++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/util/construction/PTransformTranslation.java
@@ -22,6 +22,7 @@
 import static org.apache.beam.sdk.util.construction.BeamUrns.getUrn;
 import static org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Preconditions.checkState;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
@@ -417,6 +418,12 @@ public RunnerApi.PTransform translate(
       knownPayloadTranslators;
 
   @Internal
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "Returns a Guava ImmutableMap."
+              + " Spotbugs matches its known-immutable list by fully qualified name, so it cannot recognise collections relocated into org.apache.beam.vendor.guava."
+              + " See https://github.com/spotbugs/spotbugs/issues/1601.")
   public static Map<Class<? extends PTransform>, TransformPayloadTranslator>
       getKnownPayloadTranslators() {
     if (knownPayloadTranslators == null) {
diff --git a/sdks/java/io/thrift/src/main/java/org/apache/beam/sdk/io/thrift/ThriftSchema.java b/sdks/java/io/thrift/src/main/java/org/apache/beam/sdk/io/thrift/ThriftSchema.java
index e4e698faffa4..b24f132a0010 100644
--- a/sdks/java/io/thrift/src/main/java/org/apache/beam/sdk/io/thrift/ThriftSchema.java
+++ b/sdks/java/io/thrift/src/main/java/org/apache/beam/sdk/io/thrift/ThriftSchema.java
@@ -19,6 +19,7 @@
 
 import static java.util.Collections.unmodifiableMap;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.Arrays;
@@ -121,6 +122,10 @@ private ThriftSchema(Map<String, FieldType> typedefs) {
    *
    * @see #custom() for how to manually pass the beam type for container typedefs
    */
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "The default provider holds an empty typedef map that is never mutated. Callers needing typedefs go through custom(), which builds a separate instance.")
   public static @NonNull SchemaProvider provider() {
     return defaultProvider;
   }
diff --git a/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/NamedTestResult.java b/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/NamedTestResult.java
index a4f40598d5a3..112c29b30ee1 100644
--- a/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/NamedTestResult.java
+++ b/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/NamedTestResult.java
@@ -18,6 +18,7 @@
 package org.apache.beam.sdk.testutils;
 
 import com.google.cloud.bigquery.LegacySQLTypeName;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Map;
 import org.apache.beam.sdk.testutils.publishing.InfluxDBPublisher;
 import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.ImmutableMap;
@@ -83,6 +84,12 @@ public Map<String, Object> toMap() {
         .build();
   }
 
+  @SuppressFBWarnings(
+      value = "MS_EXPOSE_REP",
+      justification =
+          "Returns a Guava ImmutableMap."
+              + " Spotbugs matches its known-immutable list by fully qualified name, so it cannot recognise collections relocated into org.apache.beam.vendor.guava."
+              + " See https://github.com/spotbugs/spotbugs/issues/1601.")
   public static Map<String, String> getSchema() {
     return schema;
   }
diff --git a/sdks/java/testing/tpcds/src/main/java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java b/sdks/java/testing/tpcds/src/main/java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java
index a22bab1c89e6..763e8164dee0 100644
--- a/sdks/java/testing/tpcds/src/main/java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java
+++ b/sdks/java/testing/tpcds/src/main/java/org/apache/beam/sdk/tpcds/TpcdsSchemas.java
@@ -17,12 +17,18 @@
  */
 package org.apache.beam.sdk.tpcds;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import org.apache.beam.sdk.schemas.Schema;
 import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.ImmutableMap;
 
+@SuppressFBWarnings(
+    value = "MS_EXPOSE_REP",
+    justification =
+        "Fixed TPC-DS table definitions, built once at class load."
+            + " Schema is mutable only through setUUID, which no caller here uses, and copying twenty-four schemas on every accessor call would cost real time in a benchmark harness.")
 public class TpcdsSchemas {
   /**
    * Get all tpcds table schemas automatically by reading json files. In this case all field will be