Add filter for bytecode that javac generates for String in switch

[Godin]

I propose to do one more baby step in story of filtering - add filter for bytecode that javac generates for String in switch. And modify it for ECJ separately a bit later since it is a bit more tricky.

[marchof]

Beside minor naming issues I think this filter is not specific enough: It filters every switch on String.hashCode(), e.g.

	switch (s.hashCode()) { // $line-falsePositive.switch$
	case 42:
		nop(); // $line-falsePositive.case42$
	case 123:
		nop(); // $line-falsePositive.case123$
	}

I don't think it is very likely that we find such cases in real code but I think the filter could be more specific.

[marchof]

Should be called StringSwitchFilterTest as target is StringSwitchFilter. Or StringSwitchFilterJavacTest.

[Godin]

This explains why switching between code and test wasn't working in IDE 😆 Fixed.

[marchof]

As this filter is specific for javac we should call it StringSwitchJavacFilter.

[Godin]

Haven't used Javac prefix, thinking to update it for ECJ later, but you're right. Done.

[marchof]

@Godin BTW, +1 for moving on with baby steps, thanks!

[Godin]

I think this filter is not specific enough: It filters every switch on String.hashCode()
I don't think it is very likely that we find such cases in real code but I think the filter could be more specific.

Well, was about to do push-back and say "if unlikely, then why complexify? This will make step bigger and harder than baby". But actually making it more specific wasn't that hard thanks to previously implemented AbstractMatcher and implementation with it is quite nice. So lets say that I was lazy 😉
Please re-review.

[marchof]

Nice, thx!
Have two minor suggestions for the Matcher. Not mandatory though.

[marchof]

Couldn't that be nextIs(Opcodes.ICONST_M1) followed by if (cursor == null) return;?

[Godin]

@marchof Unfortunately no: nextIs first moves to next instruction and then checks, so can't be used to verify current instruction, and we can't place cursor before very first instruction of method.

[Godin]

@marchof this is kind of catch 22 - either we need to special case checking of beginning, or will have problem with end of methods, i.e. if we change semantic and cursor will be pointing to instruction for checking and moved after checking, then null can not be used to represent no-match as it is now, because next of last instruction is null. Maybe we can represent no-match by a special non-null value, but will be better to do this separately from this PR as it requires change of semantics of next methods and cursor.

[Godin]

@marchof another option is to have fake instruction at the beginning:

private void moveBack(int num) {
	for (int i = 0; i < num; i++) {
		if (cursor.getPrevious() == null) {
			cursor = new InsnNode(Opcodes.NOP) {
				final AbstractInsnNode next = cursor;
				@Override
				public AbstractInsnNode getNext() {
					return next;
				}
			};
		} else {
			cursor = cursor.getPrevious();
		}
	}
}

But I doubt that this is a good idea in general and definitely too much of complexity if done only here, compared to current handling of beginning.

[marchof]

Would be nice to have a fluent API for this in Matcher

moveBack(4);

or something like this.

[Godin]

@marchof I would prefer to leave this as is and extract only if will be used somewhere else, otherwise IDE complains that value of parameter is always 4.

[marchof]

OK, then good to me!