GroovyBugError thrown when it shouldn't be

Originally reported in GRECLIPSE-1420.

Well, I guess there are several ways to make the problem go away. Firstly, the tilde isn't needed so either of these will also reproduce the problem:

print "Foo" === ".*"
print "Foo" !== ".*"

Both COMPARE_IDENTICAL and COMPARE_NOT_IDENTICAL were added as a hack for the benefit of Groovy++ but the code in BEH.eval assumes that all operators are covered within switch branches and the code wasn't updated when the hack was added. So we could consider using SyntaxException but it might be nicer to either drop support for those two operators or add proper support in. As an interim measure, we should at least add branches for those two cases - which could just throw the SyntaxException for now.

Proposed interim patch:

@@ -241,6 +244,12 @@ public class BinaryExpressionHelper {
             evaluateCompareExpression(isCaseMethod, expression);
             break;

+        case COMPARE_IDENTICAL:
+        case COMPARE_NOT_IDENTICAL:
+            Token op = expression.getOperation();
+            Throwable cause = new SyntaxException("Operator " + op + " not supported", op.getStartLine(), op.getStartColumn());
+            throw new GroovyRuntimeException(cause);
+
         default:
             throw new GroovyBugError("Operation: " + expression.getOperation() + " not supported");
         }

For the folks in CPH, perhaps the longer term solution can be discussed at the GDC if you read this before you are finished?

I also wonder whether the RuntimeException throw in evaluateInstanceOf should also be a wrapped SyntaxException for consistency? However, I am not sure the grammar even allows this case any longer.

This looks fine to me. The situation in Groovy-Eclipse is workable now and it isn't relying on a fix in groovy-oore (although having one would be nice). Groovy-Eclipse now simply catches the GroovyBugError and adds it as a compiler warning as well as logging it appropriately. Much better, of course, would be if GBErrors aren't thrown unless there really is an internal compiler error.

Thanks for the quick turn around on this!

Andrew, this is an internal bug, thus the GroovyBugError is the right thing. It showed us the lacking implementation and this fix is the right reaction to it Whenever a GroovyBugError appears, there is something to be fixed.

Interim patch added to master.

I think we should add it to 1.8 as well

proposed interim patch applied