accordproject / concerto

Business schema language and runtime
https://concerto.accordproject.org
Apache License 2.0
121 stars 106 forks source link

making a concept abstract or changing its inheritance should be considered a breaking change #937

Open DS-AdamMilazzo opened 1 week ago

DS-AdamMilazzo commented 1 week ago

Bug Report 🐛

  1. When comparing two models, Concerto doesn't consider changing a concept from concrete to abstract to be a breaking change, but it is because existing data using the concrete type will no longer validate.
  2. It also doesn't detect changes in inheritance, even when that changes the fields in a concept.

Expected Behavior

When comparing two models where the new model changes a concept from concrete to abstract, the change should be detected as a major (breaking) change. When changing the other way, from abstract to concrete, the change should be detected as a minor or patch (non-breaking) change.

When comparing two models where the new model changes a concept's inheritance, the change should be detected as a major (breaking) change. This could potentially be relaxed to a minor change if the new base type is derived (directly or indirectly) from the old base type and the newly intervening base types only add fields that would be allowed to be added directly to the concept at issue (i.e. optional fields whose names don't collide with existing fields).

Current Behavior

Currently, changing a concept between abstract and concrete is not detected as a model change, and neither is changing a concept's inheritance.

Possible Solution

Detect a change to a concept's abstractness in the model comparer and return one of two new result codes. Detect a change in base class and return a new result code (or two new result codes if you want to get fancy).

Steps to Reproduce

First, create two model files. A.cto:

namespace N@1.0.0
concept B { o String }
concept C extends B { }

and B.cto:

namespace N@1.1.0
concept B { o String }
abstract concept C { }

Then, run concerto compare --old A.cto --new B.cto No differences are reported.

Context (Environment)

It just seems like a bug that needs fixing.

Desktop

Detailed Description

Possible Implementation

In classDeclarationTypeChanged, check if the two concepts are equally abstract and if not return new comparison result type-made-concrete (minor or patch) or type-made-abstract (major).

Also check if the base type has changed and if so return a new comparison result supertype-changed (major) (or concept-extends-changed to parallel the existing scalar-extends-changed result). If you want to get fancy, you could allow some base type changes that are not breaking, as described above in the "Expected Behavior" section.