feat(*): detect primitive to declaration and namespace changes (contr…

…ibutes to accordproject#442)

Signed-off-by: Simon Stone <[email protected]>

packages/concerto-analysis/package.json

@@ -31,11 +31,13 @@
     "author": "accordproject.org",
     "license": "Apache-2.0",
     "dependencies": {
-        "@accordproject/concerto-core": "3.0.0-alpha.1"
+        "@accordproject/concerto-core": "3.0.0-alpha.1",
+        "semver": "7.3.5"
     },
     "devDependencies": {
         "@accordproject/concerto-cto": "3.0.0-alpha.1",
         "@types/jest": "28.1.1",
+        "@types/semver": "7.3.10",
         "@typescript-eslint/parser": "5.27.1",
         "@typescript-eslint/eslint-plugin": "5.27.1",
         "eslint": "8.2.0",

packages/concerto-analysis/src/comparers/class-declarations.ts

@@ -17,27 +17,29 @@ import { ComparerFactory } from '../comparer';
 
 const classDeclarationAdded: ComparerFactory = (context) => ({
     compareClassDeclaration: (a, b) => {
-        if (!a && b) {
-            const type = getClassDeclarationType(b);
-            context.report({
-                key: 'class-declaration-added',
-                message: `The ${type} "${b.getName()}" was added`,
-                element: b
-            });
+        if (a || !b) {
+            return;
         }
+        const type = getClassDeclarationType(b);
+        context.report({
+            key: 'class-declaration-added',
+            message: `The ${type} "${b.getName()}" was added`,
+            element: b
+        });
     }
 });
 
 const classDeclarationRemoved: ComparerFactory = (context) => ({
     compareClassDeclaration: (a, b) => {
-        if (a && !b) {
-            const type = getClassDeclarationType(a);
-            context.report({
-                key: 'class-declaration-removed',
-                message: `The ${type} "${a.getName()}" was removed`,
-                element: a
-            });
+        if (!a || b) {
+            return;
         }
+        const type = getClassDeclarationType(a);
+        context.report({
+            key: 'class-declaration-removed',
+            message: `The ${type} "${a.getName()}" was removed`,
+            element: a
+        });
     }
 });
 

packages/concerto-analysis/src/comparers/model-files.ts

@@ -25,6 +25,7 @@ const namespaceChanged: ComparerFactory = (context) => ({
                 message: `The namespace was changed from "${aName}" to "${bName}"`,
                 element: a
             });
+            return;
         }
     }
 });

packages/concerto-analysis/src/comparers/properties.ts

@@ -12,7 +12,8 @@
  * limitations under the License.
  */
 
-import { EnumValueDeclaration, Field } from '@accordproject/concerto-core';
+import { EnumValueDeclaration, Field, ModelUtil } from '@accordproject/concerto-core';
+import * as semver from 'semver';
 import { getClassDeclarationType, getPropertyType } from '../compare-utils';
 import { ComparerFactory } from '../comparer';
 
@@ -40,12 +41,14 @@ const propertyAdded: ComparerFactory = (context) => ({
                 message: `The required ${type} "${b.getName()}" was added to the ${classDeclarationType} "${b.getParent().getName()}"`,
                 element: b,
             });
+            return;
         } else {
             context.report({
                 key: 'optional-property-added',
                 message: `The optional ${type} "${b.getName()}" was added to the ${classDeclarationType} "${b.getParent().getName()}"`,
                 element: b,
             });
+            return;
         }
     }
 });
@@ -74,12 +77,14 @@ const propertyRemoved: ComparerFactory = (context) => ({
                 message: `The required ${type} "${a.getName()}" was removed from the ${classDeclarationType} "${a.getParent().getName()}"`,
                 element: a
             });
+            return;
         } else {
             context.report({
                 key: 'optional-property-removed',
                 message: `The optional ${type} "${a.getName()}" was removed from the ${classDeclarationType} "${a.getParent().getName()}"`,
                 element: a
             });
+            return;
         }
     }
 });
@@ -110,12 +115,69 @@ const propertyTypeChanged: ComparerFactory = (context) => ({
                 message: `The array ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from an array ${aType} to a scalar ${aType}`,
                 element: a
             });
+            return;
         } else if (!aIsArray && bIsArray) {
             context.report({
                 key: 'property-type-changed',
                 message: `The scalar ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from a scalar ${aType} to an array ${aType}`,
                 element: a
             });
+            return;
+        }
+        const aFQTN = a.getFullyQualifiedTypeName();
+        const bFQTN = b.getFullyQualifiedTypeName();
+        const aTypeName = ModelUtil.getShortName(aFQTN);
+        const bTypeName = ModelUtil.getShortName(bFQTN);
+        if (aTypeName !== bTypeName) {
+            context.report({
+                key: 'property-type-changed',
+                message: `The ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from "${aFQTN}" to "${bFQTN}" (type name differs)`,
+                element: a
+            });
+            return;
+        }
+        const aTypeFullNamespace = ModelUtil.getNamespace(aFQTN);
+        const bTypeFullNamespace = ModelUtil.getNamespace(bFQTN);
+        if (!aTypeFullNamespace && !bTypeFullNamespace) {
+            return;
+        } else if (!aTypeFullNamespace || !bTypeFullNamespace) {
+            context.report({
+                key: 'property-type-changed',
+                message: `The ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from "${aFQTN}" to "${bFQTN}" (type namespace differs)`,
+                element: a
+            });
+            return;
+        }
+        const { name: aTypeNamespace, version: aTypeVersion } = ModelUtil.parseNamespace(aTypeFullNamespace);
+        const { name: bTypeNamespace, version: bTypeVersion } = ModelUtil.parseNamespace(bTypeFullNamespace);
+        if (aTypeNamespace !== bTypeNamespace) {
+            context.report({
+                key: 'property-type-changed',
+                message: `The ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from "${aFQTN}" to "${bFQTN}" (type namespace differs)`,
+                element: a
+            });
+            return;
+        }
+        if (!aTypeVersion && !bTypeVersion) {
+            return;
+        } else if (!aTypeVersion || !bTypeVersion) {
+            context.report({
+                key: 'property-type-changed',
+                message: `The ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from "${aFQTN}" to "${bFQTN}" (type version incompatible)`,
+                element: a
+            });
+            return;
+        }
+        const versionDiff = semver.diff(aTypeVersion, bTypeVersion);
+        if (!versionDiff) {
+            return;
+        } else if (versionDiff === 'major' || versionDiff === 'premajor') {
+            context.report({
+                key: 'property-type-changed',
+                message: `The ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from "${aFQTN}" to "${bFQTN}" (type version incompatible)`,
+                element: a
+            });
+            return;
         }
     },
 });

