Give error if unit is used for branch or struct

Signed-off-by: Erik Jaegervall <[email protected]>

tests/vspec/test_datatypes_error/test.vspec

@@ -6,5 +6,5 @@ A:
 A.UInt7:
   datatype: uint7
   type: sensor
-  unit: km
   description: Oops, that datatype does not exist!
+  comment: If we specify unit as well then error message will be that non-standard datatypes cannot use unit

tests/vspec/test_datatypes_error/test_datatype_branch.vspec

@@ -0,0 +1,11 @@
+#
+A:
+  type: branch
+  description: Branch shall not have datatype
+  datatype: uint8
+
+A.UInt7:
+  datatype: uint8
+  type: sensor
+  unit: km
+  description: ok!

tests/vspec/test_datatypes_error/test_datatypes_error.py

@@ -32,3 +32,19 @@ def test_datatype_error(change_test_dir):
     os.system("rm -f out.json out.txt")
     assert os.WIFEXITED(result)
     assert os.WEXITSTATUS(result) == 0
+
+
+def test_datatype_branch(change_test_dir):
+    test_str = "../../../vspec2json.py --json-pretty -u ../test_units.yaml " \
+               "test_datatype_branch.vspec out.json > out.txt 2>&1"
+    result = os.system(test_str)
+    assert os.WIFEXITED(result)
+    # failure expected
+    assert os.WEXITSTATUS(result) != 0
+
+    test_str = 'grep \"cannot have datatype\" out.txt > /dev/null'
+    result = os.system(test_str)
+    os.system("cat out.txt")
+    os.system("rm -f out.json out.txt")
+    assert os.WIFEXITED(result)
+    assert os.WEXITSTATUS(result) == 0

tests/vspec/test_structs/VehicleDataTypesStructWithUnit.vspec

@@ -0,0 +1,26 @@
+VehicleDataTypes:
+  type: branch
+  description: Top-level branch for vehicle data types.
+
+VehicleDataTypes.TestBranch1:
+  description: "A branch"
+  type: branch
+
+VehicleDataTypes.TestBranch1.NestedStruct:
+  type: struct
+  description: "ERROR: A struct with unit defined."
+  unit: km
+
+VehicleDataTypes.TestBranch1.NestedStruct.x:
+  type: property
+  description: "x property"
+  datatype: double
+
+VehicleDataTypes.TestBranch1.ParentStruct:
+  type: struct
+  description: "A struct that is going to contain properties that are structs themselves"
+
+VehicleDataTypes.TestBranch1.ParentStruct.x_property:
+  type: property
+  description: "A property of struct-type. The struct name is specified relative to the branch"
+  datatype: NestedStruct

tests/vspec/test_structs/test-invalid-datatypes.vspec

@@ -12,11 +12,9 @@ A.UInt8:
 A.ParentStructSensor:
   datatype: VehicleDataTypes.TestBranch1.ParentStruct1
   type: sensor
-  unit: km
   description: A rich sensor with user-defined data type.
 
 A.NestedStructSensor:
   datatype: VehicleDataTypes.TestBranch1.NestedStruct1
   type: sensor
-  unit: km
   description: A rich sensor with user-defined data type.

tests/vspec/test_structs/test.vspec

@@ -12,11 +12,9 @@ A.UInt8:
 A.ParentStructSensor:
   datatype: VehicleDataTypes.TestBranch1.ParentStruct
   type: sensor
-  unit: km
   description: A rich sensor with user-defined data type.
 
 A.NestedStructSensor:
   datatype: VehicleDataTypes.TestBranch1.NestedStruct
   type: sensor
-  unit: km
   description: A rich sensor with user-defined data type.

tests/vspec/test_structs/test2.vspec

@@ -12,11 +12,9 @@ A.UInt8:
 A.ParentStructSensor:
   datatype: VehicleDataTypes.TestBranch2.ParentStruct
   type: sensor
-  unit: km
   description: A rich sensor with user-defined data type.
 
 A.NestedStructSensor:
   datatype: VehicleDataTypes.TestBranch3.NestedStruct
   type: sensor
-  unit: km
   description: A rich sensor with user-defined data type.

tests/vspec/test_structs/test_data_type_parsing.py

@@ -224,19 +224,25 @@ def test_error_when_no_user_defined_data_types_are_provided(change_test_dir):
     assert os.WEXITSTATUS(result) == 0
 
 
