Marking move from one component type to another one as impossible for…

… schema evolution (#622)

* fix formatting of schema evolution tool

* add another check for non-allowed schema changes

* add new test for impossible schema evolution detection

python/podio_schema_evolution.py

@@ -105,8 +105,8 @@ def __init__(self, name, member_name, old_member, new_member):
         self.new_member = new_member
         self.klassname = name
         super().__init__(
-            f"'{self.name}.{self.member_name}' changed type from '+\
-                    '{self.old_member.full_type} to {self.new_member.full_type}"
+            f"'{self.name}.{self.member_name}' changed type from "
+            + f"{self.old_member.full_type} to {self.new_member.full_type}"
         )
 
 
@@ -338,6 +338,12 @@ def heuristics(self):
                     f"Forbidden schema change in '{change.name}' for '{change.member_name}' from "
                     f"'{change.old_member.full_type}' to '{change.new_member.full_type}'"
                 )
+            # changing from one component type to another component type is forbidden
+            elif change.old_member.full_type in self.datamodel_old.components:
+                self.errors.append(
+                    f"Forbidden schema change in '{change.name}' for '{change.member_name}' from "
+                    f"'{change.old_member.full_type}' to '{change.new_member.full_type}'"
+                )
 
         # are there dropped/added datatype pairs that could be interpreted as rename?
         # for now assuming no change to the individual datatype definition
@@ -469,4 +475,4 @@ def read_evolution_file(self) -> None:
     comparator.read()
     comparator.compare()
     comparator.print_comparison()
-    print(comparator.get_changed_schemata(schema_filter=root_filter))
+    # print(comparator.get_changed_schemata(schema_filter=root_filter))

tests/schema_evolution/CMakeLists.txt

@@ -49,4 +49,14 @@ add_test(
     ${PROJECT_SOURCE_DIR}/tests/datalayout.yaml
 )
 
+add_test(
+  NAME schema-evolution-script-with-failure
+  COMMAND
+    ${PROJECT_SOURCE_DIR}/python/podio_schema_evolution.py
+    --evo ${CMAKE_CURRENT_SOURCE_DIR}/schema_evolution.yaml
+    ${CMAKE_CURRENT_SOURCE_DIR}/datalayout_notpossible.yaml
+    ${PROJECT_SOURCE_DIR}/tests/datalayout.yaml
+)
+set_property(TEST schema-evolution-script-with-failure PROPERTY WILL_FAIL)
+
 add_subdirectory(root_io)

tests/schema_evolution/datalayout_new.yaml

@@ -196,7 +196,7 @@ datatypes :
     Members:
       - std::int16_t i16Val{42} // some int16 value
       - std::array<float, 2> floats {3.14f, 1.23f} // some float values
-      - ex2::NamespaceStructLong s{10, 11} // one that we happen to know works
+      - ex2::NamespaceStruct s{10, 11} //
       - double d{9.876e5} // double val
       - CompWithInit comp // To make sure that the default initializer of the component does what it should
 

tests/schema_evolution/datalayout_notpossible.yaml

