Skip to content

Commit

Permalink
feat(analysis) scalar change analysis (#704)
Browse files Browse the repository at this point in the history
* feat(analysis) scalar change analysis

Signed-off-by: Dan Selman <danscode@selman.org>
  • Loading branch information
dselman authored Sep 2, 2023
1 parent d2e7c1a commit 8f05541
Show file tree
Hide file tree
Showing 23 changed files with 350 additions and 8 deletions.
6 changes: 5 additions & 1 deletion packages/concerto-analysis/src/compare-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ export const defaultCompareConfig: CompareConfig = {
'property-validator-removed': CompareResult.PATCH,
'property-validator-changed': CompareResult.MAJOR,
'map-key-type-changed': CompareResult.MAJOR,
'map-value-type-changed': CompareResult.MAJOR
'map-value-type-changed': CompareResult.MAJOR,
'scalar-extends-changed': CompareResult.MAJOR,
'scalar-validator-added' : CompareResult.MAJOR,
'scalar-validator-removed' : CompareResult.PATCH,
'scalar-validator-changed' : CompareResult.MAJOR,
}
};
7 changes: 6 additions & 1 deletion packages/concerto-analysis/src/compare-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* limitations under the License.
*/

import { ClassDeclaration, EnumValueDeclaration, Field, MapDeclaration, NumberValidator, Property, RelationshipDeclaration, StringValidator, Validator } from '@accordproject/concerto-core';
import { ClassDeclaration, EnumValueDeclaration, Field, MapDeclaration, ScalarDeclaration, NumberValidator, Property, RelationshipDeclaration, StringValidator, Validator } from '@accordproject/concerto-core';
import Declaration from '@accordproject/concerto-core/types/lib/introspect/declaration';

export function getDeclarationType(declaration: Declaration) {
Expand All @@ -34,6 +34,11 @@ export function getDeclarationType(declaration: Declaration) {
}
} else if (declaration instanceof MapDeclaration) {
return 'map';
} else if (declaration instanceof ScalarDeclaration) {
return 'scalar';
}
else {
throw new Error(`unknown declaration type "${declaration}"`);
}
}

Expand Down
29 changes: 28 additions & 1 deletion packages/concerto-analysis/src/compare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* limitations under the License.
*/

import { ClassDeclaration, MapDeclaration, ModelFile, Property } from '@accordproject/concerto-core';
import { ClassDeclaration, MapDeclaration, ModelFile, Property, ScalarDeclaration } from '@accordproject/concerto-core';
import { CompareConfig, CompareResult, defaultCompareConfig } from './compare-config';
import { CompareFinding } from './compare-message';
import { CompareResults } from './compare-results';
Expand Down Expand Up @@ -69,14 +69,26 @@ export class Compare {
return b.filter(bItem => !a.some(aItem => bItem.getName() === aItem.getName()));
}

private getAddedScalarDeclarations(a: ScalarDeclaration[], b: ScalarDeclaration[]): ScalarDeclaration[] {
return b.filter(bItem => !a.some(aItem => bItem.getName() === aItem.getName()));
}

private getMatchingMapDeclarations(a: MapDeclaration[], b: MapDeclaration[]): [a: MapDeclaration, b: MapDeclaration][] {
return a.map(aItem => [aItem, b.find(bItem => aItem.getName() === bItem.getName())]).filter(([, b]) => !!b) as [MapDeclaration, MapDeclaration][];
}

private getMatchingScalarDeclarations(a: ScalarDeclaration[], b: ScalarDeclaration[]): [a: ScalarDeclaration, b: ScalarDeclaration][] {
return a.map(aItem => [aItem, b.find(bItem => aItem.getName() === bItem.getName())]).filter(([, b]) => !!b) as [ScalarDeclaration, ScalarDeclaration][];
}

private getRemovedMapDeclarations(a: MapDeclaration[], b: MapDeclaration[]): MapDeclaration[] {
return a.filter(aItem => !b.some(bItem => aItem.getName() === bItem.getName()));
}

private getRemovedScalarDeclarations(a: ScalarDeclaration[], b: ScalarDeclaration[]): ScalarDeclaration[] {
return a.filter(aItem => !b.some(bItem => aItem.getName() === bItem.getName()));
}

private getAddedClassDeclarations(a: ClassDeclaration[], b: ClassDeclaration[]): ClassDeclaration[] {
return b.filter(bItem => !a.some(aItem => bItem.getName() === aItem.getName()));
}
Expand All @@ -98,12 +110,26 @@ export class Compare {
removed.forEach(a => comparers.forEach(comparer => comparer.compareMapDeclaration?.(a, undefined)));
}

private compareScalarDeclarations(comparers: Comparer[], a: ScalarDeclaration[], b: ScalarDeclaration[]) {
const added = this.getAddedScalarDeclarations(a, b);
const matching = this.getMatchingScalarDeclarations(a, b);
const removed = this.getRemovedScalarDeclarations(a, b);
added.forEach(b => comparers.forEach(comparer => comparer.compareScalarDeclaration?.(undefined, b)));
matching.forEach(([a, b]) => comparers.forEach(comparer => comparer.compareScalarDeclaration?.(a, b)));
removed.forEach(a => comparers.forEach(comparer => comparer.compareScalarDeclaration?.(a, undefined)));
}


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());
}

