Avoid using Constants interface deprecated in BCEL 6.x

[sewe]

At the moment, the spotbugs project shows more than 1,800 warnings, 1,506 of which are caused by our use of the org.apache.bcel.Constants interface that has been deprecated with BCEL 6.0.

IMHO, we should switch to the BCEL 6.0 replacement, org.apache.bcel.Const, to get our list of warnings down to a manageable level.

This change would then come in two parts:

1. Use the constants in Const rather than Constants ourselves.
2. No longer implement Constants in our classes.

The first part is a purely internal change, which doesn’t affect our clients. But no longer implementing the Constants interface may affect them.

Now, the question is whether we strive to be binary compatible (old plugins should run on the new SpotBugs version) or also source compatible (old plug-ins should still compile against the new SpotBugs version) for SpotBugs 3.1.0.

Binary compatibility is easy: Most constants are primitive and inlined during compilation and the few constants which are not (OPCODE_NAMES, NO_OF_OPERANDS, …) are accessed using invokestatic. Hence, existing clients which extend a SpotBugs class that previously implemented Constants would just continue to use invokestatic to access Constants.*; they wouldn’t notice that they no longer (transitively) implement the interface. This leaves only the oddball case of someone doing an instanceof Constants, which I am confident nobody did (because: why?).

Source compatibility is probably broken, as I suspect not everyone who extends, e.g., BytecodeScanningDetector adds an implements Constants clause to their detector when accessing one of the constants themselves.

Now the question: Should we do both parts of this change for SpotBugs 3.1.0, even though part 2 breaks source compatibility, or should we leave part 2 to SpotBugs 4.0.0 and only do part 1 for 3.1.0? (FYI, part 1 would already get rid of most of the deprecation warnings.)

Thoughts?

[iloveeclipse]

Without looking into the code: 3.1.0 release should be compatible as much as we can (to simplify transition from FB to SB), 4.0.0 is by definition "open" for new features and API can be broken (not that this is nice, but where we can't avoid this, we can't keep compatibility).

[sewe]

@iloveeclipse I take “compatible as much as we can” to mean source compatible, so plug-ins that compiled against FindBugs 3.0.x should compile against SpotBugs 3.1.0. If that’s the case, I’ll prepare a change for part 1 only.

This would get rid of most of the warnings already, without affecting compatibility in any way. I would also add a deprecation note to our change log, so that detector writers will know that they can’t rely on Constants being implemented for them forever.

[iloveeclipse]

OK, Since part 1 is merged, I've changed the target to 4.0.

[sewe]

OK, Since part 1 is merged, I've changed the target to 4.0.

Makes sense.

[ThrawnCA]

There are still usages of the Constants interface:

grep -rl 'bcel\.Constants\b' spotbugs*

spotbugs/src/main/java/edu/umd/cs/findbugs/CheckBcel.java
spotbugs/src/main/java/edu/umd/cs/findbugs/visitclass/Constants2.java
spotbugs/src/main/java/edu/umd/cs/findbugs/ba/BytecodeScanner.java
spotbugs/src/main/java/edu/umd/cs/findbugs/ba/type/ExceptionObjectType.java
spotbugs/src/main/java/edu/umd/cs/findbugs/ba/type/TypeFrameModelingVisitor.java
spotbugs/src/main/java/edu/umd/cs/findbugs/ba/type/StandardTypeMerger.java
spotbugs/src/main/java/edu/umd/cs/findbugs/ba/AssignedFieldMap.java
spotbugs/src/main/java/edu/umd/cs/findbugs/ba/AssertionMethods.java
spotbugs/src/main/java/edu/umd/cs/findbugs/ba/bcp/FieldAccess.java
spotbugs/src/tools/edu/umd/cs/findbugs/tools/FilterAndCombineBitfieldPropertyDatabase.java
spotbugs/src/tools/edu/umd/cs/findbugs/tools/FilterPropertyDatabase.java
spotbugs/src/test/java/edu/umd/cs/findbugs/SAXBugCollectionHandlerTest.java

[KengoTODA]

I got different result on release-3.1 branch:

spotbugs/src/test/java/edu/umd/cs/findbugs/SAXBugCollectionHandlerTest.java
spotbugs/src/patches/bcel.diff
spotbugs/src/main/java/edu/umd/cs/findbugs/CheckBcel.java

Recently @wreulicke removed some usage, so I think we're really close to finish this issue.

[KengoTODA]

I confirmed that we have no implementation depends on this Const class.