[Discussion] Should Diagnostic#collectLeaves merge some parent diagnostics with children ?

[vhemery]

Hello folks,

I am encountering a use case where Diagnostic json objects returned by Diagnostic#collectLeaves method have a "-1" uri fragment.

This is due to this validation method :
org.eclipse.emf.ecore.util.EObjectValidator.validate_DataValueConforms(EObject, EAttribute, DiagnosticChain, Map<Object, Object>)
which creates a Diagnostic hierarchy like :

1. root Diagnostic
2. child Diagnostic on EObject : "The feature '...' of '...' contains a bad value"
3. grand-child Diagnostic on the value (without model element, but with an EDataType instead) : "The value '...' must be one of {'...'}"

The #collectLeaves method, as currently implemented, only returns the grand-child Diagnostic, without the associated model EObject.
Nevertheless the relevant information is actually not the grand-child Diagnostic, but rather the concatenation of child Diagnostic and grand-child Diagnostic.
In Theia, since the Problems view can not display an arborescence of Diagnostics like in RCP, I was wondering which was the best course of action for this issue.

Should we :

• A. Modify the Diagnostics hierarchy directly in the model server to merge such diagnostics and having only relevant leave Diagnostics ? (in org.eclipse.emfcloud.modelserver.emf.common.DefaultModelValidator.validate(String))
• B. Leave the Diagnostics hierarchy as is and modify the Diagnostic#collectLeaves method to merge leave Diangostics information with the information from their relevant parent Diagnostic ?
• C. Seek for another way to display Diagnostics, without the Problems view, or with heavy modifications to the Problems view ?

For the moment, I'll patch my application with the A approach, as it is the more easily overrideable one. But I think the B approach would probably be the one making more sense in a general context...

[Edit : the grand-child Diagnostic actually has an EObject, which is not a model element but the EDatatype which makes things worse]

[martin-fleck-at]

Hi @vhemery! Thank you for raising this.

We will have a look at this again when we have some more time since, as you mentioned, a custom implementation is quite easy to do. I'm not totally in favor of flattening the hierarchy automatically on the server just because Theia has this limitation since other modelserver clients may not have the same limitation. So Option B seems like a good choice.

Option B can be both implemented on the client or on the server. From a server point of view, we could provide an additional parameter that specifies the diagnostic format: tree and list. The tree would be the structure as it is provided now and the list would be a collapsed version of the individual branches as you suggested. That way we can leverage the server which may have a little bit more information to provide a more meaningful merge of a diagnostic hierarchy, e.g., to filter out things like EDatatype. What do you think?

[vhemery]

Honestly, I did it on the server only because I was lacking time.
IMHO, having a Diagnostic with the EDatatype as the only EObject data is a mistake in EObjectValidator.
In most cases, I think most Diagnostics should be autonomous and concern user-model elements. So the merge would concern only "erratic" Diagnostics which are not auto-sufficient.

I'm not convinced of the added value of having several implementations on the server. I'm more in favor of just modifying the client Diagnostic#collectLeaves method to merge such cases.
It should be quite easy to detect them if metamodels do not mix M2 (metamodel) an M3 (Ecore) levels. The "erratic" Diagnostics would be those which have an EObject instance of M3, but with a parent Diagnostic with an EObject instance of M2.
Since M3 is perfectly known, we've got this information covered by the client.

Unless of course you can see of another use case where a more complex merge should be performed. But I can't figure one which would not be a conception mistake.

[martin-fleck-at]

Thank you very much for your insight, we appreciate it! We'll have a closer look at this and the EObjectValidator when we start our work on this issue. We have not yet encountered a more complex use case ourselves so merging diagnostics on the client-side may be enough, as you mentioned.