Skip to content

Commit 5e7e8b9

Browse files
khaledGadelhaQKhaled Gadelhaq
khaledGadelhaQ
and
Khaled Gadelhaq
authored
fix(analysis): detect changes for various model modifications (#1000)
* fix(analysis): detect changes when adding/removing optional from a property Signed-off-by: Khaled Gadelhaq <[email protected]> * improve scalar default value change detection Signed-off-by: Khaled Gadelhaq <[email protected]> * improve property default value change detection Signed-off-by: Khaled Gadelhaq <[email protected]> * fixing compare-config tests Signed-off-by: Khaled Gadelhaq <[email protected]> * Changing the compare result for default value changes Signed-off-by: Khaled Gadelhaq <[email protected]> * Fixing tests due the recent changes Signed-off-by: Khaled Gadelhaq <[email protected]> --------- Signed-off-by: Khaled Gadelhaq <[email protected]> Co-authored-by: Khaled Gadelhaq <[email protected]>
1 parent 8df15d2 commit 5e7e8b9

14 files changed

+279
-13
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ export const defaultCompareConfig: CompareConfig = {
5353
'optional-property-added': CompareResult.PATCH,
5454
'required-property-removed': CompareResult.MAJOR,
5555
'optional-property-removed': CompareResult.MAJOR,
56+
'optional-to-required-property': CompareResult.MAJOR,
57+
'required-to-optional-property': CompareResult.PATCH,
5658
'namespace-changed': CompareResult.ERROR,
5759
'enum-value-added': CompareResult.PATCH,
5860
'enum-value-removed': CompareResult.MAJOR,
@@ -67,6 +69,12 @@ export const defaultCompareConfig: CompareConfig = {
6769
'scalar-validator-added': CompareResult.MAJOR,
6870
'scalar-validator-removed': CompareResult.PATCH,
6971
'scalar-validator-changed': CompareResult.MAJOR,
72+
'scalar-default-value-added': CompareResult.MINOR,
73+
'scalar-default-value-removed': CompareResult.MAJOR,
74+
'scalar-default-value-changed': CompareResult.PATCH,
75+
'property-default-value-added': CompareResult.MINOR,
76+
'property-default-value-removed': CompareResult.MAJOR,
77+
'property-default-value-changed': CompareResult.PATCH,
7078
},
7179
};
7280

@@ -172,3 +180,6 @@ export class CompareConfigBuilder {
172180
return this;
173181
}
174182
}
183+
184+
export const DEFAULT_COMPARER_FACTORIES_COUNT = defaultCompareConfig.comparerFactories.length;
185+
export const DEFAULT_RULES_COUNT = Object.keys(defaultCompareConfig.rules).length;

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

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,60 @@ const propertyValidatorChanged: ComparerFactory = (context) => ({
236236
}
237237
});
238238

