From bfc2b4e7d0b116ef0351a90ccc5918c92716592e Mon Sep 17 00:00:00 2001 From: Camille Schneider Date: Tue, 28 Jan 2025 17:06:25 +0000 Subject: [PATCH 1/4] Add errorprone and supress existing errors --- .../executor/spanner/CloudClientExecutor.java | 2 + .../executor/spanner/CloudExecutorImpl.java | 2 + .../cloud/spanner/AbstractReadContext.java | 2 + .../cloud/spanner/OpenTelemetryApiTracer.java | 2 + .../com/google/cloud/spanner/Options.java | 2 + .../com/google/cloud/spanner/SessionPool.java | 30 +++++++++++++ .../com/google/cloud/spanner/SpannerImpl.java | 2 + .../google/cloud/spanner/SpannerOptions.java | 4 ++ .../google/cloud/spanner/TraceWrapper.java | 2 + .../connection/AbstractBaseUnitOfWork.java | 2 + .../spanner/AbstractReadContextTest.java | 2 + .../google/cloud/spanner/BackupIdTest.java | 2 + .../cloud/spanner/ChannelUsageTest.java | 2 + .../spanner/DatabaseAdminClientTest.java | 4 ++ .../cloud/spanner/DatabaseClientImplTest.java | 20 +++++++++ .../google/cloud/spanner/DatabaseTest.java | 2 + ...OpenTelemetryBuiltInMetricsTracerTest.java | 2 + .../spanner/RandomResultSetGenerator.java | 2 + .../cloud/spanner/SelectRandomBenchmark.java | 2 + .../google/cloud/spanner/SessionPoolTest.java | 14 ++++++ .../spanner/SessionPoolUnbalancedTest.java | 2 + .../com/google/cloud/spanner/TypeTest.java | 4 ++ .../connection/RandomResultSetGenerator.java | 2 + pom.xml | 45 +++++++++++++++++++ 24 files changed, 155 insertions(+) diff --git a/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudClientExecutor.java b/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudClientExecutor.java index f7fa02a9958..e7a3491f7d3 100644 --- a/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudClientExecutor.java +++ b/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudClientExecutor.java @@ -2863,6 +2863,8 @@ private com.google.spanner.v1.StructType buildStructType(StructReader struct) { } /** Convert a struct to a proto(value list) for constructing result rows and struct values. */ + // Suppressed for initial Error Prone rollout. + @SuppressWarnings({"ProtoBuilderReturnValueIgnored", "ReturnValueIgnored"}) private com.google.spanner.executor.v1.ValueList buildStruct(StructReader struct) { com.google.spanner.executor.v1.ValueList.Builder structBuilder = com.google.spanner.executor.v1.ValueList.newBuilder(); diff --git a/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudExecutorImpl.java b/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudExecutorImpl.java index f3de36ac7b4..cbe6301974a 100644 --- a/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudExecutorImpl.java +++ b/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudExecutorImpl.java @@ -72,6 +72,8 @@ public StreamObserver executeActionAsync( // Create a top-level OpenTelemetry span for streaming request. Tracer tracer = WorkerProxy.openTelemetrySdk.getTracer(CloudClientExecutor.class.getName()); Span span = tracer.spanBuilder("java_systest_execute_actions_stream").setNoParent().startSpan(); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("MustBeClosedChecker") Scope scope = span.makeCurrent(); final String traceId = span.getSpanContext().getTraceId(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index 6ea4510d3db..ea877c2c70e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -212,6 +212,8 @@ protected boolean isRouteToLeader() { return false; } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @GuardedBy("lock") @Override void beforeReadOrQueryLocked() { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/OpenTelemetryApiTracer.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/OpenTelemetryApiTracer.java index 863c531de30..3b43490e3e1 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/OpenTelemetryApiTracer.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/OpenTelemetryApiTracer.java @@ -77,6 +77,8 @@ Span getSpan() { @Override public Scope inScope() { + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("MustBeClosedChecker") final io.opentelemetry.context.Scope openTelemetryScope = span.makeCurrent(); return openTelemetryScope::close; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java index c36f1902648..8da85252be2 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java @@ -1025,6 +1025,8 @@ void appendToOptions(Options options) { options.filter = filter; } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("EqualsHashCode") @Override public boolean equals(Object o) { if (o == this) return true; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java index 0b55a893aca..9f37c53139d 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java @@ -587,6 +587,8 @@ public PooledSessionFuture replaceSession( } } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Override public PooledSessionFuture denyListSession( RetryOnDifferentGrpcChannelException retryException, PooledSessionFuture session) { @@ -1768,6 +1770,8 @@ public ApiFuture asyncClose() { return ApiFutures.immediateFuture(Empty.getDefaultInstance()); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Override public void close() { synchronized (lock) { @@ -1848,12 +1852,16 @@ public SessionImpl getDelegate() { return this.delegate; } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Override public void markBusy(ISpan span) { this.delegate.setCurrentSpan(span); this.state = SessionState.BUSY; } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private void markClosing() { this.state = SessionState.CLOSING; } @@ -2091,6 +2099,8 @@ void maintainPool() { removeLongRunningSessions(currTime); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private void removeIdleSessions(Instant currTime) { synchronized (lock) { // Determine the minimum last use time for a session to be deemed to still be alive. Remove @@ -2190,6 +2200,8 @@ void removeLongRunningSessions(Instant currentTime) { } } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private void removeLongRunningSessions( final Instant currentTime, final InactiveTransactionRemovalOptions inactiveTransactionRemovalOptions) { @@ -2691,6 +2703,8 @@ private void invalidateSession(PooledSession session) { } } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private Tuple findSessionToKeepAlive( Queue queue, Instant keepAliveThreshold, int numAlreadyChecked) { int numChecked = 0; @@ -2885,6 +2899,8 @@ private void releaseSession(PooledSession session, boolean isNewSession) { } /** Releases a session back to the pool. This might cause one of the waiters to be unblocked. */ + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private void releaseSession( PooledSession session, boolean isNewSession, @Nullable Integer position) { Preconditions.checkNotNull(session); @@ -2945,6 +2961,8 @@ private void releaseSession( * running many small, quick queries using a small number of parallel threads. This can cause a * high TPS, without actually having a high degree of parallelism. */ + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @VisibleForTesting boolean shouldRandomize() { return this.options.getRandomizePositionQPSThreshold() > 0 @@ -2952,6 +2970,8 @@ boolean shouldRandomize() { && this.numSessionsInUse >= this.numChannels; } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private boolean isUnbalanced(PooledSession session) { int channel = session.getChannel(); int numChannels = sessionClient.getSpanner().getOptions().getNumChannels(); @@ -3054,10 +3074,14 @@ private void handleCreateSessionsFailure(SpannerException e, int count) { } } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") void setResourceNotFoundException(ResourceNotFoundException e) { this.resourceNotFoundException = MoreObjects.firstNonNull(this.resourceNotFoundException, e); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private void decrementPendingClosures(int count) { pendingClosure -= count; if (pendingClosure == 0) { @@ -3070,6 +3094,8 @@ private void decrementPendingClosures(int count) { * {@code IllegalStateException}. The returned future blocks till all the sessions created in this * pool have been closed. */ + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") ListenableFuture closeAsync(ClosedException closedException) { ListenableFuture retFuture = null; synchronized (lock) { @@ -3269,6 +3295,8 @@ public void onSessionCreateFailure(Throwable t, int createFailureForSessionCount * Initializes and creates Spanner session relevant metrics using OpenCensus. When coupled with an * exporter, it allows users to monitor client behavior. */ + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private void initOpenCensusMetricsCollection( MetricRegistry metricRegistry, List labelValues, @@ -3402,6 +3430,8 @@ private void initOpenCensusMetricsCollection( * Initializes and creates Spanner session relevant metrics using OpenTelemetry. When coupled with * an exporter, it allows users to monitor client behavior. */ + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private void initOpenTelemetryMetricsCollection( OpenTelemetry openTelemetry, Attributes attributes, diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 8f5baca64f6..dfab163610e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -375,6 +375,8 @@ public void close() { close(Long.MAX_VALUE, TimeUnit.MILLISECONDS); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") void close(long timeout, TimeUnit unit) { List> closureFutures; synchronized (this) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 0995c478427..be984f34914 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1828,6 +1828,8 @@ public static void enableOpenCensusTraces() { * Always resets the activeTracingFramework. This variable is used for internal testing, and is * not a valid production scenario */ + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @ObsoleteApi( "The OpenCensus project is deprecated. Use enableOpenTelemetryTraces to switch to" + " OpenTelemetry traces") @@ -2023,6 +2025,8 @@ private ApiTracerFactory createApiTracerFactory( return new CompositeTracerFactory(apiTracerFactories); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private ApiTracerFactory getDefaultApiTracerFactory() { if (isEnableApiTracing()) { if (activeTracingFramework == TracingFramework.OPEN_TELEMETRY) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TraceWrapper.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TraceWrapper.java index df5874cb3c6..eeb527300c3 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TraceWrapper.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TraceWrapper.java @@ -130,6 +130,8 @@ ISpan getBlankSpan() { } } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("MustBeClosedChecker") IScope withSpan(ISpan span) { if (SpannerOptions.getActiveTracingFramework().equals(TracingFramework.OPEN_TELEMETRY)) { OpenTelemetrySpan openTelemetrySpan; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractBaseUnitOfWork.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractBaseUnitOfWork.java index 5f4facf1489..279380b394c 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractBaseUnitOfWork.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractBaseUnitOfWork.java @@ -401,6 +401,8 @@ public ApiCallContext configure( } future.addListener( new Runnable() { + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Override public void run() { synchronized (this) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractReadContextTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractReadContextTest.java index eea6658d26d..e5736770268 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractReadContextTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractReadContextTest.java @@ -303,6 +303,8 @@ public void testExecuteBatchDmlLastStatement() { .getLastStatements()); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("JUnit4TestNotRun") public void executeSqlRequestBuilderWithRequestOptions() { ExecuteSqlRequest request = context diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BackupIdTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BackupIdTest.java index 76ba64a2ff2..c2e48b2279c 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BackupIdTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BackupIdTest.java @@ -40,6 +40,8 @@ public void basics() { assertThat(bid.toString()).isEqualTo(name); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("CheckReturnValue") @Test public void badName() { IllegalArgumentException e = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java index 30e06719181..2d7d272a408 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java @@ -228,6 +228,8 @@ public void testCreatesNumChannels() { assertEquals(numChannels, batchCreateSessionLocalIps.size()); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("CheckReturnValue") @Test public void testUsesAllChannels() throws InterruptedException { final int multiplier = 2; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientTest.java index 752f4c524cc..f5c52b1b1f2 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientTest.java @@ -631,6 +631,8 @@ public void instanceListDatabaseOperations() assertThat(instance.listDatabaseOperations().iterateAll()).hasSize(6); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("CheckReturnValue") @Test public void instanceListDatabaseOperationsWithMetadata() throws Exception { Instance instance = @@ -713,6 +715,8 @@ public void instanceListBackupOperations() assertThat(instance.listBackupOperations().iterateAll()).hasSize(2); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("CheckReturnValue") @Test public void instanceListBackupOperationsWithProgress() throws InvalidProtocolBufferException { Instance instance = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java index 6ceb3e979bf..bbdfd6f8d5e 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java @@ -2331,6 +2331,8 @@ public void singleUse() { DatabaseClientImpl client = (DatabaseClientImpl) spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") Set checkedOut = client.pool.checkedOutSessions; assertThat(checkedOut).isEmpty(); try (ResultSet rs = client.singleUse().executeQuery(SELECT1)) { @@ -3919,6 +3921,8 @@ public void testCreateSessionsFailure_shouldNotPropagateToCloseMethod() { @Test public void testReadWriteTransaction_usesOptions() { SessionPool pool = mock(SessionPool.class); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("DoNotMock") PooledSessionFuture session = mock(PooledSessionFuture.class); when(pool.getSession()).thenReturn(session); TransactionOption option = mock(TransactionOption.class); @@ -3935,6 +3939,8 @@ public void testReadWriteTransaction_usesOptions() { @Test public void testTransactionManager_usesOptions() { SessionPool pool = mock(SessionPool.class); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("DoNotMock") PooledSessionFuture session = mock(PooledSessionFuture.class); when(pool.getSession()).thenReturn(session); TransactionOption option = mock(TransactionOption.class); @@ -3948,6 +3954,8 @@ public void testTransactionManager_usesOptions() { @Test public void testRunAsync_usesOptions() { SessionPool pool = mock(SessionPool.class); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("DoNotMock") PooledSessionFuture session = mock(PooledSessionFuture.class); when(pool.getSession()).thenReturn(session); TransactionOption option = mock(TransactionOption.class); @@ -3961,6 +3969,8 @@ public void testRunAsync_usesOptions() { @Test public void testTransactionManagerAsync_usesOptions() { SessionPool pool = mock(SessionPool.class); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("DoNotMock") PooledSessionFuture session = mock(PooledSessionFuture.class); when(pool.getSession()).thenReturn(session); TransactionOption option = mock(TransactionOption.class); @@ -4240,6 +4250,8 @@ public void singleUseNoAction_ClearsCheckedOutSession() { DatabaseClientImpl client = (DatabaseClientImpl) spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") Set checkedOut = client.pool.checkedOutSessions; assertThat(checkedOut).isEmpty(); @@ -4255,6 +4267,8 @@ public void singleUseReadOnlyTransactionNoAction_ClearsCheckedOutSession() { DatabaseClientImpl client = (DatabaseClientImpl) spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") Set checkedOut = client.pool.checkedOutSessions; assertThat(checkedOut).isEmpty(); @@ -4268,6 +4282,8 @@ public void readWriteTransactionNoAction_ClearsCheckedOutSession() { DatabaseClientImpl client = (DatabaseClientImpl) spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") Set checkedOut = client.pool.checkedOutSessions; assertThat(checkedOut).isEmpty(); @@ -4281,6 +4297,8 @@ public void readOnlyTransactionNoAction_ClearsCheckedOutSession() { DatabaseClientImpl client = (DatabaseClientImpl) spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") Set checkedOut = client.pool.checkedOutSessions; assertThat(checkedOut).isEmpty(); @@ -4294,6 +4312,8 @@ public void transactionManagerNoAction_ClearsCheckedOutSession() { DatabaseClientImpl client = (DatabaseClientImpl) spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") Set checkedOut = client.pool.checkedOutSessions; assertThat(checkedOut).isEmpty(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseTest.java index dc65f738619..0f33a4e10b6 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseTest.java @@ -165,6 +165,8 @@ public void testUnspecifiedDialectDefaultsToGoogleStandardSqlDialect() { assertEquals(Dialect.GOOGLE_STANDARD_SQL, database.getDialect()); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("SetUnrecognized") @Test public void testUnrecognizedDialectThrowsException() { assertThrows( diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java index 9392e202b7a..8a58dfd8602 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java @@ -279,6 +279,8 @@ public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception { } } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("CheckReturnValue") @Test public void testMetricsWithGaxRetryUnaryRpc() { Stopwatch stopwatch = Stopwatch.createStarted(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RandomResultSetGenerator.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RandomResultSetGenerator.java index 051546352cb..ad6efb47c7a 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RandomResultSetGenerator.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RandomResultSetGenerator.java @@ -86,6 +86,8 @@ public class RandomResultSetGenerator { .build(), }; + // Suppressed for initial Error Prone rollout. + @SuppressWarnings({"ProtoBuilderReturnValueIgnored", "ReturnValueIgnored"}) private static ResultSetMetadata generateMetadata() { StructType.Builder rowTypeBuilder = StructType.newBuilder(); for (int col = 0; col < TYPES.length; col++) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SelectRandomBenchmark.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SelectRandomBenchmark.java index e18cddd3bf5..c4c6f2eee2a 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SelectRandomBenchmark.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SelectRandomBenchmark.java @@ -113,6 +113,8 @@ public void teardown() throws Exception { } /** Measures the time needed to execute a burst of read requests. */ + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("ReturnValueIgnored") @Benchmark public void burstRead(final BenchmarkState server) throws Exception { int totalQueries = server.maxSessions * 8; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java index b027ebbc07f..66b4a10832a 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java @@ -808,6 +808,8 @@ public void idleSessionCleanup() throws Exception { pool.closeAsync(new SpannerImpl.ClosedException()).get(5L, TimeUnit.SECONDS); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Test public void longRunningTransactionsCleanup_whenActionSetToClose_verifyInactiveSessionsClosed() throws Exception { @@ -854,6 +856,8 @@ public void longRunningTransactionsCleanup_whenActionSetToClose_verifyInactiveSe pool.closeAsync(new SpannerImpl.ClosedException()).get(5L, TimeUnit.SECONDS); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Test public void longRunningTransactionsCleanup_whenActionSetToWarn_verifyInactiveSessionsOpen() throws Exception { @@ -902,6 +906,8 @@ public void longRunningTransactionsCleanup_whenActionSetToWarn_verifyInactiveSes pool.closeAsync(new SpannerImpl.ClosedException()).get(5L, TimeUnit.SECONDS); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Test public void longRunningTransactionsCleanup_whenUtilisationBelowThreshold_verifyInactiveSessionsOpen() @@ -945,6 +951,8 @@ public void longRunningTransactionsCleanup_whenActionSetToWarn_verifyInactiveSes pool.closeAsync(new SpannerImpl.ClosedException()).get(5L, TimeUnit.SECONDS); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Test public void longRunningTransactionsCleanup_whenAllAreExpectedlyLongRunning_verifyInactiveSessionsOpen() @@ -1011,6 +1019,8 @@ public void longRunningTransactionsCleanup_whenActionSetToWarn_verifyInactiveSes pool.closeAsync(new SpannerImpl.ClosedException()).get(5L, TimeUnit.SECONDS); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Test public void longRunningTransactionsCleanup_whenBelowDurationThreshold_verifyInactiveSessionsOpen() throws Exception { @@ -1056,6 +1066,8 @@ public void longRunningTransactionsCleanup_whenBelowDurationThreshold_verifyInac pool.closeAsync(new SpannerImpl.ClosedException()).get(5L, TimeUnit.SECONDS); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Test public void longRunningTransactionsCleanup_whenException_doNothing() throws Exception { Clock clock = mock(Clock.class); @@ -1099,6 +1111,8 @@ public void longRunningTransactionsCleanup_whenException_doNothing() throws Exce pool.closeAsync(new SpannerImpl.ClosedException()).get(5L, TimeUnit.SECONDS); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") @Test public void longRunningTransactionsCleanup_whenTaskRecurrenceBelowThreshold_verifyInactiveSessionsOpen() diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolUnbalancedTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolUnbalancedTest.java index 5a9365eaed9..71b3d75d077 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolUnbalancedTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolUnbalancedTest.java @@ -49,6 +49,8 @@ static List mockedSessions(int... channels) { static PooledSessionFuture mockedCheckedOutSession(int channel) { PooledSession session = mockedSession(channel); + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("DoNotMock") PooledSessionFuture future = mock(PooledSessionFuture.class); when(future.get()).thenReturn(session); when(future.isDone()).thenReturn(true); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TypeTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TypeTest.java index 8fc168eae96..a3310a818b2 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TypeTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TypeTest.java @@ -541,6 +541,8 @@ public void emptyStruct() { assertProtoEquals(t.toProto(), "code: STRUCT struct_type {}"); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("CheckReturnValue") @Test public void structFieldIndexNotFound() { Type t = Type.struct(StructField.of("f1", Type.int64())); @@ -549,6 +551,8 @@ public void structFieldIndexNotFound() { assertThat(e.getMessage().contains("Field not found: f2")); } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("CheckReturnValue") @Test public void structFieldIndexAmbiguous() { Type t = Type.struct(StructField.of("f1", Type.int64()), StructField.of("f1", Type.string())); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java index e21e0020f6e..0c5c7bc8717 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java @@ -185,6 +185,8 @@ private static void appendProtoTypes(List types, Dialect dialect) { } } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings({"ProtoBuilderReturnValueIgnored", "ReturnValueIgnored"}) public static ResultSetMetadata generateAllTypesMetadata(Type[] types) { StructType.Builder rowTypeBuilder = StructType.newBuilder(); for (int col = 0; col < types.length; col++) { diff --git a/pom.xml b/pom.xml index 7220e5fdffe..a19cba0e855 100644 --- a/pom.xml +++ b/pom.xml @@ -54,6 +54,7 @@ UTF-8 github google-cloud-spanner-parent + 2.31.0 @@ -133,6 +134,50 @@ + + + java9 + + [9,) + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.13.0 + + UTF-8 + true + + -XDcompilePolicy=simple + -Xplugin:ErrorProne + -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED + -J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED + -J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED + + + + com.google.errorprone + error_prone_core + ${error-prone.version} + + + + + + + + + + google-cloud-spanner grpc-google-cloud-spanner-v1 From 3355d2ad591c0c878a8a3056f1b2cc72b16424d7 Mon Sep 17 00:00:00 2001 From: Camille Schneider Date: Thu, 30 Jan 2025 18:38:15 +0000 Subject: [PATCH 2/4] Suppressing some errorprone checks with flags --- .../google/cloud/executor/spanner/CloudClientExecutor.java | 2 -- .../src/test/java/com/google/cloud/spanner/BackupIdTest.java | 2 -- .../test/java/com/google/cloud/spanner/ChannelUsageTest.java | 2 -- .../com/google/cloud/spanner/DatabaseAdminClientTest.java | 4 ---- .../cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java | 2 -- .../com/google/cloud/spanner/RandomResultSetGenerator.java | 2 -- .../java/com/google/cloud/spanner/SelectRandomBenchmark.java | 2 -- .../src/test/java/com/google/cloud/spanner/TypeTest.java | 4 ---- .../cloud/spanner/connection/RandomResultSetGenerator.java | 2 -- pom.xml | 3 ++- 10 files changed, 2 insertions(+), 23 deletions(-) diff --git a/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudClientExecutor.java b/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudClientExecutor.java index e7a3491f7d3..f7fa02a9958 100644 --- a/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudClientExecutor.java +++ b/google-cloud-spanner-executor/src/main/java/com/google/cloud/executor/spanner/CloudClientExecutor.java @@ -2863,8 +2863,6 @@ private com.google.spanner.v1.StructType buildStructType(StructReader struct) { } /** Convert a struct to a proto(value list) for constructing result rows and struct values. */ - // Suppressed for initial Error Prone rollout. - @SuppressWarnings({"ProtoBuilderReturnValueIgnored", "ReturnValueIgnored"}) private com.google.spanner.executor.v1.ValueList buildStruct(StructReader struct) { com.google.spanner.executor.v1.ValueList.Builder structBuilder = com.google.spanner.executor.v1.ValueList.newBuilder(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BackupIdTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BackupIdTest.java index c2e48b2279c..76ba64a2ff2 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BackupIdTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BackupIdTest.java @@ -40,8 +40,6 @@ public void basics() { assertThat(bid.toString()).isEqualTo(name); } - // Suppressed for initial Error Prone rollout. - @SuppressWarnings("CheckReturnValue") @Test public void badName() { IllegalArgumentException e = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java index 2d7d272a408..30e06719181 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ChannelUsageTest.java @@ -228,8 +228,6 @@ public void testCreatesNumChannels() { assertEquals(numChannels, batchCreateSessionLocalIps.size()); } - // Suppressed for initial Error Prone rollout. - @SuppressWarnings("CheckReturnValue") @Test public void testUsesAllChannels() throws InterruptedException { final int multiplier = 2; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientTest.java index f5c52b1b1f2..752f4c524cc 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientTest.java @@ -631,8 +631,6 @@ public void instanceListDatabaseOperations() assertThat(instance.listDatabaseOperations().iterateAll()).hasSize(6); } - // Suppressed for initial Error Prone rollout. - @SuppressWarnings("CheckReturnValue") @Test public void instanceListDatabaseOperationsWithMetadata() throws Exception { Instance instance = @@ -715,8 +713,6 @@ public void instanceListBackupOperations() assertThat(instance.listBackupOperations().iterateAll()).hasSize(2); } - // Suppressed for initial Error Prone rollout. - @SuppressWarnings("CheckReturnValue") @Test public void instanceListBackupOperationsWithProgress() throws InvalidProtocolBufferException { Instance instance = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java index 8a58dfd8602..9392e202b7a 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java @@ -279,8 +279,6 @@ public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception { } } - // Suppressed for initial Error Prone rollout. - @SuppressWarnings("CheckReturnValue") @Test public void testMetricsWithGaxRetryUnaryRpc() { Stopwatch stopwatch = Stopwatch.createStarted(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RandomResultSetGenerator.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RandomResultSetGenerator.java index ad6efb47c7a..051546352cb 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RandomResultSetGenerator.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RandomResultSetGenerator.java @@ -86,8 +86,6 @@ public class RandomResultSetGenerator { .build(), }; - // Suppressed for initial Error Prone rollout. - @SuppressWarnings({"ProtoBuilderReturnValueIgnored", "ReturnValueIgnored"}) private static ResultSetMetadata generateMetadata() { StructType.Builder rowTypeBuilder = StructType.newBuilder(); for (int col = 0; col < TYPES.length; col++) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SelectRandomBenchmark.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SelectRandomBenchmark.java index c4c6f2eee2a..e18cddd3bf5 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SelectRandomBenchmark.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SelectRandomBenchmark.java @@ -113,8 +113,6 @@ public void teardown() throws Exception { } /** Measures the time needed to execute a burst of read requests. */ - // Suppressed for initial Error Prone rollout. - @SuppressWarnings("ReturnValueIgnored") @Benchmark public void burstRead(final BenchmarkState server) throws Exception { int totalQueries = server.maxSessions * 8; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TypeTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TypeTest.java index a3310a818b2..8fc168eae96 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TypeTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TypeTest.java @@ -541,8 +541,6 @@ public void emptyStruct() { assertProtoEquals(t.toProto(), "code: STRUCT struct_type {}"); } - // Suppressed for initial Error Prone rollout. - @SuppressWarnings("CheckReturnValue") @Test public void structFieldIndexNotFound() { Type t = Type.struct(StructField.of("f1", Type.int64())); @@ -551,8 +549,6 @@ public void structFieldIndexNotFound() { assertThat(e.getMessage().contains("Field not found: f2")); } - // Suppressed for initial Error Prone rollout. - @SuppressWarnings("CheckReturnValue") @Test public void structFieldIndexAmbiguous() { Type t = Type.struct(StructField.of("f1", Type.int64()), StructField.of("f1", Type.string())); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java index 0c5c7bc8717..e21e0020f6e 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java @@ -185,8 +185,6 @@ private static void appendProtoTypes(List types, Dialect dialect) { } } - // Suppressed for initial Error Prone rollout. - @SuppressWarnings({"ProtoBuilderReturnValueIgnored", "ReturnValueIgnored"}) public static ResultSetMetadata generateAllTypesMetadata(Type[] types) { StructType.Builder rowTypeBuilder = StructType.newBuilder(); for (int col = 0; col < types.length; col++) { diff --git a/pom.xml b/pom.xml index a19cba0e855..548f50e3e78 100644 --- a/pom.xml +++ b/pom.xml @@ -151,7 +151,8 @@ true -XDcompilePolicy=simple - -Xplugin:ErrorProne + -Xplugin:ErrorProne -Xep:CheckReturnValue:OFF \ + -Xep:ProtoBuilderReturnValueIgnored:OFF -Xep:ReturnValueIgnored:OFF -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED From 1e8f828b0edac7a01a3428891a01e16659294e74 Mon Sep 17 00:00:00 2001 From: Camille Schneider Date: Mon, 3 Feb 2025 12:06:07 +0000 Subject: [PATCH 3/4] fix errorprone compiler flag multi-line arg for Windows --- pom.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 548f50e3e78..74e28db0f91 100644 --- a/pom.xml +++ b/pom.xml @@ -151,8 +151,7 @@ true -XDcompilePolicy=simple - -Xplugin:ErrorProne -Xep:CheckReturnValue:OFF \ - -Xep:ProtoBuilderReturnValueIgnored:OFF -Xep:ReturnValueIgnored:OFF + -Xplugin:ErrorProne -Xep:CheckReturnValue:OFF -Xep:ProtoBuilderReturnValueIgnored:OFF -Xep:ReturnValueIgnored:OFF -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED From 826d6bb82bbfff44487009248781c428573e77a3 Mon Sep 17 00:00:00 2001 From: Camille Schneider Date: Tue, 29 Apr 2025 15:46:03 +0000 Subject: [PATCH 4/4] Update ErrorProne version --- .../java/com/google/cloud/spanner/AbstractReadContext.java | 2 ++ pom.xml | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index ea877c2c70e..b58aa28c6d9 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -458,6 +458,8 @@ void initTransaction() { } } + // Suppressed for initial Error Prone rollout. + @SuppressWarnings("GuardedBy") private void initTransactionInternal(BeginTransactionRequest request) { try { Transaction transaction = diff --git a/pom.xml b/pom.xml index 74e28db0f91..6e9b6340f00 100644 --- a/pom.xml +++ b/pom.xml @@ -54,7 +54,7 @@ UTF-8 github google-cloud-spanner-parent - 2.31.0 + 2.38.0 @@ -151,7 +151,8 @@ true -XDcompilePolicy=simple - -Xplugin:ErrorProne -Xep:CheckReturnValue:OFF -Xep:ProtoBuilderReturnValueIgnored:OFF -Xep:ReturnValueIgnored:OFF + --should-stop=ifError=FLOW + -Xplugin:ErrorProne -Xep:CheckReturnValue:OFF -Xep:ProtoBuilderReturnValueIgnored:OFF -Xep:ReturnValueIgnored:OFF -Xep:BoxedPrimitiveEquality:OFF -Xep:UnicodeInCode:OFF -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED