Rule parser chokes on complex boolean expressions

Setup, Configuration and Use Scripts & Rules
, ,
1

I’m currently running 2.5.0-SNAPSHOT (Build #1601) as I needed a recent build of the Z-Wave binding for my home configuration.

I noticed that the rule parser chokes on the following boolean expression:

if ( ( (intState >= 0) && (intPrevState >= 0) && (intState != intPrevState) ) || (stateStr != prevStateStr) ) {
	sendNotification(notificationRecipient, ruleTitle + ": " + logLine)
}

According to the Xtend documentation on infix operators this is a valid expression.

There’s no way I could avoid the rule engine reject processing the rule due to “errors”. Here are a couple errors reported on the line containing the boolean expression:

2019-06-05 18:10:46.469 [WARN ] [el.core.internal.ModelRepositoryImpl] - Configuration model 'roller-shutter-new.rules' has errors, therefore ignoring it: [318,80]: no viable alternative at input ' '
[318,81]: missing ')' at '('
[318,107]: mismatched input ')' expecting ']'
[323,2]: extraneous input ']' expecting 'end'

2019-06-05 18:11:53.907 [WARN ] [el.core.internal.ModelRepositoryImpl] - Configuration model 'roller-shutter-new.rules' has errors, therefore ignoring it: [318,71]: no viable alternative at input '||'
[318,73]: no viable alternative at input ' '

It appears that the rule processor dislikes lines that contain boolean expressions that combine multiple (space-separated) bracket levels, logical && (AND) and logical || (OR)…

If I rewrite this valid boolean expression as follows, it works without complaining:

if (intState >= 0 && intPrevState >= 0 && intState != intPrevState) {
	sendNotification(notificationRecipient, ruleTitle + ": " + logLine)
} else if (stateStr != prevStateStr) {
	sendNotification(notificationRecipient, ruleTitle + ": " + logLine)
}

I suppose this is a side effect of the particular way round brackets are used in the rules DSL?

2

I don’t think there have been any changes to the Xtend library in quite some time so it is unlikely that there has been a recent change that is causing this.

To simplify things try getting rid of all the extraneous parens. IMHO they only serve to make the line more difficult to read.

if ( ( intState >= 0 && intPrevState >= 0 && intState != intPrevState ) || stateStr != prevStateStr ) {

Does it complaint about that line?

Does VS Code complain about the line?

Which column is column 81? You don’t have any indentation so it’s impossible to tell.

Are you certain this is the actual line it’s complaining about? One of the errors is complaining about a ] and there is no ] in this line. Perhaps getting more context about the lines before this if statement would be useufl.

3

It’s not very clear how you’d get two different errors from the same line a minute apart? Expecting a square bracket ] is bit mysterious too.

This copy-paste of your suspect line loads and runs as expected in OH 2.4 with no warnings.

	var Number intState = 22
	var Number intPrevState = 33
	var String stateStr = "banana"
	var String prevStateStr = "rhubarb"
	if ( ( (intState >= 0) && (intPrevState >= 0) && (intState != intPrevState) ) || (stateStr != prevStateStr) ) {
		logInfo("test", "here")
	} else {
		logInfo("test", "there")
	}

I’d suspect something amiss with bracketry in some earlier line

4

VS Code never complained.

The error about the missing closing square bracket is a result of a lambda iterator:

gShutter.allMembers.forEach[ s |
    // code comes here
]

Here’s another unsuccessful attempt:

//       10        20        30        40        50        60        70        80        90       100
//       v         v         v         v         v         v         v         v         v         v
if (intState >= 0 && intPrevState >= 0 && intState != intPrevState) || (stateStr != prevStateStr) {
	logInfo(ruleTitle, "Test of long boolean expression - IF clause")
} else {
	logInfo(ruleTitle, "Test of long boolean expression - ELSE clause")
}

And the related errors tell me that the rule parser dislikes the ‘||’ (column 69) and then complains about the space after the ‘||’ (column 71). The forEach[ | ] iterator ends on line 353:

2019-06-06 09:32:58.893 [WARN ] [el.core.internal.ModelRepositoryImpl] - Configuration model 'roller-shutter-new.rules' has errors, therefore ignoring it: [340,69]: no viable alternative at input '||'
[340,71]: no viable alternative at input ' '
[342,3]: mismatched input 'else' expecting ']'
[353,2]: extraneous input ']' expecting 'end'

However your edit does seem to work without errors:

//       10        20        30        40        50        60        70        80        90       100
//       v         v         v         v         v         v         v         v         v         v
if ( ( intState >= 0 && intPrevState >= 0 && intState != intPrevState ) || stateStr != prevStateStr ) {
	logInfo(ruleTitle, "Test of long boolean expression - IF clause")
} else {
	logInfo(ruleTitle, "Test of long boolean expression - ELSE clause")
}

I decided to run additional tests:

