From b7d6cc7082ef595a335a98039c0df8e2a0ce9324 Mon Sep 17 00:00:00 2001 From: Simon Stone Date: Fri, 5 Aug 2022 19:00:31 +0100 Subject: [PATCH] feat(*): detect enum value add/remove (contributes to #442) Signed-off-by: Simon Stone --- .../concerto-analysis/src/compare-config.ts | 11 ++- .../concerto-analysis/src/compare-utils.ts | 14 ++- .../concerto-analysis/src/comparers/fields.ts | 62 ------------- .../concerto-analysis/src/comparers/index.ts | 4 +- .../src/comparers/properties.ts | 87 +++++++++++++++++++ .../test/fixtures/enum-value-added-a.cto | 5 ++ .../test/fixtures/enum-value-added-b.cto | 6 ++ .../test/unit/compare.test.ts | 36 ++++++-- 8 files changed, 150 insertions(+), 75 deletions(-) delete mode 100644 packages/concerto-analysis/src/comparers/fields.ts create mode 100644 packages/concerto-analysis/src/comparers/properties.ts create mode 100644 packages/concerto-analysis/test/fixtures/enum-value-added-a.cto create mode 100644 packages/concerto-analysis/test/fixtures/enum-value-added-b.cto diff --git a/packages/concerto-analysis/src/compare-config.ts b/packages/concerto-analysis/src/compare-config.ts index 0ac0fa81aa..c5d362e2e1 100644 --- a/packages/concerto-analysis/src/compare-config.ts +++ b/packages/concerto-analysis/src/compare-config.ts @@ -49,9 +49,12 @@ export const defaultCompareConfig: CompareConfig = { 'class-declaration-added': CompareResult.MINOR, 'class-declaration-removed': CompareResult.MAJOR, 'class-declaration-type-changed': CompareResult.MAJOR, - 'required-field-added': CompareResult.MAJOR, - 'optional-field-added': CompareResult.PATCH, - 'field-removed': CompareResult.MAJOR, - 'namespace-changed': CompareResult.ERROR + 'required-property-added': CompareResult.MAJOR, + 'optional-property-added': CompareResult.PATCH, + 'required-property-removed': CompareResult.MAJOR, + 'optional-property-removed': CompareResult.MAJOR, + 'namespace-changed': CompareResult.ERROR, + 'enum-value-added': CompareResult.PATCH, + 'enum-value-removed': CompareResult.MAJOR, } }; diff --git a/packages/concerto-analysis/src/compare-utils.ts b/packages/concerto-analysis/src/compare-utils.ts index 8b8df1eac3..a1da7184f8 100644 --- a/packages/concerto-analysis/src/compare-utils.ts +++ b/packages/concerto-analysis/src/compare-utils.ts @@ -12,7 +12,7 @@ * limitations under the License. */ -import { ClassDeclaration } from '@accordproject/concerto-core'; +import { ClassDeclaration, EnumValueDeclaration, Field, Property, RelationshipDeclaration } from '@accordproject/concerto-core'; export function getClassDeclarationType(classDeclaration: ClassDeclaration) { if (classDeclaration.isAsset()) { @@ -31,3 +31,15 @@ export function getClassDeclarationType(classDeclaration: ClassDeclaration) { throw new Error(`unknown class declaration type "${classDeclaration}"`); } } + +export function getPropertyType(property: Property) { + if (property instanceof Field) { + return 'field'; + } else if (property instanceof RelationshipDeclaration) { + return 'relationship'; + } else if (property instanceof EnumValueDeclaration) { + return 'enum value'; + } else { + throw new Error(`unknown property type "${property}"`); + } +} diff --git a/packages/concerto-analysis/src/comparers/fields.ts b/packages/concerto-analysis/src/comparers/fields.ts deleted file mode 100644 index 0864a4fd49..0000000000 --- a/packages/concerto-analysis/src/comparers/fields.ts +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { Field } from '@accordproject/concerto-core'; -import { getClassDeclarationType } from '../compare-utils'; -import { ComparerFactory } from '../comparer'; - -const fieldAdded: ComparerFactory = (context) => ({ - compareProperty: (a, b) => { - if (a || !b) { - return; - } else if (!(b instanceof Field)) { - return; - } - const isOptional = b.isOptional(); - const hasDefault = !!b.getDefaultValue(); - const required = !isOptional && !hasDefault; - const classDeclarationType = getClassDeclarationType(b.getParent()); - if (required) { - context.report({ - key: 'required-field-added', - message: `The required field "${b.getName()}" was added to the ${classDeclarationType} "${b.getParent().getName()}"`, - element: b, - }); - } else { - context.report({ - key: 'optional-field-added', - message: `The optional field "${b.getName()}" was added to the ${classDeclarationType} "${b.getParent().getName()}"`, - element: b, - }); - } - } -}); - -const fieldRemoved: ComparerFactory = (context) => ({ - compareProperty: (a, b) => { - if (!a || b) { - return; - } else if (!(a instanceof Field)) { - return; - } - const classDeclarationType = getClassDeclarationType(a.getParent()); - context.report({ - key: 'field-removed', - message: `The field "${a.getName()}" was removed from the ${classDeclarationType} "${a.getParent().getName()}"`, - element: a - }); - } -}); - -export const fieldComparerFactories = [fieldAdded, fieldRemoved]; diff --git a/packages/concerto-analysis/src/comparers/index.ts b/packages/concerto-analysis/src/comparers/index.ts index a039ae2da9..37ea72c4ce 100644 --- a/packages/concerto-analysis/src/comparers/index.ts +++ b/packages/concerto-analysis/src/comparers/index.ts @@ -13,11 +13,11 @@ */ import { classDeclarationComparerFactories } from './class-declarations'; -import { fieldComparerFactories } from './fields'; import { modelFileComparerFactories } from './model-files'; +import { propertyComparerFactories } from './properties'; export const comparerFactories = [ ...classDeclarationComparerFactories, - ...fieldComparerFactories, + ...propertyComparerFactories, ...modelFileComparerFactories ]; diff --git a/packages/concerto-analysis/src/comparers/properties.ts b/packages/concerto-analysis/src/comparers/properties.ts new file mode 100644 index 0000000000..9e9d1b7f28 --- /dev/null +++ b/packages/concerto-analysis/src/comparers/properties.ts @@ -0,0 +1,87 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { EnumValueDeclaration, Field } from '@accordproject/concerto-core'; +import { getClassDeclarationType, getPropertyType } from '../compare-utils'; +import { ComparerFactory } from '../comparer'; + +const propertyAdded: ComparerFactory = (context) => ({ + compareProperty: (a, b) => { + if (a || !b) { + return; + } + const classDeclarationType = getClassDeclarationType(b.getParent()); + if (b instanceof EnumValueDeclaration) { + context.report({ + key: 'enum-value-added', + message: `The enum value "${b.getName()}" was added to the ${classDeclarationType} "${b.getParent().getName()}"`, + element: b, + }); + return; + } + const isOptional = b.isOptional(); + const hasDefault = b instanceof Field ? !!b.getDefaultValue() : false; + const required = !isOptional && !hasDefault; + const type = getPropertyType(b); + if (required) { + context.report({ + key: 'required-property-added', + message: `The required ${type} "${b.getName()}" was added to the ${classDeclarationType} "${b.getParent().getName()}"`, + element: b, + }); + } else { + context.report({ + key: 'optional-property-added', + message: `The optional ${type} "${b.getName()}" was added to the ${classDeclarationType} "${b.getParent().getName()}"`, + element: b, + }); + } + } +}); + +const propertyRemoved: ComparerFactory = (context) => ({ + compareProperty: (a, b) => { + if (!a || b) { + return; + } + const classDeclarationType = getClassDeclarationType(a.getParent()); + if (a instanceof EnumValueDeclaration) { + context.report({ + key: 'enum-value-removed', + message: `The enum value "${a.getName()}" was removed from the ${classDeclarationType} "${a.getParent().getName()}"`, + element: b, + }); + return; + } + const isOptional = a.isOptional(); + const hasDefault = a instanceof Field ? !!a.getDefaultValue() : false; + const required = !isOptional && !hasDefault; + const type = getPropertyType(a); + if (required) { + context.report({ + key: 'required-property-removed', + message: `The required ${type} "${a.getName()}" was removed from the ${classDeclarationType} "${a.getParent().getName()}"`, + element: a + }); + } else { + context.report({ + key: 'optional-property-removed', + message: `The optional ${type} "${a.getName()}" was removed from the ${classDeclarationType} "${a.getParent().getName()}"`, + element: a + }); + } + } +}); + +export const propertyComparerFactories = [propertyAdded, propertyRemoved]; diff --git a/packages/concerto-analysis/test/fixtures/enum-value-added-a.cto b/packages/concerto-analysis/test/fixtures/enum-value-added-a.cto new file mode 100644 index 0000000000..ceb8dbfc81 --- /dev/null +++ b/packages/concerto-analysis/test/fixtures/enum-value-added-a.cto @@ -0,0 +1,5 @@ +namespace org.accordproject.concerto.test@1.2.3 + +enum Thing { + o FOO +} diff --git a/packages/concerto-analysis/test/fixtures/enum-value-added-b.cto b/packages/concerto-analysis/test/fixtures/enum-value-added-b.cto new file mode 100644 index 0000000000..0b369747c3 --- /dev/null +++ b/packages/concerto-analysis/test/fixtures/enum-value-added-b.cto @@ -0,0 +1,6 @@ +namespace org.accordproject.concerto.test@1.2.3 + +enum Thing { + o FOO + o BAR +} diff --git a/packages/concerto-analysis/test/unit/compare.test.ts b/packages/concerto-analysis/test/unit/compare.test.ts index f6ad4aaaeb..c0e866daac 100644 --- a/packages/concerto-analysis/test/unit/compare.test.ts +++ b/packages/concerto-analysis/test/unit/compare.test.ts @@ -83,7 +83,7 @@ test('should detect a required field being added', async () => { const results = new Compare().compare(a, b); expect(results.findings).toEqual(expect.arrayContaining([ expect.objectContaining({ - key: 'required-field-added', + key: 'required-property-added', message: 'The required field "value" was added to the concept "Thing"' }) ])); @@ -95,8 +95,8 @@ test('should detect a required field being removed', async () => { const results = new Compare().compare(a, b); expect(results.findings).toEqual(expect.arrayContaining([ expect.objectContaining({ - key: 'field-removed', - message: 'The field "value" was removed from the concept "Thing"' + key: 'required-property-removed', + message: 'The required field "value" was removed from the concept "Thing"' }) ])); expect(results.result).toBe(CompareResult.MAJOR); @@ -107,7 +107,7 @@ test('should detect an optional field being added', async () => { const results = new Compare().compare(a, b); expect(results.findings).toEqual(expect.arrayContaining([ expect.objectContaining({ - key: 'optional-field-added', + key: 'optional-property-added', message: 'The optional field "value" was added to the concept "Thing"' }) ])); @@ -119,8 +119,8 @@ test('should detect an optional field being removed', async () => { const results = new Compare().compare(a, b); expect(results.findings).toEqual(expect.arrayContaining([ expect.objectContaining({ - key: 'field-removed', - message: 'The field "value" was removed from the concept "Thing"' + key: 'optional-property-removed', + message: 'The optional field "value" was removed from the concept "Thing"' }) ])); expect(results.result).toBe(CompareResult.MAJOR); @@ -140,3 +140,27 @@ test('should detect an optional field being removed', async () => { expect(results.result).toBe(CompareResult.MAJOR); }); }); + +test('should detect a enum value being added', async () => { + const [a, b] = await getModelFiles('enum-value-added-a.cto', 'enum-value-added-b.cto'); + const results = new Compare().compare(a, b); + expect(results.findings).toEqual(expect.arrayContaining([ + expect.objectContaining({ + key: 'enum-value-added', + message: 'The enum value "BAR" was added to the enum "Thing"' + }) + ])); + expect(results.result).toBe(CompareResult.PATCH); +}); + +test('should detect an enum value being removed', async () => { + const [a, b] = await getModelFiles('enum-value-added-b.cto', 'enum-value-added-a.cto'); + const results = new Compare().compare(a, b); + expect(results.findings).toEqual(expect.arrayContaining([ + expect.objectContaining({ + key: 'enum-value-removed', + message: 'The enum value "BAR" was removed from the enum "Thing"' + }) + ])); + expect(results.result).toBe(CompareResult.MAJOR); +});