@@ -0,0 +1,207 @@
+---
+schema_version : 3
+
+options :
+  # should getters / setters be prefixed with get / set?
+  getSyntax: False
+  # should POD members be exposed with getters/setters in classes that have them as members?
+  exposePODMembers: True
+  includeSubfolder: True
+
+components :
+
+  SimpleStruct:
+    Members:
+      - int x
+      - int y
+      - int z
+      - int t
+      - std::array<int, 4> p
+    # can also add c'tors:
+    ExtraCode :
+      declaration: "
+      SimpleStruct() : x(0),z(0) {}\n
+      SimpleStruct( const int* v) : x(v[0]),z(v[2]) {}
+      "
+
+  NotSoSimpleStruct:
+    Members:
+      - SimpleStruct data // component members can have descriptions
+
+  ex2::NamespaceStruct:
+    Members:
+      - int x
+      - int y_new
+
+  ex2::NamespaceStructLong:
+    Members:
+      - int64_t x
+      - int64_t y_new
+
+  ex2::NamespaceInNamespaceStruct:
+    Members:
+      - ex2::NamespaceStruct data
+
+  StructWithFixedWithTypes:
+    Members:
+      - uint16_t fixedUnsigned16 // unsigned int with exactly 16 bits
+      - std::int64_t fixedInteger64 // int with exactly 64 bits
+      - int32_t fixedInteger32 // int with exactly 32 bits
+
+  CompWithInit:
+    Members:
+      - int i{42} // is there even another value to initialize ints to?
+      - std::array<double, 10> arr {1.2, 3.4} // half initialized double array
+
+datatypes :
+
+  EventInfoNewName:
+    Description : "Event info"
+    Author : "B. Hegner"
+    Members :
+     - int Number // event number
+    MutableExtraCode :
+       declaration: "void setNumber(int n) { Number( n ) ; } "
+    ExtraCode:
+      declaration: "int getNumber() const;"
+      implementation: "int {name}::getNumber() const { return Number(); }"
+
+  ExampleHit :
+    Description : "Example Hit"
+    Author : "B. Hegner"
+    Members:
+     - unsigned long long cellID      // cellID
+     - double x      // x-coordinate
+     - double y      // y-coordinate
+     - double z      // z-coordinate
+     - double energy // energy
+     - double t      // time
+
+  ExampleMC :
+    Description : "Example MC-particle"
+    Author: "F.Gaede"
+    Members:
+      - double energy  // energy
+      - int PDG  // PDG code
+    OneToManyRelations:
+      - ExampleMC parents   // parents
+      - ExampleMC daughters  // daughters
+
+  ExampleCluster :
+    Description : "Cluster"
+    Author : "B. Hegner"
+    Members:
+      - double energy // cluster energy
+    OneToManyRelations:
+     - ExampleHit Hits // hits contained in the cluster
+     - ExampleCluster Clusters // sub clusters used to create this cluster
+
+  ExampleReferencingType :
+    Description : "Referencing Type"
+    Author : "B. Hegner"
+    OneToManyRelations:
+     - ExampleCluster         Clusters  // some refs to Clusters
+     - ExampleReferencingType Refs      // refs into same type
+
+  ExampleWithVectorMember :
+    Description : "Type with a vector member"
+    Author : "B. Hegner"
+    VectorMembers:
+     - int count // various ADC counts
+
+  ExampleWithOneRelation :
+    Description : "Type with one relation member"
+    Author : "Benedikt Hegner"
+    OneToOneRelations:
+     - ExampleCluster cluster // a particular cluster
+
+  ExampleWithArrayComponent:
+    Description: "A type that has a component with an array"
+    Author: "Thomas Madlener"
+    Members:
+      - SimpleStruct s // a component that has an array
+
+  ExampleWithComponent :
+    Description : "Type with one component"
+    Author : "Benedikt Hegner"
+    Members :
+     - NotSoSimpleStruct component // a component
+
+  ExampleForCyclicDependency1 :
+    Description : "Type for cyclic dependency"
+    Author : "Benedikt Hegner"
+    OneToOneRelations :
+      - ExampleForCyclicDependency2 ref // a ref
+
+  ExampleForCyclicDependency2 :
+    Description : "Type for cyclic dependency"
+    Author : "Benedikt Hegner"
+    OneToOneRelations :
+      - ExampleForCyclicDependency1 ref // a ref
+
+#  ExampleWithArray :
+#    Description : "Type with an array"
+#    Author : "Benedikt Hegner"
+#    Members:
+#      - std::array<int,33> array // the array
+
+  ex42::ExampleWithNamespace :
+    Description : "Type with namespace and namespaced member"
+    Author : "Joschka Lingemann"
+    Members:
+      - ex2::NamespaceStruct component // a component
+
+  ex42::ExampleWithARelation :
+    Description : "Type with namespace and namespaced relation"
+    Author : "Joschka Lingemann"
+    Members:
+     - double number // just a number
+    OneToOneRelations :
+     - ex42::ExampleWithNamespace ref // a ref in a namespace
+    OneToManyRelations :
+     - ex42::ExampleWithNamespace refs // multiple refs in a namespace
+
+  ExampleWithDifferentNamespaceRelations:
+    Description: "Datatype using a namespaced relation without being in the same namespace"
+    Author: "Thomas Madlener"
+    OneToOneRelations:
+      - ex42::ExampleWithNamespace rel // a relation in a different namespace
+    OneToManyRelations:
+      - ex42::ExampleWithNamespace rels // relations in a different namespace
+
+  ExampleWithArray:
+    Description: "Datatype with an array member"
+    Author: "Joschka Lingemann"
+    Members:
+      - NotSoSimpleStruct arrayStruct // component that contains an array
+      - std::array<int,4> myArray // array-member without space to test regex
+      - std::array<int, 4> anotherArray2 // array-member with space to test regex
+      - std::array<int, 4> snail_case_array // snail case to test regex
+      - std::array<int, 4> snail_case_Array3 // mixing things up for regex
+      - std::array<ex2::NamespaceStruct, 4> structArray // an array containing structs
+
+  ExampleWithFixedWidthIntegers:
+    Description: "Datatype using fixed width integer types as members"
+    Author: "Thomas Madlener"
+    Members:
+      - std::int16_t fixedI16 // some integer with exactly 16 bits
+      - uint64_t fixedU64 // unsigned int with exactly 64 bits
+      - uint32_t fixedU32 // unsigned int with exactly 32 bits
+      - StructWithFixedWithTypes fixedWidthStruct // struct with more fixed width types
+      - std::array<int16_t, 2> fixedWidthArray // 32 bits split into two times 16 bits
+
+  ExampleWithUserInit:
+    Description: "Datatype with user defined initialization values"
+    Author: "Thomas Madlener"
+    Members:
+      - std::int16_t i16Val{42} // some int16 value
+      - std::array<float, 2> floats {3.14f, 1.23f} // some float values
+      - ex2::NamespaceStructLong s{10, 11} // this schema evolution should fail
+      - double d{9.876e5} // double val
+      - CompWithInit comp // To make sure that the default initializer of the component does what it should
+
+  ExampleOfDroppedType:
+    Description: "Datatype with user defined initialization values"
+    Author: "Thomas Madlener"
+    Members:
+      - int x // some member