//       10        20        30        40        50        60        70        80        90       100
//       v         v         v         v         v         v         v         v         v         v
// 1. Works:
if ( ( (intState >= 0) && intPrevState >= 0 && intState != intPrevState ) || stateStr != prevStateStr ) {
// 2. Works, but meaningless (wrong placement of closing parenthesis):
if ( ( (intState >= 0 && intPrevState) >= 0 && intState != intPrevState ) || stateStr != prevStateStr ) {
// 3. Works:
if ( ( (intState >= 0) && (intPrevState >= 0) && intState != intPrevState ) || stateStr != prevStateStr ) {
// 4. Works:
if ( ( (intState >= 0) && (intPrevState >= 0) && (intState != intPrevState) ) || stateStr != prevStateStr ) {
// 5. Now works while yesterday it didn't:
if ( ( (intState >= 0) && (intPrevState >= 0) && (intState != intPrevState) ) || (stateStr != prevStateStr) ) {
// 6. Also works:
if ( ( ( intState >= 0 ) && ( intPrevState >= 0 ) && ( intState != intPrevState ) ) || ( stateStr != prevStateStr ) ) {

// 7. And the failing one at the top of this reply still fails:
if (intState >= 0 && intPrevState >= 0 && intState != intPrevState) || (stateStr != prevStateStr) {
// 8. As does this one:
if ( (intState >= 0 && intPrevState >= 0 && intState != intPrevState) || (stateStr != prevStateStr) ) {
// 9. But (see above), this one works:
if ( ( intState >= 0 && intPrevState >= 0 && intState != intPrevState ) || stateStr != prevStateStr ) {
// 10. So let's try this one... works as well:
if ( ( intState >= 0 && intPrevState >= 0 && intState != intPrevState ) || ( stateStr != prevStateStr ) ) {

In essence the error could not be reproduced.

5

Aha, be cautious about the errors coming out from inside lambdas - there is undoubtedly an error, but the cause reporting may be garbled.

A curiosity observed about operators like >= ; the rules engine seems to use the type of the preceding object to figure out what algorithm to use here, e.g. == for integer is not same as == for string is not same as == for decimal.
A long winded way to say that if() may have good syntax but blow up at runtime because of the variables used.

The most common cause of errors in a lambda is that some variable that you expected to be in scope, isn’t.

Okay, that one’s just bad syntax
if ( xx) || ( yy ) {

I can’t spot a problem with that.

I wonder if this is all being inconsistent because you are chasing some race or reentrant problem.

6

Indeed, they can be tricky to spot. Incidentally I fixed one yesterday in the same rule. The tell-tale sign that something went wrong is a thrown exception, such as:

2019-06-05 21:44:57.109 [ERROR] [org.quartz.core.JobRunShell         ] - Job DEFAULT.2019-06-05T21:44:57.098+02:00: Proxy for org.eclipse.xtext.xbase.lib.Procedures$Procedure0: [ | {
  logDebug(<XFeatureCallImplCustom>,<XBinaryOperationImplCustom>)
  sendCommand(<XFeatureCallImplCustom>,<XStringLiteralImpl>)
  <XFeatureCallImplCustom>.put(<XFeatureCallImplCustom>,<XNullLiteralImplCustom>)
} ] threw an unhandled Exception: 
java.lang.reflect.UndeclaredThrowableException: null
        at com.sun.proxy.$Proxy4125487.apply(Unknown Source) ~[?:?]
        at org.eclipse.smarthome.model.script.internal.actions.TimerExecutionJob.execute(TimerExecutionJob.java:49) ~[?:?]
        at org.quartz.core.JobRunShell.run(JobRunShell.java:202) [180:org.openhab.core.scheduler:2.5.0.201905250304]
        at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:573) [180:org.openhab.core.scheduler:2.5.0.201905250304]
Caused by: org.eclipse.smarthome.model.script.engine.ScriptExecutionException: The name 'shutter' cannot be resolved to an item or type; line 223, column 18, length 7

To fix it I’ve been looking at portions in my code that have a similar syntactic lay-out as:

org.eclipse.xtext.xbase.lib.Procedures$Procedure0: [ | {
  logDebug(<XFeatureCallImplCustom>,<XBinaryOperationImplCustom>)
  sendCommand(<XFeatureCallImplCustom>,<XStringLiteralImpl>)
  <XFeatureCallImplCustom>.put(<XFeatureCallImplCustom>,<XNullLiteralImplCustom>)
} ]

That was easy to find, and the reported missing variable error also confirmed the same spot:

			// Schedule to refresh the state 10 seconds after a STOP command
			timers.put(shutterName, createTimer(now.plusSeconds(10), [ |
					logDebug(ruleTitle, logLineStart + "scheduled state refresh runs NOW")
					sendCommand(shutter, "REFRESH")
					timers.put(shutterName, null)
				]))

That one was easy to fix (it was a missing variable after reintegrating code from a lambda procedure to the body of a rule):

	val GenericItem shutter = triggeringItem as GenericItem

That’s also what I thought, but the different variants of the boolean expression always compare items explicitly typed and of the same type, as in:

	var int currentStateInt = -1 // If current state is not an integer value, leave negative.
	var String currentStateStr = "" // Contains the string representation of the state, irrespective of the type
	if (triggeringItem.state instanceof DecimalType) {
		currentStateInt = (triggeringItem.state as Number).intValue // Set the int representation of the current state
		currentStateStr = currentStateInt.toString
	} else { // Current state does not represent a decimal (integer) value
		currentStateStr = triggeringItem.state.toString
	}

In my rule, I defined a boolean logic expression that checks whether:

  • (a) current and previous state of triggered item represent both integer values AND differ, or
  • (b) [optional due to fall-through of (a): either current of Previous state is not a numeric value AND] the string representations of current and previous state differ.
7

do int variable types have a .toString method?

8

Not in Java, but they do in Xtend (basis of the rules DSL): https://stackoverflow.com/questions/20355324/xtend-how-to-convert-integer-to-string

I suppose int in rules DSL is internally mapped to Java Integer objects.

9

I’m surprised 2 worked. Maybe Xtend is able to treat ints as booleans which is the only way I can think of that would allow the second case to work without an error.

You are missing opening and closing parens. The proper syntax for an if statement is

if(<boolean expression>)

This attempt is trying to do

if <boolean expression>

I’ve seen something like this before but it had to do with lambdas, not boolean expressions. When dealing with lambdas (i.e. any time you see [ ] like in forEach) you sometimes have to have a space after the [ and before the ]. I’ve never seen this reported for if statements though.