Fix SIC_INNER_SHOULD_BE_STATIC_ANON violations (#789)

basil · web-flow · commit 9cbcc3d8bb95 · 2025-03-13T17:32:11.000-06:00
diff --git a/src/main/java/hudson/remoting/AbstractByteArrayCommandTransport.java b/src/main/java/hudson/remoting/AbstractByteArrayCommandTransport.java
@@ -3,6 +3,7 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.ObjectOutputStream;
+import java.util.Objects;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.jenkinsci.remoting.util.AnonymousClassWarnings;
@@ -56,22 +57,32 @@ public interface ByteArrayReceiver {
     @Override
     public final void setup(final Channel channel, final CommandReceiver receiver) {
         this.channel = channel;
-        setup(new ByteArrayReceiver() {
-            @Override
-            public void handle(byte[] payload) {
-                try {
-                    Command cmd = Command.readFrom(channel, payload);
-                    receiver.handle(cmd);
-                } catch (IOException | ClassNotFoundException e) {
-                    LOGGER.log(Level.WARNING, "Failed to construct Command in channel " + channel.getName(), e);
-                }
-            }
+        setup(new ByteArrayReceiverImpl(channel, receiver));
+    }
+
+    private static class ByteArrayReceiverImpl implements ByteArrayReceiver {
+        private final Channel channel;
+        private final CommandReceiver receiver;
+
+        public ByteArrayReceiverImpl(Channel channel, CommandReceiver receiver) {
+            this.channel = Objects.requireNonNull(channel);
+            this.receiver = Objects.requireNonNull(receiver);
+        }
 
-            @Override
-            public void terminate(IOException e) {
-                receiver.terminate(e);
+        @Override
+        public void handle(byte[] payload) {
+            try {
+                Command cmd = Command.readFrom(channel, payload);
+                receiver.handle(cmd);
+            } catch (IOException | ClassNotFoundException e) {
+                LOGGER.log(Level.WARNING, "Failed to construct Command in channel " + channel.getName(), e);
             }
-        });
+        }
+
+        @Override
+        public void terminate(IOException e) {
+            receiver.terminate(e);
+        }
     }
 
     @Override
diff --git a/src/main/java/hudson/remoting/Capability.java b/src/main/java/hudson/remoting/Capability.java
@@ -146,26 +146,32 @@ void writePreamble(OutputStream os) throws IOException {
      * Writes this capability to a stream.
      */
     private void write(OutputStream os) throws IOException {
-        try (ObjectOutputStream oos = new ObjectOutputStream(Channel.Mode.TEXT.wrap(os)) {
-            @Override
-            public void close() throws IOException {
-                flush();
-                // TODO: Cannot invoke the private clear() method, but GC well do it for us. Not worse than the original
-                // solution
-                // Here the code does not close the proxied stream OS on completion
-            }
-
-            @Override
-            protected void annotateClass(Class<?> c) throws IOException {
-                AnonymousClassWarnings.check(c);
-                super.annotateClass(c);
-            }
-        }) {
+        try (ObjectOutputStream oos = new ObjectOutputStreamImpl(Channel.Mode.TEXT.wrap(os))) {
             oos.writeObject(this);
             oos.flush();
         }
     }
 
+    private static class ObjectOutputStreamImpl extends ObjectOutputStream {
+        public ObjectOutputStreamImpl(OutputStream out) throws IOException {
+            super(out);
+        }
+
+        @Override
+        public void close() throws IOException {
+            flush();
+            // TODO: Cannot invoke the private clear() method, but GC well do it for us. Not worse than the original
+            // solution
+            // Here the code does not close the proxied stream OS on completion
+        }
+
+        @Override
+        protected void annotateClass(Class<?> c) throws IOException {
+            AnonymousClassWarnings.check(c);
+            super.annotateClass(c);
+        }
+    }
+
     /**
      * The opposite operation of {@link #write}.
      */
diff --git a/src/main/java/hudson/remoting/ChannelBuilder.java b/src/main/java/hudson/remoting/ChannelBuilder.java
@@ -19,6 +19,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
@@ -294,16 +295,24 @@ public ChannelBuilder withRoles(Role... roles) {
      * @since 2.47
      */
     public ChannelBuilder withRoles(final Collection<? extends Role> actual) {
-        return withRoleChecker(new RoleChecker() {
-            @Override
-            public void check(@NonNull RoleSensitive subject, @NonNull Collection<Role> expected) {
-                if (!actual.containsAll(expected)) {
-                    Collection<Role> c = new ArrayList<>(expected);
-                    c.removeAll(actual);
-                    throw new SecurityException("Unexpected role: " + c);
-                }
+        return withRoleChecker(new RoleCheckerImpl(actual));
+    }
+
+    private static class RoleCheckerImpl extends RoleChecker {
+        private final Collection<? extends Role> actual;
+
+        public RoleCheckerImpl(Collection<? extends Role> actual) {
+            this.actual = Objects.requireNonNull(actual);
+        }
+
+        @Override
+        public void check(@NonNull RoleSensitive subject, @NonNull Collection<Role> expected) {
+            if (!actual.containsAll(expected)) {
+                Collection<Role> c = new ArrayList<>(expected);
+                c.removeAll(actual);
+                throw new SecurityException("Unexpected role: " + c);
             }
-        });
+        }
     }
 
     private static boolean isCallableProhibitedByRequiredRoleCheck(Callable<?, ?> callable) {
@@ -332,32 +341,38 @@ private static boolean isCallableProhibitedByRequiredRoleCheck(Callable<?, ?> ca
      * @since 2.47
      */
     public ChannelBuilder withRoleChecker(final RoleChecker checker) {
-        return with(new CallableDecorator() {
-            @Override
-            public <V, T extends Throwable> Callable<V, T> userRequest(Callable<V, T> op, Callable<V, T> stem) {
-                try {
-                    RequiredRoleCheckerWrapper wrapped = new RequiredRoleCheckerWrapper(checker);
-                    stem.checkRoles(wrapped);
-                    if (wrapped.isChecked()) {
-                        LOGGER.log(
-                                Level.FINER, () -> "Callable " + stem.getClass().getName() + " checked roles");
-                    } else if (isCallableProhibitedByRequiredRoleCheck(stem)) {
-                        LOGGER.log(
-                                Level.INFO,
-                                () -> "Rejecting callable " + stem.getClass().getName()
-                                        + " for ignoring RoleChecker in #checkRoles, see https://www.jenkins.io/redirect/required-role-check");
-                        throw new SecurityException(
-                                "Security hardening prohibits the Callable implementation "
-                                        + stem.getClass().getName()
-                                        + " from ignoring RoleChecker, see https://www.jenkins.io/redirect/required-role-check");
-                    }
-                } catch (AbstractMethodError e) {
-                    checker.check(stem, Role.UNKNOWN); // not implemented, assume 'unknown'
-                }
+        return with(new CallableDecoratorImpl(checker));
+    }
+
+    private static class CallableDecoratorImpl extends CallableDecorator {
+        private final RoleChecker checker;
+
+        public CallableDecoratorImpl(RoleChecker checker) {
+            this.checker = Objects.requireNonNull(checker);
+        }
 
-                return stem;
+        @Override
+        public <V, T extends Throwable> Callable<V, T> userRequest(Callable<V, T> op, Callable<V, T> stem) {
+            try {
+                RequiredRoleCheckerWrapper wrapped = new RequiredRoleCheckerWrapper(checker);
+                stem.checkRoles(wrapped);
+                if (wrapped.isChecked()) {
+                    LOGGER.log(Level.FINER, () -> "Callable " + stem.getClass().getName() + " checked roles");
+                } else if (isCallableProhibitedByRequiredRoleCheck(stem)) {
+                    LOGGER.log(
+                            Level.INFO,
+                            () -> "Rejecting callable " + stem.getClass().getName()
+                                    + " for ignoring RoleChecker in #checkRoles, see https://www.jenkins.io/redirect/required-role-check");
+                    throw new SecurityException("Security hardening prohibits the Callable implementation "
+                            + stem.getClass().getName()
+                            + " from ignoring RoleChecker, see https://www.jenkins.io/redirect/required-role-check");
+                }
+            } catch (AbstractMethodError e) {
+                checker.check(stem, Role.UNKNOWN); // not implemented, assume 'unknown'
             }
-        });
+
+            return stem;
+        }
     }
 
     /**
diff --git a/src/main/java/hudson/remoting/LocalChannel.java b/src/main/java/hudson/remoting/LocalChannel.java
@@ -24,6 +24,7 @@
 package hudson.remoting;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
+import java.util.Objects;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -58,33 +59,41 @@ public <V, T extends Throwable> Future<V> callAsync(@NonNull final Callable<V, T
             }
         });
 
-        return new Future<>() {
-            @Override
-            public boolean cancel(boolean mayInterruptIfRunning) {
-                return f.cancel(mayInterruptIfRunning);
-            }
+        return new FutureImpl<>(f);
+    }
 
-            @Override
-            public boolean isCancelled() {
-                return f.isCancelled();
-            }
+    private static class FutureImpl<V> implements Future<V> {
+        private final java.util.concurrent.Future<V> f;
 
-            @Override
-            public boolean isDone() {
-                return f.isDone();
-            }
+        public FutureImpl(java.util.concurrent.Future<V> f) {
+            this.f = Objects.requireNonNull(f);
+        }
 
-            @Override
-            public V get() throws InterruptedException, ExecutionException {
-                return f.get();
-            }
+        @Override
+        public boolean cancel(boolean mayInterruptIfRunning) {
+            return f.cancel(mayInterruptIfRunning);
+        }
 
-            @Override
-            public V get(long timeout, @NonNull TimeUnit unit)
-                    throws InterruptedException, ExecutionException, TimeoutException {
-                return f.get(timeout, unit);
-            }
-        };
+        @Override
+        public boolean isCancelled() {
+            return f.isCancelled();
+        }
+
+        @Override
+        public boolean isDone() {
+            return f.isDone();
+        }
+
+        @Override
+        public V get() throws InterruptedException, ExecutionException {
+            return f.get();
+        }
+
+        @Override
+        public V get(long timeout, @NonNull TimeUnit unit)
+                throws InterruptedException, ExecutionException, TimeoutException {
+            return f.get(timeout, unit);
+        }
     }
 
     @Override
diff --git a/src/main/java/hudson/remoting/RemoteInputStream.java b/src/main/java/hudson/remoting/RemoteInputStream.java
diff --git a/src/spotbugs/excludesFilter.xml b/src/spotbugs/excludesFilter.xml
