What about Publish Diagnostics?

[manuEbg]

I am not yet sure if our modelicaSpecific JSON-RPC's should, in case of an Error, return a List of diagnostics or if they should add the Diagnostics into a List full of currently active Diagnostics, which is maintained by the Server and published to the Client via the publishDiagnostic Notification.

@CSchoel Maybe you can let me know how you think about this Topic?

[manuEbg]

My Idea would be that for Example the CheckModel method

• returns a String, that either contains the Result or the Error
• in case off Error adds Diagnostic to DiagnosticList
• Server is watching DiagnosticsList and every time something changes publishes the list

[CSchoel]

Yes, I think that is exactly the way you have to do this. And you have to keep in mind that Diagnostics are always tied to Documents, so if you have multiple classes defined in the same file, you will have to pool the Diagnostics objects related to those classes together in a single list. I think the relevant part of the LSP spec is this sentence:

Newly pushed diagnostics always replace previously pushed diagnostics. There is no merging that happens on the client side.

This sounds like you have to publish the full list every time that Diagnostics are added or removed.

[manuEbg]

I have not worked on this Issue yet.

[manuEbg]

I have started Working on this Idea.

My Idea would be that for Example the CheckModel method

* returns a String, that either contains the Result or the Error

* in case off Error adds Diagnostic to DiagnosticList

* Server is _watching_ DiagnosticsList and every time something changes publishes the list

I created an DiagnosticHandler, which should maintain Lists of Diagnostics. One List for every File. Additionally I created ModelicaDiagnosticClass. This class is Used to convert the Errors returned by omc-java-api into LSP-Diagnostics.

For now this only works with loadModel. The only Model I tested is the SyntaxError. https://github.com/MopeSWTP-SS21/MopeSWTP/tree/feature/Diagnostics

[manuEbg]

Today I continued my work on this Issue. Every time the Server finds an "Error:" in the returnString of checkModel, loadModel, loadFile a Diagnostic will be created and added to the DiagnosticsList that belongs to the DocumentUri. After that the whole List(all Uris) gets published. For now the only DiagnosticAttributes being used, are:

• Range

• Message

• Severity (Currently always Error)

  Maybe i should add Attribute source and fill it with value Modelica...

So I think the only thing missing to close https://github.com/mopeswtp-ss21/mopeswtp/issues/60 would be a proper way of cleaning the Diagnostics List. I thought about cleaning it before every checkModel, loadModel, loadFile call... But i am not sure if this would make (good) sense...

@CSchoel What do you think?

ps.: Code can be found in feature/Diagnostics

[CSchoel]

I did not yet have a look into the code, but here are my first thoughts:

Diagnostics should only be cleaned up if the related issue was actually fixed. In Modelica, this is not so easy to determine as we have multiple levels of checks. I suppose a good idea would be to keep separate DiagnosticLists for each level ("load", "check", "instantiate", "simulate") and then clean them whenever a check of the respective level is requested by the client:

• loadModel cleans "load"
• checkModel cleans "load" and "check"
• instantiateModel cleans "load", "check", and "instantiate"
• simulate (currently not part of our feature set) cleans all diagnostics for the file

What makes matters worse is the fact that loading/checking/instantiating/simulating a model might reveal errors in other files in a totally different part of the project. To correctly handle cleanup across multiple files, you must essentially keep track of all dependencies of a file/class to other files/classes. This is probably possible with the OM Scripting API, but quite complicated.

Alternatively, you could use the "test model" functionality to check each model in a package as deeply as possible (up until the "instantiate" level) and just issue a complete re-check of the whole project when one file within the project has changed. In fact, this seems preferable for me since model checking should be quite fast in the OMC and this behavior would mimic the way that the compiler of a normal programming language like Java works: Compile the whole project, report errors on the whole project. This would solve the cleanup issue, as a re-check would also lead to a complete cleanup (with the exception of "simulate"-level errors, which are definitely tied to one specific model and can only be checked when that specific model is simulated again).

[CSchoel]

With "test model" I was referring to the new functionality that we dubbed testClass in #54 .

[CSchoel]

For Prototype 3 it might be OK to use a more basic approach: Clean up all the diagnostic list before any loadModel or checkModel request is handled. This way we are sure that no old Diagnostics are kept on the server. From the clients point of view, Diagnostics can disappear without apparent reason, but at least the user will always see all Diagnostics relevant to the last command they issued.