SerializableIdiom does not reset correctly when analyzing a record or…

… an enum (#2962) * test: make the base sample class serializable so it is analyzed first * fix: skip fields analysis for records and enums
spotbugs · Apr 21, 2024 · 99d354c · 99d354c
1 parent 2b9acc2
commit 99d354c
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 ## Unreleased - 2024-??-??
 ### Fixed
 - Fix FP `SING_SINGLETON_GETTER_NOT_SYNCHRONIZED` with eager instances ([#2932]https://github.com/spotbugs/spotbugs/issues/2932)
+- Fix FP `SE_BAD_FIELD` for record fields ([#2935]https://github.com/spotbugs/spotbugs/issues/2935)
 
 ## 4.8.4 - 2024-04-07
 ### Fixed

diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2793Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2793Test.java
@@ -18,11 +18,15 @@ class Issue2793Test extends AbstractIntegrationTest {
     void testRecordSerialVersionUid() {
         performAnalysis("../java17/ghIssues/Issue2793.class",
                 "../java17/ghIssues/Issue2793$RecordWithoutSerialVersionUid.class",
-                "../java17/ghIssues/Issue2793$ExternalizableRecord.class");
+                "../java17/ghIssues/Issue2793$ExternalizableRecord.class",
+                "../java17/ghIssues/Issue2793$SerializableClass.class",
+                "../java17/ghIssues/Issue2793$YangLibModule.class");
 
-        assertBugCount("SE_NO_SERIALVERSIONID", 0);
+        assertBugCount("SE_NO_SERIALVERSIONID", 2);
 
         assertBugCount("SE_NO_SUITABLE_CONSTRUCTOR_FOR_EXTERNALIZATION", 0);
+
+        assertBugCount("SE_BAD_FIELD", 0);
     }
 
     private void assertBugCount(String type, int expectedCount) {

diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/SerializableIdiom.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/SerializableIdiom.java
@@ -650,6 +650,10 @@ public void sawOpcode(int seen) {
 
     @Override
     public void visit(Field obj) {
+        if (isEnum || isRecord) {
+            return;
+        }
+
         int flags = obj.getAccessFlags();
         String genericSignature = obj.getGenericSignature();
         if (genericSignature != null && genericSignature.startsWith("T")) {

diff --git a/spotbugsTestCases/src/java17/Issue2793.java b/spotbugsTestCases/src/java17/Issue2793.java
@@ -5,11 +5,11 @@
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
 import java.io.Serializable;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
 
-public class Issue2793 {
+// For issue #2935 we need a serializable class analyzed right before the YangLibModule record
+// Due to the ordering of class analysis, we're making this class serializable to reproduce the issue
+public class Issue2793 implements Serializable { // SE_NO_SERIALVERSIONID
 	public record RecordWithoutSerialVersionUid(int x, int y, String message) implements Serializable {
 
 	}
@@ -23,4 +23,17 @@ public void readExternal(ObjectInput in) throws IOException {
 
 		}
 	}
+
+	public static class SerializableClass implements Serializable { // SE_NO_SERIALVERSIONID
+		public Integer x;
+		public Integer y;
+	}
+
+	public record YangLibModule(SerializableClass identifier,
+								SerializableClass namespace,
+								Map<SerializableClass, YangLibModule> submodules,
+								Set<SerializableClass> features,
+								Set<SerializableClass> deviationModuleNames,
+								SerializableClass source) {
+	}
 }