Skip to content

Commit bb7b56b

Browse files
committed
Fix isAtomicField method arg type
1 parent 66095db commit bb7b56b

File tree

4 files changed

+78
-88
lines changed

4 files changed

+78
-88
lines changed

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/AtomicOperationsCombinedDetectorTest.java

+67-83
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111

1212
class AtomicOperationsCombinedDetectorTest extends AbstractIntegrationTest {
1313

14-
public static final String OPERATIONS_ARE_NOT_ATOMIC = "AT_COMBINED_ATOMIC_OPERATIONS_ARE_NOT_ATOMIC";
15-
public static final String NEEDS_SYNCHRONIZATION = "AT_ATOMIC_OPERATION_NEEDS_SYNCHRONIZATION";
14+
private static final String OPERATIONS_ARE_NOT_ATOMIC = "AT_COMBINED_ATOMIC_OPERATIONS_ARE_NOT_ATOMIC";
15+
private static final String NEEDS_SYNCHRONIZATION = "AT_ATOMIC_OPERATION_NEEDS_SYNCHRONIZATION";
1616

1717
@Test
1818
void testSafeSynchronizedList() {
@@ -31,26 +31,26 @@ void testSafeSynchronizedList() {
3131
@Test
3232
void testUnsafeSynchronizedList() {
3333
performAnalysis("atomicMethods/UnsafeSynchronizedList.class");
34-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 1);
35-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
36-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedList", "addAndPrintNumbers", 13);
34+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 1);
35+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
36+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedList", "addAndPrintNumbers", 13);
3737
}
3838

3939
@Test
4040
void testUnsafeSynchronizedListWithMultipleSync() {
4141
performAnalysis("atomicMethods/UnsafeSynchronizedListWithMultipleSync.class");
42-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 1);
43-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
44-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListWithMultipleSync", "addAndPrintNumbers", 32);
42+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 1);
43+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
44+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListWithMultipleSync", "addAndPrintNumbers", 32);
4545
}
4646

4747
@Test
4848
void testUnsafeSynchronizedListWithMethodInit() {
4949
performAnalysis("atomicMethods/UnsafeSynchronizedListWithMethodInit.class");
50-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 1);
51-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 1);
52-
assertBug(NEEDS_SYNCHRONIZATION, "UnsafeSynchronizedListWithMethodInit", "init", 12);
53-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListWithMethodInit", "addAndPrintNumbers", 17);
50+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 1);
51+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 1);
52+
assertBugInMethodAtLine(NEEDS_SYNCHRONIZATION, "UnsafeSynchronizedListWithMethodInit", "init", 12);
53+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListWithMethodInit", "addAndPrintNumbers", 17);
5454
}
5555

5656
@Test
@@ -68,19 +68,19 @@ void testUnsafeSynchronizedSet() {
6868
"atomicMethods/UnsafeSynchronizedSet.class",
6969
"atomicMethods/UnsafeSynchronizedSortedSet.class",
7070
"atomicMethods/UnsafeSynchronizedNavigableSet.class");
71-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 3);
72-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
73-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSet", "addAndPrintNumbers", 13);
74-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSortedSet", "addAndPrintNumbers", 12);
75-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedNavigableSet", "addAndPrintNumbers", 12);
71+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 3);
72+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
73+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSet", "addAndPrintNumbers", 13);
74+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSortedSet", "addAndPrintNumbers", 12);
75+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedNavigableSet", "addAndPrintNumbers", 12);
7676
}
7777

7878
@Test
7979
void testUnsafeSynchronizedCollection() {
8080
performAnalysis("atomicMethods/UnsafeSynchronizedCollection.class");
81-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 1);
82-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
83-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedCollection", "addAndPrintNumbers", 12);
81+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 1);
82+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
83+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedCollection", "addAndPrintNumbers", 12);
8484
}
8585

8686
@Test
@@ -89,11 +89,11 @@ void testUnsafeSynchronizedMap() {
8989
"atomicMethods/UnsafeSynchronizedMap.class",
9090
"atomicMethods/UnsafeSynchronizedNavigableMap.class",
9191
"atomicMethods/UnsafeSynchronizedSortedMap.class");
92-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 3);
93-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
94-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedMap", "addAndPrintNumbers", 13);
95-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedNavigableMap", "addAndPrintNumbers", 12);
96-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSortedMap", "addAndPrintNumbers", 12);
92+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 3);
93+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
94+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedMap", "addAndPrintNumbers", 13);
95+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedNavigableMap", "addAndPrintNumbers", 12);
96+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSortedMap", "addAndPrintNumbers", 12);
9797
}
9898

