MopeSWTP-SS21 / MopeSWTP

MIT License
1 stars 0 forks source link

Improve ErrorHandling in LoadModel #61

Closed manuEbg closed 3 years ago

manuEbg commented 3 years ago

Report errors that can occur during loadModel

manuEbg commented 3 years ago

I have not worked on this Issue yet.

manuEbg commented 3 years ago

loading a non-existing top-level class, such as FooBar

In feature/Diagnostics the Client output after loading FooBar looks something like this:

 INFO [pool-1-thread-1] (MopeLSPClient.java:31) - DiagnosticLocation: /home/swtp/.openmodelica/libraries/index.json
Diagnostics: 
Diagnostic [
  range = Range [
    start = Position [
      line = 0
      character = 0
    ]
    end = Position [
      line = 0
      character = 0
    ]
  ]
  severity = Error
  code = null
  codeDescription = null
  source = null
  message = "Error: The package index /home/swtp/.openmodelica/libraries/index.json could not be parsed.\n"
  tags = null
  relatedInformation = null
  data = null
]

Result [result=false, error=Optional[[/home/swtp/.openmodelica/libraries/index.json:0:0-0:0:readonly] Error: The package index /home/swtp/.openmodelica/libraries/index.json could not be parsed.
Error: Failed to load package FooBar (default) using MODELICAPATH /usr/bin/../lib/omlibrary:/home/swtp/.openmodelica/libraries/:/home/swtp/modelica/LotkaVolterra.]]

Are there any Problems with that?

loading a non-existing class within an existing top-level class, such as Modelica.FooBar

The only Way I can think of to report this error is:

  1. loadModel(Modelica.FooBar)
  2. searchClassNames("Modelica.FooBar")
  3. check Result and Report Error.

are there any better Ideas?

syntax errors, such as a missing semicolon

I think this is already working...

CSchoel commented 3 years ago

The index.json error is a bug in OpenModelica that is being worked on. While it exists, it is fine to report it, since this is an issue in OpenModelica and not in Mo|E.

However, there should also be a second diagnostic item that reports the real error that the class could not be loaded.

CSchoel commented 3 years ago

Regarding non-existing classes, you may again look at MoST.jl for ideas.

CSchoel commented 3 years ago

Alternatively existClass (p. 34) might be a more "correct" and less hacky option.

manuEbg commented 3 years ago

After checking all these Methods , i decided to use existClass. So now the following Diagnostics are published

Diagnostic [
  range = Range [
    start = Position [
      line = 0
      character = 0
    ]
    end = Position [
      line = 0
      character = 0
    ]
  ]
  severity = Error
  code = null
  codeDescription = null
  source = null
  message = "Could not load Model FooBar"
  tags = null
  relatedInformation = null
  data = null
]
Diagnostic [
  range = Range [
    start = Position [
      line = 0
      character = 0
    ]
    end = Position [
      line = 0
      character = 0
    ]
  ]
  severity = Warning
  code = null
  codeDescription = null
  source = null
  message = "Could not load Model Modelica.FooBar, loaded top level model instead"
  tags = null
  relatedInformation = null
  data = null
]

Currently these Diagnostics are Published under Location path/to/project. I am not sure what would be the correct Location, but i thought about using the workspaceFolder

CSchoel commented 3 years ago

@manuEbg Quick question: Have you verified that this setup produces correct diagnostics when the top-level class cannot be loaded?

manuEbg commented 3 years ago

@CSchoel Do You mean that i have a Syntax Error in LV_Manu and try to load LV_Manu.FooBar? nope havent tried this yet

CSchoel commented 3 years ago

No, not just a Syntax error - although that might also be worth checking. What I mean is loading the model IDefinitelyDontExistAnywhereOnTheHardDrive or something like that. Just a model that cannot be loaded because it does not exist at all.

manuEbg commented 3 years ago

loading Model LV_Manu.FooBar with a Syntax Error in the Package resulted in the following Diagnostics:

 INFO [pool-1-thread-1] (MopeLSPClient.java:31) - DiagnosticLocation: /home/swtp/modelica/LotkaVolterra/LV_Manu/LV2Species.mo
Diagnostics: 
Diagnostic [
  range = Range [
    start = Position [
      line = 4
      character = 17
    ]
    end = Position [
      line = 5
      character = 2
    ]
  ]
  severity = Error
  code = null
  codeDescription = null
  source = null
  message = "Error: No viable alternative near token: ;\n"
  tags = null
  relatedInformation = null
  data = null
]

 INFO [pool-1-thread-1] (MopeLSPClient.java:31) - DiagnosticLocation: path/to/project
Diagnostics: 
Diagnostic [
  range = Range [
    start = Position [
      line = 0
      character = 0
    ]
    end = Position [
      line = 0
      character = 0
    ]
  ]
  severity = Error
  code = null
  codeDescription = null
  source = null
  message = "Could not load Model LV_Manu.FooBar"
  tags = null
  relatedInformation = null
  data = null
]

I think this looks quite good 😄

manuEbg commented 3 years ago

Shouldn't loading FooBar be the same as IDefinitelyDontExistAnywhereOnTheHardDrive? However here are the Diagnostics produced by loadModel(IDefinitelyDontExistAnywhereOnTheHardDrive) :)

DiagnosticLocation: path/to/project
Diagnostic [
  range = Range [
    start = Position [
      line = 0
      character = 0
    ]
    end = Position [
      line = 0
      character = 0
    ]
  ]
  severity = Error
  code = null
  codeDescription = null
  source = null
  message = "Could not load Model IDefinitelyDontExistAnywhereOnTheHardDrive"
  tags = null
  relatedInformation = null
  data = null
]
DiagnosticLocation: /home/swtp/.openmodelica/libraries/index.json
Diagnostic [
  range = Range [
    start = Position [
      line = 0
      character = 0
    ]
    end = Position [
      line = 0
      character = 0
    ]
  ]
  severity = Error
  code = null
  codeDescription = null
  source = null
  message = "Error: The package index /home/swtp/.openmodelica/libraries/index.json could not be parsed.\n"
  tags = null
  relatedInformation = null
  data = null
]
CSchoel commented 3 years ago

Looks fine. :+1:

But you are right: The escaped newline sign is worth a small new issue. I'll draft one up. I'm sure you're always delighted, when I flood you with issues on Monday. :laughing:

BTW: These examples would make for great unit tests. :wink: