Better handling of check expressions

openc3/lib/openc3/script/api_shared.rb

@@ -864,7 +864,18 @@
       # Note: We have to preserve the original 'value' variable because we're going to eval against it
       value_str = value.is_a?(String) ? "'#{value}'" : value
       with_value = "with value == #{value_str}"
-      if eval(string)
+
+      eval_is_valid = _check_eval_validity(value, comparison_to_eval)
+      unless eval_is_valid
+        message = "Invalid comparison for types"
+        if $disconnect
+          puts "ERROR: #{message}"
+        else
+          raise CheckError, message
+        end
+      end
+
+      if eval_is_valid && eval(string)
         puts "#{check_str} success #{with_value}"
       else
         message = "#{check_str} failed #{with_value}"
@@ -883,5 +894,23 @@
         raise e
       end
     end
+
+    def _check_eval_validity(value, comparison)
+      return true if comparison.nil? || comparison.empty?
+
+      begin
+        operator, operand = extract_operator_and_operand_from_comparison(comparison)
+      rescue RuntimeError, JSON::ParserError
+        # If we can't parse the operand, let the eval happen anyway
+        # It will raise an appropriate error (like NameError for undefined constants)
+        return true
+      end
+
+      if [">=", "<=", ">", "<"].include?(operator)
+        return false if value.nil? || operand.nil? || value.is_a?(Array) || operand.is_a?(Array)
+      end
+
+      return true
+    end
   end
 end

openc3/lib/openc3/script/extract.rb

@@ -15,6 +15,7 @@
 # This file may also be used under the terms of a commercial license
 # if purchased from OpenC3, Inc.
 
+require 'json'
 require 'openc3/utilities/store'
 
 module OpenC3
@@ -154,22 +155,54 @@
     end
 
     def extract_fields_from_check_text(text)
-      split_string = text.split
-      raise "ERROR: Check improperly specified: #{text}" if split_string.length < 3
+      target_name, packet_name, item_name, comparison = text.split(nil, 4) # Ruby: second split arg is max number of resultant elements
+      raise "ERROR: Check improperly specified: #{text}" if item_name.nil?
 
-      target_name = split_string[0]
-      packet_name = split_string[1]
-      item_name = split_string[2]
-      comparison_to_eval = nil
-      return [target_name, packet_name, item_name, comparison_to_eval] if split_string.length == 3
-      raise "ERROR: Check improperly specified: #{text}" if split_string.length < 4
+      # comparison is either nil, the comparison string, or an empty string.
+      # We need it to not be an empty string.
+      comparison = nil if comparison&.length == 0
+
+      operator, _ = comparison&.split(nil, 2)
+      raise "ERROR: Use '==' instead of '=': #{comparison}" if operator == "="
+
+      return [target_name, packet_name, item_name, comparison]
+    end
+
+    # Splits `check()` comparison expressions, e.g. "== 'foo bar'" becomes ["==", "foo bar"]
+    def extract_operator_and_operand_from_comparison(comparison)
+      valid_operators = ["==", "!=", ">=", "<=", ">", "<", "in"]
 
-      split_string = text.split(/ /) # Split on regex spaces to preserve spaces in comparison
-      index = split_string.rindex(item_name)
-      comparison_to_eval = split_string[(index + 1)..(split_string.length - 1)].join(" ")
-      raise "ERROR: Use '==' instead of '=': #{text}" if split_string[3] == '='
+      operator, operand = comparison.split(nil, 2) # Ruby: second split arg is max number of resultant elements
 
