Implement #390

[WarpspeedSCP]

I've implemented #390 as a detector, DontAssertInstanceofInTests.

This only covers usages of Assert.assertTrue(obj instanceof Class) which are present in the body of an @Test annotated method.

I would like to add usages such as assertEquals(true, <instanceof expression>) but I am not completely sure of the best way to go about it.

• Added an entry into CHANGELOG.md

[KengoTODA]

Format your code by running ./gradlew spotlessApply. I will check more with my IDE later.

[KengoTODA]

If the purpose is skipping sawOpcode(int), it could be better to implement beforeOpcode(int) instead.

[WarpspeedSCP]

does beforeOpcode control if we want to skip processing an entire method? The intention was to avoid processing any methods that weren't annotated as tests.

[KengoTODA]

I think so. The javadoc describes its responsibility as return false if we should skip calling sawOpcode, and the implementation is like below:

spotbugs/spotbugs/src/main/java/edu/umd/cs/findbugs/visitclass/DismantleBytecode.java

Lines 877 to 879 in c195f7e

	if (beforeOpcode(opcode)) {
	sawOpcode(opcode);
	}

[WarpspeedSCP]

From my understanding, beforeOpcode would run on every opcode of a method.

Now that I look, there's a shouldVisitCode method in BytecodeScanningDetector which would do exactly what I was trying to do, so I'll just override that instead of that visitCode hack.

It appears that OpcodeStackDetector.visitCode uses BytecodeScanningDetector.shouldVisitCode :

spotbugs/spotbugs/src/main/java/edu/umd/cs/findbugs/bcel/OpcodeStackDetector.java

Lines 60 to 62 in c195f7e

	if (!shouldVisitCode(obj)) {
	return;
	}

so I changed my detector to be an OpcodeStackDetector instead.

[rydenius]

Following the advice of the documentation and using the instanceOf matcher, then proceeding the test by casting the object to that type in order to inspect it will then instead trigger a BC_UNCONFIRMED_CAST_OF_RETURN_VALUE error, something that does not happen after a regular instanceof call. So I need to introduce an exclude pattern for my test, or introduce a not that elegant if statement. Can you recommend a better way that was intended? I now feel that these two rules conflict a bit with each other.