Expand All @@ -120,6 +146,7 @@ export class Compare {
comparers.forEach(comparer => comparer.compareModelFiles?.(a, b));
this.compareClassDeclarations(comparers, a.getAllDeclarations(), b.getAllDeclarations());
this.compareMapDeclarations(comparers, a.getMapDeclarations(), b.getMapDeclarations());
this.compareScalarDeclarations(comparers, a.getScalarDeclarations(), b.getScalarDeclarations());
}

private buildResults(findings: CompareFinding[]) {
Expand Down
11 changes: 10 additions & 1 deletion packages/concerto-analysis/src/comparer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* limitations under the License.
*/

import { ClassDeclaration, MapDeclaration, ModelFile, Property } from '@accordproject/concerto-core';
import { ClassDeclaration, MapDeclaration, ModelFile, Property, ScalarDeclaration } from '@accordproject/concerto-core';
import { CompareContext } from './compare-context';

/**
Expand Down Expand Up @@ -42,6 +42,15 @@ export type Comparer = {
*/
compareMapDeclaration?: (a: MapDeclaration | undefined, b: MapDeclaration | undefined) => void;

/**
* Called to compare two scalar declarations. If a is undefined, but b is defined, then this scalar declaration was
* created in the second model. If a is defined, but b is undefined, then this map declaration was removed in the
* second model.
* @param a The first scalar declaration for comparision, or undefined if it is undefined in the first model.
* @param b The second scalar declaration for comparision, or undefined if it is undefined in the second model.
*/
compareScalarDeclaration?: (a: ScalarDeclaration | undefined, b: ScalarDeclaration | undefined) => void;

/**
* Called to compare two properties. If a is undefined, but b is definecd, then this property was
* created in the second model. If a is defined, but b is undefined, then this property was removed in the
Expand Down
4 changes: 3 additions & 1 deletion packages/concerto-analysis/src/comparers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ import { classDeclarationComparerFactories } from './class-declarations';
import { modelFileComparerFactories } from './model-files';
import { propertyComparerFactories } from './properties';
import { mapDeclarationComparerFactories } from './map-declarations';
import { scalarDeclarationComparerFactories } from './scalar-declarations';

export const comparerFactories = [
...classDeclarationComparerFactories,
...propertyComparerFactories,
...modelFileComparerFactories,
...mapDeclarationComparerFactories
...mapDeclarationComparerFactories,
...scalarDeclarationComparerFactories
];
72 changes: 72 additions & 0 deletions packages/concerto-analysis/src/comparers/scalar-declarations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* 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 { ComparerFactory } from '../comparer';
import { getValidatorType } from '../compare-utils';

const scalarDeclarationExtendsChanged: ComparerFactory = (context) => ({
compareScalarDeclaration: (a, b) => {

if (!a || !b) {
return;
}

if(a.getType() !== b.getType()) {
context.report({
key: 'scalar-extends-changed',
message: `The scalar extends was changed from "${a.getType()}" to "${b.getType()}"`,
element: b.getName()
});
}
},
});

const scalarValidatorChanged: ComparerFactory = (context) => ({
compareScalarDeclaration: (a, b) => {
if (!a || !b) {
return;
}
const aValidator = a.getValidator();
const bValidator = b.getValidator();
if (!aValidator && !bValidator) {
return;
} else if (!aValidator && bValidator) {
const bValidatorType = getValidatorType(bValidator);
context.report({
key: 'scalar-validator-added',
message: `A ${bValidatorType} validator was added to the scalar "${a.getName()}"`,
element: a.getName()
});
return;
} else if (aValidator && !bValidator) {
const aValidatorType = getValidatorType(aValidator);
context.report({
key: 'scalar-validator-removed',
message: `A ${aValidatorType} validator was removed from the scalar "${a.getName()}"`,
element: a.getName()
});
return;
} else if (!aValidator.compatibleWith(bValidator)) {
const aValidatorType = getValidatorType(aValidator);
context.report({
key: 'scalar-validator-changed',
message: `A ${aValidatorType} validator for the scalar "${a.getName()}" was changed and is no longer compatible`,
element: a.getName()
});
return;
}
}
});

export const scalarDeclarationComparerFactories = [scalarDeclarationExtendsChanged, scalarValidatorChanged];
3 changes: 3 additions & 0 deletions packages/concerto-analysis/test/fixtures/scalar-added.cto
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends String
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends Integer
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends Integer range=[-1,1]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends Integer range=[-2,1]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends Integer range=[-1,2]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends Integer range=[-1,0]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends Integer
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends String regex=/foo/
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends String regex=/bar/
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends String length=[2,10]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends String length=[1,10]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends String length=[3,10]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends String length=[2,100]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends String length=[2,8]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace org.accordproject.concerto.test@1.2.3

scalar Thing extends String
5 changes: 5 additions & 0 deletions packages/concerto-analysis/test/unit/compare-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ test('should throw for unknown class declaration type', () => {
expect(() => getDeclarationType(classDeclaration)).toThrow('unknown class declaration type "ClassDeclaration {id=foo@1.0.0.undefined super=Concept enum=false abstract=false}"');
});

test('should throw for unknown thing', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
expect(() => getDeclarationType('thing' as any)).toThrow('unknown declaration type "thing"');
});

test('should throw for unknown class property type', () => {
expect(() => getPropertyType(property)).toThrow('unknown property type "[object Object]');
});
Expand Down
Loading

0 comments on commit 8f05541

Please sign in to comment.