Concerto.instanceOf is misleading or buggy: It can be successful on non valid data

[jeromesimeon]

Bug Report 🐛

The new Concerto.instanceOf call works on non-valid data so can easily be confusing and give the impression that an object is an instance of a class even when it isn't.

[This could be considered "as designed" but I think the call and doc are misleading and will lead to confusion and users believing they have performed validation when they haven't.]

Example:

const ModelLoader = require('./lib/modelloader');
const Concerto = require('./lib/concerto');

const cto = `
namespace test
concept C {
  o String foo
}
concept C1 extends C {
}
concept C2 extends C1 {
}
concept D {
  o String bar
}
concept D1 extends D {
}
concept D2 extends D1 {
}
`;
const json1 = { // valid
  $class: 'test.C2',
  foo: 'bliblablu'
};
const json2 = { // valid
  $class: 'test.D2',
  bar: 'bliblablu'
};
const json3 = { // not valid
  $class: 'test.C2',
  bar: 'bliblablu'
};

const tests = [
  { obj: json1, $class: 'test.C' },
  { obj: json1, $class: 'test.D' },
  { obj: json2, $class: 'test.C' },
  { obj: json2, $class: 'test.D' },
  { obj: json3, $class: 'test.C' },
  { obj: json3, $class: 'test.D' },
];

async function run() {
  const mm = await ModelLoader.loadModelManagerFromModelFiles([cto],['test.cto']);
  const concerto = new Concerto(mm);

  tests.forEach(({ obj, $class }) => {
    console.log(`JSON ${JSON.stringify(obj)} INSTANCE OF ${$class} =  ${concerto.instanceOf(obj,$class)}`);
  });
}
// Launch!
run()

Run:

zsh-5.8$ node test.js 
JSON {"$class":"test.C2","foo":"bliblablu"} INSTANCE OF test.C =  true
JSON {"$class":"test.C2","foo":"bliblablu"} INSTANCE OF test.D =  false
JSON {"$class":"test.D2","bar":"bliblablu"} INSTANCE OF test.C =  false
JSON {"$class":"test.D2","bar":"bliblablu"} INSTANCE OF test.D =  true
JSON {"$class":"test.C2","bar":"bliblablu"} INSTANCE OF test.C =  true // returns true even if the object isn't an instance of `test.C`
JSON {"$class":"test.C2","bar":"bliblablu"} INSTANCE OF test.D =  false

Expected Behavior

Instance of should fail for non valid data.

Current Behavior

Instance of sometimes returns true for non valid data.

Possible Solution

I think the current call is fundamentally not between an object and a class, but simply looking into the type hierarchy. I would either:

1. change the call so it fails (throws an error?) on non-valid data
2. change the call so it is between two classes rather than between an object and a class

[mttrbrts]

This is very odd, the implementation appears to ignore all properties (except for $class) entirely!
https://github.com/accordproject/concerto/blob/master/packages/concerto-core/lib/concerto.js#L217