9999
@Test
@@ -108,36 +108,36 @@ void testSynchronizedListInSynchronizedMethod() {
108108
@Test
109109
void testUnsafeSynchronizedListAndSet() {
110110
performAnalysis("atomicMethods/UnsafeSynchronizedListAndSet.class");
111-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 2);
112-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
113-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListAndSet", "addAndPrintNumbers", 16);
114-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListAndSet", "addAndPrintNumbers", 20);
111+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 2);
112+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
113+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListAndSet", "addAndPrintNumbers", 16);
114+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListAndSet", "addAndPrintNumbers", 20);
115115
}
116116

117117
@Test
118118
void testPartialSynchronizedListAndSet() {
119119
performAnalysis("atomicMethods/PartialSynchronizedListAndSet.class", "atomicMethods/PartialSynchronizedListAndSet2.class");
120-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 2);
121-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
122-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "PartialSynchronizedListAndSet", "addAndPrintNumbers", 16);
123-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "PartialSynchronizedListAndSet2", "addAndPrintNumbers", 22);
120+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 2);
121+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
122+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "PartialSynchronizedListAndSet", "addAndPrintNumbers", 16);
123+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "PartialSynchronizedListAndSet2", "addAndPrintNumbers", 22);
124124
}
125125

126126
@Test
127127
void testUnsafeSynchronizedListWithPrivateMethods() {
128128
performAnalysis("atomicMethods/UnsafeSynchronizedListWithPrivateMethods.class");
129-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 1);
130-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
131-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListWithPrivateMethods", "addAndPrintNumbers", 13);
129+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 1);
130+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
131+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedListWithPrivateMethods", "addAndPrintNumbers", 13);
132132
}
133133

134134
@Test
135135
void testUnsafeSynchronizedSetInMultipleMethods() {
136136
performAnalysis("atomicMethods/UnsafeSynchronizedSetInMultipleMethods.class");
137-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 2);
138-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
139-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSetInMultipleMethods", "addAndPrintNumbers", 13);
140-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSetInMultipleMethods", "removeAndPrintNumbers", 19);
137+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 2);
138+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
139+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSetInMultipleMethods", "addAndPrintNumbers", 13);
140+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedSetInMultipleMethods", "removeAndPrintNumbers", 19);
141141
}
142142

143143
@Test
@@ -149,10 +149,10 @@ void testSafeSynchronizedMapIncrement() {
149149
@Test
150150
void testUnsafeSynchronizedMapIncrement() {
151151
performAnalysis("atomicMethods/UnsafeSynchronizedMapIncrement.class");
152-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 1);
153-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 1);
154-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedMapIncrement", "increment", 17);
155-
assertBug(NEEDS_SYNCHRONIZATION, "UnsafeSynchronizedMapIncrement", "getCount", 21);
152+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 1);
153+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 1);
154+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeSynchronizedMapIncrement", "increment", 17);
155+
assertBugInMethodAtLine(NEEDS_SYNCHRONIZATION, "UnsafeSynchronizedMapIncrement", "getCount", 21);
156156
}
157157

158158
@Test
@@ -170,29 +170,29 @@ void testUnsafeAtomicReference() {
170170
"atomicMethods/UnsafeAtomicReference4.class",
171171
"atomicMethods/UnsafeAtomicReference4$Inner.class",
172172
"atomicMethods/UnsafeAtomicReference5.class");
173-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 11);
174-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 3);
175-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference", "update", 17);
176-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference", "add", 22);
177-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference2", "incrementAtomicInteger", 18);
178-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference2", "incrementAtomicInteger", 18);
179-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference2", "incrementAtomicInteger2", 26);
180-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference2", "incrementAtomicInteger2", 26);
181-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference3", "preparation", 11);
182-
assertBug(NEEDS_SYNCHRONIZATION, "UnsafeAtomicReference3", "run", 15);
183-
assertBug(NEEDS_SYNCHRONIZATION, "UnsafeAtomicReference3", "after", 19);
184-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference4", "initProcessing", 22);
185-
assertBug(NEEDS_SYNCHRONIZATION, "UnsafeAtomicReference4", "finishProcessing", 33);
186-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference5", "lambda$incrementAtomicInteger$0", 15);
173+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 11);
174+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 3);
175+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference", "update", 17);
176+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference", "add", 22);
177+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference2", "incrementAtomicInteger", 18);
178+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference2", "incrementAtomicInteger", 18);
179+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference2", "incrementAtomicInteger2", 26);
180+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference2", "incrementAtomicInteger2", 26);
181+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference3", "preparation", 11);
182+
assertBugInMethodAtLine(NEEDS_SYNCHRONIZATION, "UnsafeAtomicReference3", "run", 15);
183+
assertBugInMethodAtLine(NEEDS_SYNCHRONIZATION, "UnsafeAtomicReference3", "after", 19);
184+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference4", "initProcessing", 22);
185+
assertBugInMethodAtLine(NEEDS_SYNCHRONIZATION, "UnsafeAtomicReference4", "finishProcessing", 33);
186+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReference5", "lambda$incrementAtomicInteger$0", 15);
187187
}
188188