packages/concerto-analysis/test/fixtures/field-namespace-changed-a.cto

@@ -0,0 +1,7 @@
+namespace [email protected]
+
+import { Bar } from [email protected]
+
+concept Thing {
+    o Bar bar
+}

packages/concerto-analysis/test/fixtures/field-namespace-changed-b.cto

@@ -0,0 +1,7 @@
+namespace [email protected]
+
+import { Bar } from [email protected]
+
+concept Thing {
+    o Bar bar
+}

packages/concerto-analysis/test/fixtures/field-namespace-changed-major-b.cto

@@ -0,0 +1,7 @@
+namespace [email protected]
+
+import { Bar } from [email protected]
+
+concept Thing {
+    o Bar bar
+}

packages/concerto-analysis/test/fixtures/field-namespace-changed-minor-b.cto

@@ -0,0 +1,7 @@
+namespace [email protected]
+
+import { Bar } from [email protected]
+
+concept Thing {
+    o Bar bar
+}

packages/concerto-analysis/test/fixtures/field-namespace-changed-patch-b.cto

@@ -0,0 +1,7 @@
+namespace [email protected]
+
+import { Bar } from [email protected]
+
+concept Thing {
+    o Bar bar
+}

packages/concerto-analysis/test/fixtures/primitive-to-declaration-a.cto

@@ -0,0 +1,9 @@
+namespace [email protected]
+
+concept Bar {
+
+}
+
+concept Thing {
+    o String bar
+}

packages/concerto-analysis/test/fixtures/primitive-to-declaration-b.cto

@@ -0,0 +1,9 @@
+namespace [email protected]
+
+concept Bar {
+
+}
+
+concept Thing {
+    o Bar bar
+}

packages/concerto-analysis/test/unit/compare.test.ts

@@ -212,3 +212,65 @@ test('should detect an array changing to a scalar', async () => {
     ]));
     expect(results.result).toBe(CompareResult.MAJOR);
 });
+
+test('should detect a primitive typed field changing to a declaration typed field', async () => {
+    const [a, b] = await getModelFiles('primitive-to-declaration-a.cto', 'primitive-to-declaration-b.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'property-type-changed',
+            message: 'The field "bar" in the concept "Thing" changed type from "String" to "[email protected]" (type name differs)'
+        })
+    ]));
+    expect(results.result).toBe(CompareResult.MAJOR);
+});
+
+test('should detect a declaration typed field changing to a primitive typed field', async () => {
+    const [a, b] = await getModelFiles('primitive-to-declaration-b.cto', 'primitive-to-declaration-a.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'property-type-changed',
+            message: 'The field "bar" in the concept "Thing" changed type from "[email protected]" to "String" (type name differs)'
+        })
+    ]));
+    expect(results.result).toBe(CompareResult.MAJOR);
+});
+
+test('should detect a declaration typed field namespace change', async () => {
+    const [a, b] = await getModelFiles('field-namespace-changed-a.cto', 'field-namespace-changed-b.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'property-type-changed',
+            message: 'The field "bar" in the concept "Thing" changed type from "[email protected]" to "[email protected]" (type namespace differs)'
+        })
+    ]));
+    expect(results.result).toBe(CompareResult.MAJOR);
+});
+
+test('should not detect a declaration typed field namespace patch version change', async () => {
+    const [a, b] = await getModelFiles('field-namespace-changed-a.cto', 'field-namespace-changed-patch-b.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toHaveLength(0);
+    expect(results.result).toBe(CompareResult.NONE);
+});
+
+test('should not detect a declaration typed field namespace minor version change', async () => {
+    const [a, b] = await getModelFiles('field-namespace-changed-a.cto', 'field-namespace-changed-minor-b.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toHaveLength(0);
+    expect(results.result).toBe(CompareResult.NONE);
+});
+
+test('should detect a declaration typed field namespace major version change', async () => {
+    const [a, b] = await getModelFiles('field-namespace-changed-a.cto', 'field-namespace-changed-major-b.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'property-type-changed',
+            message: 'The field "bar" in the concept "Thing" changed type from "[email protected]" to "[email protected]" (type version incompatible)'
+        })
+    ]));
+    expect(results.result).toBe(CompareResult.MAJOR);
+});