-      return [target_name, packet_name, item_name, comparison_to_eval]
+      if operand.nil?
+        # Don't allow operator without operand
+        raise "ERROR: Invalid comparison, must specify an operand: #{comparison}" if !operator.nil?
+        return [nil, nil]
+      end
+
+      raise "ERROR: Invalid operator: #{comparison}" unless valid_operators.include?(operator)
+
+      # Handle string operand: remove surrounding double/single quotes
+      if operand.match?(/^(['"])(.*)\1$/m) # Starts with single or double quote, and ends with matching quote
+        operand = operand[1..-2]
+        return [operator, operand]
+      end
+
+      # Handle other operand types
+      if operand == "nil"
+        operand = nil
+      elsif operand == "false"
+        operand = false
+      elsif operand == "true"
+        operand = true
+      else
+        begin
+          operand = JSON.parse(operand)
+        rescue JSON::ParserError
+          raise "ERROR: Unable to parse operand: #{operand}"
+        end
+      end
+      return [operator, operand]
     end
   end
 end

openc3/python/openc3/api/api_shared.py

@@ -9,6 +9,7 @@
 # This file may also be used under the terms of a commercial license
 # if purchased from OpenC3, Inc.
 
+import json
 import sys
 import time
 import traceback
@@ -19,6 +20,7 @@
 from openc3.utilities.extract import (
     extract_fields_from_check_text,
     extract_fields_from_tlm_text,
+    extract_operator_and_operand_from_comparison,
 )
 
 
@@ -965,16 +967,21 @@ def _check_eval(target_name, packet_name, item_name, comparison_to_eval, value):
     else:
         value_str = value
     with_value = f"with value == {value_str}"
+
     try:
-        if eval(string):
+        eval_is_valid = _check_eval_validity(value, comparison_to_eval)
+        if eval_is_valid and eval(string):
             print(f"{check_str} success {with_value}")
         else:
             message = f"{check_str} failed {with_value}"
             raise CheckError(message)
-    except NameError as error:
-        parts = error.args[0].split("'")
-        new_error = NameError(f"Uninitialized constant {parts[1]}. Did you mean '{parts[1]}' as a string?")
-        raise new_error from error
+    except (NameError, json.JSONDecodeError) as error:
+        if isinstance(error, NameError):
+            parts = error.args[0].split("'")
+            new_error = NameError(f"Uninitialized constant {parts[1]}. Did you mean '{parts[1]}' as a string?")
+            raise new_error from error
+        else:
+            raise
 
 
 def _frange(value):
@@ -986,6 +993,28 @@ def _frange(value):
         return value
 
 
+def _check_eval_validity(value, comparison):
+    if not comparison:
+        return True
+
+    try:
+        operator, operand = extract_operator_and_operand_from_comparison(comparison)
+    except (RuntimeError, json.JSONDecodeError):
+        # If we can't parse the operand, let the eval happen anyway
+        # It will raise an appropriate error (like NameError for undefined constants)
+        return True
+
+    if operator in [">=", "<=", ">", "<"] and (
+        value is None or operand is None or isinstance(value, list) or isinstance(operand, list)
+    ):
+        return False
+
+    # Ruby doesn't have the "in" operator
+    return not (
+        operator == "in" and (isinstance(operand, str) and not isinstance(value, str) or not isinstance(operand, list))
+    )
+
+
 # Interesting formatter to a specific number of significant digits:
 # https://stackoverflow.com/questions/3410976/how-to-round-a-number-to-significant-figures-in-python?rq=3
 # def format(value, sigfigs=9):

openc3/python/openc3/script/api_shared.py

@@ -9,6 +9,7 @@
 # This file may also be used under the terms of a commercial license
 # if purchased from OpenC3, Inc.
 
+import json
 import sys
 import time
 import traceback
@@ -19,6 +20,7 @@
 from openc3.utilities.extract import (
     extract_fields_from_check_text,
     extract_fields_from_tlm_text,
+    extract_operator_and_operand_from_comparison,
 )
 
 from .exceptions import CheckError
@@ -1046,8 +1048,16 @@ def _check_eval(target_name, packet_name, item_name, comparison_to_eval, value):
     else:
         value_str = value
     with_value = f"with value == {value_str}"
+
+    eval_is_valid = _check_eval_validity(value, comparison_to_eval)
+    if not eval_is_valid:
+        message = "Invalid comparison for types"
+        if openc3.script.DISCONNECT:
+            print(f"ERROR: {message}")
+        else:
+            raise CheckError(message)
     try:
-        if eval(string):
+        if eval_is_valid and eval(string):
             print(f"{check_str} success {with_value}")
         else:
             message = f"{check_str} failed {with_value}"
@@ -1070,6 +1080,28 @@ def _frange(value):
         return value
 
 
+def _check_eval_validity(value, comparison):
+    if not comparison:
+        return True
+
+    try:
+        operator, operand = extract_operator_and_operand_from_comparison(comparison)
+    except (RuntimeError, json.JSONDecodeError):
+        # If we can't parse the operand, let the eval happen anyway
+        # It will raise an appropriate error (like NameError for undefined constants)
+        return True
+
+    if operator in [">=", "<=", ">", "<"] and (
+        value is None or operand is None or isinstance(value, list) or isinstance(operand, list)
+    ):
+        return False
+
+    # Ruby doesn't have the "in" operator
+    return not (
+        operator == "in" and (isinstance(operand, str) and not isinstance(value, str) or not isinstance(operand, list))
+    )
+
+
 # Interesting formatter to a specific number of significant digits:
 # https://stackoverflow.com/questions/3410976/how-to-round-a-number-to-significant-figures-in-python?rq=3
 # def format(value, sigfigs=9):

openc3/python/openc3/utilities/extract.py

@@ -14,14 +14,16 @@
 # if purchased from OpenC3, Inc.
 
 import ast
+import json
 import re
 
 
 SCANNING_REGULAR_EXPRESSION = re.compile(
-    r"(?:\"(?:[^\\\"]|\\.)*\") | (?:'(?:[^\\']|\\.)*') | (?:\[.*\]) | \S+", re.VERBOSE
+    r"(?:\"(?:[^\\\"]|\\.)*\") | (?:'(?:[^\\']|\\.)*') | (?:\[(?:[^\\\[\]]|\\.)*\]) | \S+", re.VERBOSE
 )
 
 SPLIT_WITH_REGEX = re.compile(r"\s+with\s+", re.IGNORECASE)
+SPLIT_WITH_OPTIONAL_WHITESPACE_REGEX = re.compile(r"\s*with\s*", re.IGNORECASE)
 
 # Regular expression to identify a String as a floating point number
 FLOAT_CHECK_REGEX = re.compile(r"\A\s*[-+]?\d*\.\d+\s*\Z")
@@ -135,8 +137,12 @@
 
 
 def extract_fields_from_cmd_text(text):
-    split_string = re.split(SPLIT_WITH_REGEX, text, maxsplit=2)
-    if len(split_string) == 1 and SPLIT_WITH_REGEX.match(text):
+    split_string = re.split(SPLIT_WITH_REGEX, text, maxsplit=1)  # 1 split, therefore 2 elements
+    if len(split_string) == 0 or split_string[0] == "":
+        raise RuntimeError("ERROR: text must not be empty")
+    if (len(split_string) == 1 and re.search(SPLIT_WITH_OPTIONAL_WHITESPACE_REGEX, text)) or (
+        len(split_string) == 2 and split_string[1] == ""
+    ):
         raise RuntimeError(f"ERROR: 'with' must be followed by parameters : {text:s}")
 
     # Extract target_name and cmd_name
@@ -182,7 +188,7 @@
 
 
 def extract_fields_from_tlm_text(text):
-    split_string = text.split(" ")
+    split_string = text.split()
     if len(split_string) != 3:
         raise RuntimeError(f"ERROR: Telemetry Item must be specified as 'TargetName PacketName ItemName' : {text}")
     target_name = split_string[0]
@@ -215,21 +221,58 @@
 
 
 def extract_fields_from_check_text(text):
-    split_string = text.split(" ")
-    if len(split_string) < 3:
-        raise RuntimeError(f"ERROR: Check improperly specified: {text}")
-    target_name = split_string[0]
-    packet_name = split_string[1]
-    item_name = split_string[2]
-    comparison_to_eval = None
-    if len(split_string) == 3:
-        return [target_name, packet_name, item_name, comparison_to_eval]
-    if len(split_string) < 4:
+    fields_split = text.split(None, 3)  # Python: second split arg is max number of splits
+    if len(fields_split) < 3:
         raise RuntimeError(f"ERROR: Check improperly specified: {text}")
+    target_name, packet_name, item_name, *comparison = fields_split
+
+    # comparison is a list, guaranteed to be of length 0 or 1 because of the split 3 with the splat operator above.
+    # We need it to be either None or the comparison string.
+    if len(comparison):
+        comparison = comparison[0]
+    else:
+        comparison = None
+
+    if comparison and len(comparison):
+        operator, *_ = comparison.split(None, 1)
+        if operator == "=":
+            raise RuntimeError(f"ERROR: Use '==' instead of '=': {comparison}")
 
-    # TODO: Ruby version has additional code to split on regex spaces
-    comparison_to_eval = " ".join(split_string[3:])
-    if split_string[3] == "=":
-        raise RuntimeError(f"ERROR: Use '==' instead of '=': {text}")
+    return target_name, packet_name, item_name, comparison
 
-    return target_name, packet_name, item_name, comparison_to_eval
+
+# Splits `check()` comparison expressions, e.g. "== 'foo bar'" becomes ["==", "foo bar"]
+def extract_operator_and_operand_from_comparison(comparison):
+    valid_operators = ["==", "!=", ">=", "<=", ">", "<", "in"]
+
+    operator, operand = comparison.split(None, 1)  # Python: second split arg is max number of splits
+
+    if operand is None:
+        if operator is not None:
+            raise RuntimeError(f"ERROR: Invalid comparison, must specify an operand: {comparison}")
+        return [None, None]
+
+    if operator not in valid_operators:
+        raise RuntimeError(f"ERROR: Invalid operator: {comparison}")
+
+    # Handle string operand: remove surrounding double/single quotes
+    quote_match = re.match(
+        r"^(['\"])(.*)\1$", operand, re.DOTALL
+    )  # Starts with single or double quote, and ends with matching quote
+    if quote_match:
+        operand = quote_match.group(2)
+        return operator, operand
+
+    # Handle other operand types
+    if operand == "None":
+        operand = None
+    elif operand == "False":
+        operand = False
+    elif operand == "True":
+        operand = True
+    else:
+        try:
+            operand = json.loads(operand)
+        except json.JSONDecodeError as err:
+            raise RuntimeError(f"ERROR: Unable to parse operand: {operand}") from err
+    return operator, operand