fix(concerto-analysis): route Map and Scalar declarations through dedicated comparison paths

packages/concerto-analysis/src/compare-config.ts

@@ -63,8 +63,12 @@ export const defaultCompareConfig: CompareConfig = {
         'property-validator-added': CompareResult.MAJOR,
         'property-validator-removed': CompareResult.PATCH,
         'property-validator-changed': CompareResult.MAJOR,
+        'map-declaration-added': CompareResult.MINOR,
+        'map-declaration-removed': CompareResult.MAJOR,
         'map-key-type-changed': CompareResult.MAJOR,
         'map-value-type-changed': CompareResult.MAJOR,
+        'scalar-declaration-added': CompareResult.MINOR,
+        'scalar-declaration-removed': CompareResult.MAJOR,
         'scalar-extends-changed': CompareResult.MAJOR,
         'scalar-validator-added': CompareResult.MAJOR,
         'scalar-validator-removed': CompareResult.PATCH,

packages/concerto-analysis/src/compare.ts

@@ -122,14 +122,6 @@ export class Compare {
 
     private compareClassDeclaration(comparers: Comparer[], a: ClassDeclaration, b: ClassDeclaration) {
         comparers.forEach(comparer => comparer.compareClassDeclaration?.(a, b));
-        // MapDeclarations do not contain properties, nothing to compare.
-        if(a instanceof MapDeclaration || b instanceof MapDeclaration) {
-            return;
-        }
-        // ScalarDeclarations do not contain properties, nothing to compare.
-        if(a instanceof ScalarDeclaration || b instanceof ScalarDeclaration) {
-            return;
-        }
         this.compareProperties(comparers, a.getOwnProperties(), b.getOwnProperties());
     }
 
@@ -144,7 +136,7 @@ export class Compare {
 
     private compareModelFiles(comparers: Comparer[], a: ModelFile, b: ModelFile) {
         comparers.forEach(comparer => comparer.compareModelFiles?.(a, b));
-        this.compareClassDeclarations(comparers, a.getAllDeclarations(), b.getAllDeclarations());
+        this.compareClassDeclarations(comparers, a.getClassDeclarations(), b.getClassDeclarations());
         this.compareMapDeclarations(comparers, a.getMapDeclarations(), b.getMapDeclarations());
         this.compareScalarDeclarations(comparers, a.getScalarDeclarations(), b.getScalarDeclarations());
     }

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

@@ -14,6 +14,32 @@
 
 import { ComparerFactory } from '../comparer';
 
+const mapDeclarationAdded: ComparerFactory = (context) => ({
+    compareMapDeclaration: (a, b) => {
+        if (a || !b) {
+            return;
+        }
+        context.report({
+            key: 'map-declaration-added',
+            message: `The map "${b.getName()}" was added`,
+            element: b
+        });
+    }
+});
+
+const mapDeclarationRemoved: ComparerFactory = (context) => ({
+    compareMapDeclaration: (a, b) => {
+        if (!a || b) {
+            return;
+        }
+        context.report({
+            key: 'map-declaration-removed',
+            message: `The map "${a.getName()}" was removed`,
+            element: a
+        });
+    }
+});
+
 const mapDeclarationTypeChanged: ComparerFactory = (context) => ({
     compareMapDeclaration: (a, b) => {
 
@@ -39,4 +65,4 @@ const mapDeclarationTypeChanged: ComparerFactory = (context) => ({
     },
 });
 
-export const mapDeclarationComparerFactories = [mapDeclarationTypeChanged];
+export const mapDeclarationComparerFactories = [mapDeclarationAdded, mapDeclarationRemoved, mapDeclarationTypeChanged];

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

@@ -15,6 +15,32 @@
 import { ComparerFactory } from '../comparer';
 import { getValidatorType } from '../compare-utils';
 
+const scalarDeclarationAdded: ComparerFactory = (context) => ({
+    compareScalarDeclaration: (a, b) => {
+        if (a || !b) {
+            return;
+        }
+        context.report({
+            key: 'scalar-declaration-added',
+            message: `The scalar "${b.getName()}" was added`,
+            element: b
+        });
+    }
+});
+
+const scalarDeclarationRemoved: ComparerFactory = (context) => ({
+    compareScalarDeclaration: (a, b) => {
+        if (!a || b) {
+            return;
+        }
+        context.report({
+            key: 'scalar-declaration-removed',
+            message: `The scalar "${a.getName()}" was removed`,
+            element: a
+        });
+    }
+});
+
 const scalarDeclarationExtendsChanged: ComparerFactory = (context) => ({
     compareScalarDeclaration: (a, b) => {
 
@@ -100,4 +126,4 @@ const scalarDefaultValueChanged: ComparerFactory = (context) => ({
     }
 });
 
-export const scalarDeclarationComparerFactories = [scalarDeclarationExtendsChanged, scalarValidatorChanged, scalarDefaultValueChanged];
+export const scalarDeclarationComparerFactories = [scalarDeclarationAdded, scalarDeclarationRemoved, scalarDeclarationExtendsChanged, scalarValidatorChanged, scalarDefaultValueChanged];

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

@@ -70,9 +70,8 @@ test('should detect a change of namespace', async () => {
     expect(results.result).toBe(CompareResult.ERROR);
 });
 
-['asset', 'concept', 'enum', 'event', 'participant', 'transaction', 'map', 'scalar'].forEach(type => {
+['asset', 'concept', 'enum', 'event', 'participant', 'transaction'].forEach(type => {
     test(`should detect a ${type} being added`, async () => {
-        process.env.ENABLE_MAP_TYPE = 'true'; // TODO Remove on release of MapType
         const [a, b] = await getModelFiles('empty.cto', `${type}-added.cto`);
         const results = new Compare().compare(a, b);
         expect(results.findings).toEqual(expect.arrayContaining([
@@ -97,6 +96,64 @@ test('should detect a change of namespace', async () => {
     });
 });
 
+test('should detect a map being added', async () => {
+    process.env.ENABLE_MAP_TYPE = 'true'; // TODO Remove on release of MapType
+    const [a, b] = await getModelFiles('empty.cto', 'map-added.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'map-declaration-added',
+            message: 'The map "Thing" was added'
+        })
+    ]));
+    expect(results.result).toBe(CompareResult.MINOR);
+});
+
+test('should detect a map being removed', async () => {
+    process.env.ENABLE_MAP_TYPE = 'true';
+    const [a, b] = await getModelFiles('map-added.cto', 'empty.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'map-declaration-removed',
+            message: 'The map "Thing" was removed'
+        })
+    ]));
+    expect(results.result).toBe(CompareResult.MAJOR);
+});
+
+test('should detect a scalar being added', async () => {
+    const [a, b] = await getModelFiles('empty.cto', 'scalar-added.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'scalar-declaration-added',
+            message: 'The scalar "Thing" was added'
+        })
+    ]));
+    expect(results.result).toBe(CompareResult.MINOR);
+});
+
+test('should detect a scalar being removed', async () => {
+    const [a, b] = await getModelFiles('scalar-added.cto', 'empty.cto');
+    const results = new Compare().compare(a, b);
+    expect(results.findings).toEqual(expect.arrayContaining([
+        expect.objectContaining({
+            key: 'scalar-declaration-removed',
+            message: 'The scalar "Thing" was removed'
+        })
+    ]));
+    expect(results.result).toBe(CompareResult.MAJOR);
+});
+
+test('should not emit class-declaration findings for map or scalar types', async () => {
+    process.env.ENABLE_MAP_TYPE = 'true';
+    const [a, b] = await getModelFiles('empty.cto', 'map-added.cto');
+    const results = new Compare().compare(a, b);
+    const classFindings = results.findings.filter(f => f.key.startsWith('class-declaration-'));
+    expect(classFindings).toHaveLength(0);
+});
+
 test('should detect a required field being added', async () => {
     const [a, b] = await getModelFiles('required-field-added-a.cto', 'required-field-added-b.cto');
     const results = new Compare().compare(a, b);
@@ -278,7 +335,6 @@ test('should detect a map key type changing from x to y', async () => {
             message: 'The map key type was changed to "DateTime"'
         })
     ]));
-    expect(results.findings[0].message).toBe('The map key type was changed to "DateTime"');
     expect(results.result).toBe(CompareResult.MAJOR);
 });
 
