TelluIoT / ThingML

The ThingML modelling language
https://github.com/TelluIoT/ThingML
Apache License 2.0
101 stars 32 forks source link

Improve Error / Warning reporting on checker #216

Closed ffleurey closed 6 years ago

ffleurey commented 6 years ago

The checker has a tendency to make my whole program yellow/red, especially when they are not finished. See attached image. I see two problems here:

1/ For warnings on properties and functions, the error or warning should be reported on the function name so that only the name gets underlined. I think the same problem occurs also in other places in the checker.

2/ The code unreachable static analysis is not clever enough so it should be a warning.

checker

brice-morin commented 6 years ago

For 1, it is somehow handled by XText/Xtend, but it might be possible to do something, though we haven't figured out how, yet.

For 2, I guess I will not check unreachable code when there are are externs. Otherwise, this check seemed clever enough.

ffleurey commented 6 years ago

For 1, I have done it before but I do not remember the details... I'll check.

ffleurey commented 6 years ago

One more optimization to think about: Do not run the checker when there are parsing errors. This will avoid running the checker all the time when typing in the editor. It contributes to slow down the editor and constantly annotate the model with a lot of checker warnings and errors which are not relevant (because the program does not parse in the first place).

brice-morin commented 6 years ago

Is the checker still slow on your industry PC? We made some tests on reasonably big ThingML projects and it was acceptable... Anyway, I will switch all checks to @Check(NORMAL), which should only check on save.

ffleurey commented 6 years ago

No , it is not so slow and it is good that it runs all the time but the point is that before running the checker you can just check if there are already errors on the model. If yes it mean that the model does not parse or something is not resolved and there is no point in checking the model further.

jakhog commented 6 years ago

I don't think we have any control over when the checker is run in Eclipse. Further, we get passed properly parsed objects into the checker functions, so I don't think Eclipse/XText calls it if it can't create a valid model (or at least parts of a model)?

jakhog commented 6 years ago

For 1/: Shouldn't this just be to change the EObject source, EStructuralFeature feature calls to warning(...) or error(...) in the checks?

E.g. for the example above, I guess we should pass it the function itself as a source, and then NAMEDELEMENT_NAME as the feature?

jakhog commented 6 years ago

I've fixed the highlighting problems in the places I could find. But probably not everywhere.

However, it's quite a simple fix. So it's easier if we just do it when we find concrete examples where it's not perfect...

brice-morin commented 6 years ago

OK, so I guess we can close this issue. And fix the remaining ones as we encounter them, now that we have a rather easy procedure to adjust the scope of highlights. Just re-open if I missed something in this all too unfocused issue...