189189
@Test
190190
void testUnsafeAtomicReferenceValueSet() {
191191
performAnalysis("atomicMethods/UnsafeAtomicReferenceValueSet.class");
192-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 1);
193-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 1);
194-
assertBug(NEEDS_SYNCHRONIZATION, "UnsafeAtomicReferenceValueSet", "init", 10);
195-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReferenceValueSet", "update", 15);
192+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 1);
193+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 1);
194+
assertBugInMethodAtLine(NEEDS_SYNCHRONIZATION, "UnsafeAtomicReferenceValueSet", "init", 10);
195+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeAtomicReferenceValueSet", "update", 15);
196196
}
197197

198198
@Test
@@ -204,9 +204,9 @@ void testSafeLocalAtomicInteger() {
204204
@Test
205205
void testUnsafeLocalAtomicInteger() {
206206
performAnalysis("atomicMethods/UnsafeLocalAtomicInteger.class");
207-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 1);
208-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
209-
assertBug(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeLocalAtomicInteger", "increment", 23);
207+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 1);
208+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
209+
assertBugInMethodAtLine(OPERATIONS_ARE_NOT_ATOMIC, "UnsafeLocalAtomicInteger", "increment", 23);
210210
}
211211

212212
@Test
@@ -234,23 +234,7 @@ void testSafeAtomicCallsWithLambda() {
234234
}
235235

236236
private void assertZeroBugs() {
237-
assertNumOfBugs(OPERATIONS_ARE_NOT_ATOMIC, 0);
238-
assertNumOfBugs(NEEDS_SYNCHRONIZATION, 0);
239-
}
240-
241-
private void assertNumOfBugs(String bugType, int num) {
242-
final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder()
243-
.bugType(bugType).build();
244-
assertThat(getBugCollection(), containsExactly(num, bugTypeMatcher));
245-
}
246-
247-
private void assertBug(String bugType, String className, String methodName, int line) {
248-
final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
249-
.bugType(bugType)
250-
.inClass(className)
251-
.inMethod(methodName)
252-
.atLine(line)
253-
.build();
254-
assertThat(getBugCollection(), hasItem(bugInstanceMatcher));
237+
assertBugTypeCount(OPERATIONS_ARE_NOT_ATOMIC, 0);
238+
assertBugTypeCount(NEEDS_SYNCHRONIZATION, 0);
255239
}
256240
}

spotbugs/src/main/java/edu/umd/cs/findbugs/detect/AtomicOperationsCombinedDetector.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
import edu.umd.cs.findbugs.ba.CFG;
5151
import edu.umd.cs.findbugs.ba.CFGBuilderException;
5252
import edu.umd.cs.findbugs.ba.ClassContext;
53-
import edu.umd.cs.findbugs.ba.ClassMember;
5453
import edu.umd.cs.findbugs.ba.DataflowAnalysisException;
5554
import edu.umd.cs.findbugs.ba.Location;
5655
import edu.umd.cs.findbugs.ba.OpcodeStackScanner;
@@ -152,12 +151,12 @@ private void collectFieldsForAtomicityCheck(ClassContext classContext, Method me
152151
}
153152
}
154153

155-
private static boolean isAtomicField(ClassMember classMember) {
156-
if (classMember == null) {
154+
private static boolean isAtomicField(XMethod xMethod) {
155+
if (xMethod == null) {
157156
return false;
158157
}
159-
return CollectionAnalysis.isSynchronizedCollection(classMember)
160-
|| (classMember.getClassName().startsWith("java.util.concurrent.atomic") && classMember.getSignature().endsWith(")V"));
158+
return CollectionAnalysis.isSynchronizedCollection(xMethod)
159+
|| (xMethod.getClassName().startsWith("java.util.concurrent.atomic") && xMethod.getSignature().endsWith(")V"));
161160
}
162161

163162
private void analyzeFieldsForAtomicityViolations(ClassContext classContext, Method method) throws CheckedAnalysisException {

spotbugs/src/main/java/edu/umd/cs/findbugs/util/CollectionAnalysis.java

+3
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
*/
3030
public final class CollectionAnalysis {
3131

32+
/**
33+
* Private constructor to prevent instantiation, because it is a utility class.
34+
*/
3235
private CollectionAnalysis() {
3336
}
3437

spotbugs/src/main/java/edu/umd/cs/findbugs/util/MethodAnalysis.java

+4
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,15 @@
2828
*/
2929
public final class MethodAnalysis {
3030

31+
/**
32+
* Private constructor to prevent instantiation, because it is a utility class.
33+
*/
3134
private MethodAnalysis() {
3235
}
3336

3437
/**
3538
* Check if the location is duplicated in the method.
39+
* Locations in finally blocks will be duplicated in the bytecode level.
3640
*
3741
* @return {@code true} if the location is duplicated in the method, {@code false} otherwise
3842
* @throws CheckedAnalysisException if an error occurs during the analysis

0 commit comments

Comments
 (0)