@@ -292,7 +348,6 @@ test('should detect a map value type changing from x to y', async () => {
             message: 'The map value type was changed to "DateTime"'
         })
     ]));
-    expect(results.findings[0].message).toBe('The map value type was changed to "DateTime"');
     expect(results.result).toBe(CompareResult.MAJOR);
 });
 
@@ -305,7 +360,6 @@ test('should detect a scalar extends changing from x to y', async () => {
             message: 'The scalar extends was changed from "String" to "Integer"'
         })
     ]));
-    expect(results.findings[0].message).toBe('The scalar extends was changed from "String" to "Integer"');
     expect(results.result).toBe(CompareResult.MAJOR);
 });
 
@@ -696,6 +750,16 @@ test('should give a MAJOR CompareResult for Map Type compare config rules)', asy
     expect(defaultCompareConfig.rules['map-value-type-changed']).toBe(CompareResult.MAJOR);
 });
 
+test('should give correct CompareResult for map declaration add/remove rules', () => {
+    expect(defaultCompareConfig.rules['map-declaration-added']).toBe(CompareResult.MINOR);
+    expect(defaultCompareConfig.rules['map-declaration-removed']).toBe(CompareResult.MAJOR);
+});
+
+test('should give correct CompareResult for scalar declaration add/remove rules', () => {
+    expect(defaultCompareConfig.rules['scalar-declaration-added']).toBe(CompareResult.MINOR);
+    expect(defaultCompareConfig.rules['scalar-declaration-removed']).toBe(CompareResult.MAJOR);
+});
+
 test('should detect a required property changed to optional', async () => {
     const [a, b] = await getModelFiles('required-to-optional-a.cto', 'required-to-optional-b.cto');
     const results = new Compare().compare(a, b);