239-
export const propertyComparerFactories = [propertyAdded, propertyRemoved, propertyTypeChanged, propertyValidatorChanged];
239+
const propertyOptionalChanged: ComparerFactory = (context) => ({
240+
compareProperty: (a, b) => {
241+
if (!a || !b) {
242+
return;
243+
}
244+
const aIsOptional = a.isOptional();
245+
const bIsOptional = b.isOptional();
246+
if (aIsOptional === bIsOptional) {
247+
return;
248+
}
249+
if(aIsOptional && !bIsOptional) {
250+
const classDeclarationType = getDeclarationType(a.getParent());
251+
const type = getPropertyType(a);
252+
context.report({
253+
key: 'optional-to-required-property',
254+
message: `The optional ${type} "${a.getName()}" was changed to be required in the ${classDeclarationType} "${a.getParent().getName()}"`,
255+
element: a,
256+
});
257+
return;
258+
}else{
259+
const classDeclarationType = getDeclarationType(a.getParent());
260+
const type = getPropertyType(a);
261+
context.report({
262+
key: 'required-to-optional-property',
263+
message: `The required ${type} "${a.getName()}" was changed to be optional in the ${classDeclarationType} "${a.getParent().getName()}"`,
264+
element: a,
265+
});
266+
return;
267+
}
268+
}
269+
});
270+
271+
const propertyDefaultChanged: ComparerFactory = (context) => ({
272+
compareProperty: (a, b) => {
273+
if (!a || !b) {return;}
274+
275+
const aAST = JSON.parse(JSON.stringify(a.ast));
276+
const bAST = JSON.parse(JSON.stringify(b.ast));
277+
const aDefaultValue = aAST.defaultValue ?? null;
278+
const bDefaultValue = bAST.defaultValue ?? null;
279+
280+
if (aDefaultValue === bDefaultValue) {return;}
281+
282+
const changeType = !aDefaultValue ? 'added'
283+
: !bDefaultValue ? 'removed' : 'changed';
284+
const classDeclarationType = getDeclarationType(a.getParent());
285+
context.report({
286+
key: `property-default-value-${changeType}`,
287+
message: changeType === 'added' ? `Default value "${bDefaultValue}" added to property "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}"`
288+
: changeType === 'removed' ? `Default value "${aDefaultValue}" removed from property "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}"`
289+
: `Default value changed from "${aDefaultValue}" to "${bDefaultValue}" in property "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}"`,
290+
element: a.getName()
291+
});
292+
}
293+
});
294+
295+
export const propertyComparerFactories = [propertyAdded, propertyRemoved, propertyTypeChanged, propertyValidatorChanged, propertyOptionalChanged, propertyDefaultChanged];

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,35 @@ const scalarValidatorChanged: ComparerFactory = (context) => ({
6969
}
7070
});
7171

72-
export const scalarDeclarationComparerFactories = [scalarDeclarationExtendsChanged, scalarValidatorChanged];
72+
const scalarDefaultValueChanged: ComparerFactory = (context) => ({
73+
compareScalarDeclaration: (a, b) => {
74+
if (!a || !b) {return;}
75+
76+
const aDefaultValue = a.getDefaultValue() ?? null;
77+
const bDefaultValue = b.getDefaultValue() ?? null;
78+
79+
if (aDefaultValue === bDefaultValue) {return;}
80+
81+
if (!aDefaultValue && bDefaultValue) {
82+
context.report({
83+
key: 'scalar-default-value-added',
84+
message: `Default value "${bDefaultValue}" added to scalar "${a.getName()}"`,
85+
element: a.getName()
86+
});
87+
} else if (aDefaultValue && !bDefaultValue) {
88+
context.report({
89+
key: 'scalar-default-value-removed',
90+
message: `Default value "${aDefaultValue}" removed from scalar "${a.getName()}"`,
91+
element: a.getName()
92+
});
93+
} else {
94+
context.report({
95+
key: 'scalar-default-value-changed',
96+
message: `Default value changed from "${aDefaultValue}" to "${bDefaultValue}" in scalar "${a.getName()}"`,
97+
element: a.getName()
98+
});
99+
}
100+
}
101+
});
102+
103+
export const scalarDeclarationComparerFactories = [scalarDeclarationExtendsChanged, scalarValidatorChanged, scalarDefaultValueChanged];

‎packages/concerto-analysis/test/fixtures/optional-to-required-a.cto‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace [email protected]
2+
3+
concept Thing {
4+
o String value optional
5+
}

‎packages/concerto-analysis/test/fixtures/optional-to-required-b.cto‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace [email protected]
2+
3+
concept Thing {
4+
o String value // Changed to required
5+
}

‎packages/concerto-analysis/test/fixtures/property-added.cto‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace [email protected]
2+
3+
concept Thing {
4+
o String name
5+
}

‎packages/concerto-analysis/test/fixtures/property-default-value-added.cto‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace [email protected]
2+
3+
concept Thing {
4+
o String name default="Fred"
5+
}

‎packages/concerto-analysis/test/fixtures/property-default-value-changed.cto‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace [email protected]
2+
3+
concept Thing {
4+
o String name default="NotFred"
5+
}

‎packages/concerto-analysis/test/fixtures/required-to-optional-a.cto‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace [email protected]
2+
3+
concept Thing {
4+
o String value // Required property
5+
}

‎packages/concerto-analysis/test/fixtures/required-to-optional-b.cto‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace [email protected]
2+
3+
concept Thing {
4+
o String value optional // Changed to optional
5+
}

0 commit comments

Comments
 (0)