-def test_warning_when_data_type_is_provided_for_struct_nodes(change_test_dir):
-    """
-    Test that warning message is provided when datatype is specified for struct nodes.
+@pytest.mark.parametrize("vspec_file,types_file,error_msg", [
+    ('test.vspec', 'VehicleDataTypesStructWithDataType.vspec',
+     'cannot have datatype, only allowed for signal and property'),
+    ('test.vspec', 'VehicleDataTypesStructWithUnit.vspec',
+     'cannot have unit'),
+    ('test_with_unit_on_struct_signal.vspec', 'VehicleDataTypes.vspec',
+     'Unit specified for item not using standard datatype')])
+def test_faulty_use_of_standard_attributes(
+        vspec_file, types_file, error_msg, change_test_dir):
+    """
+    Test faulty use of datatype and unit for structs
     """
     test_str = " ".join(["../../../vspec2json.py", "-u", "../test_units.yaml", "--format", "json",
-                         "--json-pretty", "-vt",
-                         "VehicleDataTypesStructWithDataType.vspec", "-ot", "VehicleDataTypes.json", "test.vspec",
+                         "--json-pretty", "-vt", types_file, "-ot", "VehicleDataTypes.json", vspec_file,
                          "out.json", "1>", "out.txt", "2>&1"])
     result = os.system(test_str)
     assert os.WIFEXITED(result)
-    assert os.WEXITSTATUS(result) == 0
+    assert os.WEXITSTATUS(result) != 0
 
-    error_msg = 'Data type specified for struct node: NestedStruct. Ignoring it'
     test_str = f'grep \"{error_msg}\" out.txt > /dev/null'
     result = os.system(test_str)
     os.system("cat out.txt")

tests/vspec/test_structs/test_with_unit_on_struct_signal.vspec

@@ -0,0 +1,21 @@
+#
+A:
+  type: branch
+  description: Branch A.
+
+A.UInt8:
+  datatype: uint8
+  type: sensor
+  unit: km
+  description: A uint8.
+
+A.ParentStructSensor:
+  datatype: VehicleDataTypes.TestBranch1.ParentStruct
+  type: sensor
+  unit: km
+  description: A rich sensor with user-defined data type.
+
+A.NestedStructSensor:
+  datatype: VehicleDataTypes.TestBranch1.NestedStruct
+  type: sensor
+  description: A rich sensor with user-defined data type.

tests/vspec/test_units/test_units.py

@@ -96,8 +96,12 @@ def test_unit_error_no_unit_file(change_test_dir):
 def test_unit_error_unit_file_incomplete(change_test_dir):
     run_unit_error("signals_with_special_units.vspec", "-u units_hogshead.yaml", "Unknown unit")
 
-# FIle not found
+# File not found
 
 
 def test_unit_error_missing_file(change_test_dir):
     run_unit_error("signals_with_special_units.vspec", "-u file_that_does_not_exist.yaml", "FileNotFoundError")
+
+
+def test_unit_on_branch(change_test_dir):
+    run_unit_error("unit_on_branch.vspec", "-u units_all.yaml", "cannot have unit")

tests/vspec/test_units/unit_on_branch.vspec

@@ -0,0 +1,10 @@
+A:
+  type: branch
+  description: Specifying unit on branch is not allowed!
+  unit: hogshead
+
+A.UInt8:
+  datatype: uint8
+  type: sensor
+  unit: hogshead
+  description: This description is mandatory!

vspec/model/vsstree.py

@@ -97,11 +97,6 @@ def __init__(self, name, source_dict: dict, available_types: Set[str], parent=No
             logging.error(f"Orphan property detected. {self.name} is not defined under a struct")
             sys.exit(-1)
 
-        if (self.is_signal() or self.is_property()) and "datatype" not in self.source_dict.keys():
-            raise IncompleteElementException(
-                (f"Incomplete element {self.name} from {self.source_dict['$file_name$']}: "
-                 f"Elements of type {self.type.value} need to have a datatype declared."))
-
         try:
             self.validate_name_style(self.source_dict["$file_name$"])
         except NameStyleValidationException as e:
@@ -129,24 +124,37 @@ def extractCoreAttribute(name: str):
         for attribute in VSSNode.core_attributes:
             extractCoreAttribute(attribute)
 
-        # Datatype and unit need special handling, so we extract them again
-        if "datatype" in self.source_dict.keys():
-            if not self.is_struct():
+        # Datatype and unit need special handling, so we do some further analysis
+        # self.data_type shall only be set if base type is a primitive (VSSDataType)
+        if self.has_datatype():
+            if self.is_signal() or self.is_property():
                 self.data_type_str = self.source_dict["datatype"]
                 self.validate_and_set_datatype()
             else:
-                logging.warning(f"Data type specified for struct node: {self.name}. Ignoring it")
+                logging.error("Item %s cannot have datatype, only allowed for signal and property!", self.name)
+                sys.exit(-1)
+        elif (self.is_signal() or self.is_property()):
+            raise IncompleteElementException(
+                (f"Incomplete element {self.name} from {self.source_dict['$file_name$']}: "
+                 f"Elements of type {self.type.value} need to have a datatype declared."))
 
         # Units are applicable only for primitives. Not user defined types.
-        if "unit" in self.source_dict.keys() and self.has_datatype():
+        if self.has_unit():
+
+            if not (self.is_signal() or self.is_property()):
+                logging.error("Item %s cannot have unit, only allowed for signal and property!", self.name)
+                sys.exit(-1)
+
             unit = self.source_dict["unit"]
             try:
                 self.unit = Unit.from_str(unit)
             except KeyError:
                 logging.error(f"Unknown unit {unit} for signal {self.qualified_name()}. Terminating.")
                 sys.exit(-1)
-        else:
-            self.unit = None
+
+            if not self.has_datatype():
+                logging.error("Unit specified for item not using standard datatype: %s", self.name)
+                sys.exit(-1)
 
         if self.has_instances() and not self.is_branch():
             logging.error(
@@ -307,7 +315,7 @@ def has_datatype(self) -> bool:
 
     def get_datatype(self) -> str:
         """Returns:
-                The name of the dataype or empty string if no datatype
+                The name of the datatype or empty string if no datatype
         """
         return self.data_type_str