Checker should NOT check code which come from imported files

[ffleurey]

And crashes when it tries to add issues to element which are imported from other files.

I have disabled the problematic check for now. It is at the bottom of file: language/thingml/src/org/thingml/xtext/validation/checks/ThingsUsage.xtend

Exception is 208 [main] ERROR text.validation.CompositeEValidator - Error executing EValidator java.lang.IllegalArgumentException: You can only add issues for EObjects contained in the currently validated resource 'file:/home/franck/checkout/Tryggi3D/software/bluetooth/lib/ble/ATTProxy.thingml'. But the given EObject was contained in 'file:/home/franck/checkout/Tryggi3D/software/bluetooth/lib/ble/UUID.thingml' at org.eclipse.xtext.validation.AbstractDeclarativeValidator.checkIsFromCurrentlyCheckedResource(AbstractDeclarativeValidator.java:571) at org.eclipse.xtext.validation.AbstractDeclarativeValidator.acceptInfo(AbstractDeclarativeValidator.java:587) at org.eclipse.xtext.validation.AbstractDeclarativeValidator.info(AbstractDeclarativeValidator.java:366) at org.thingml.xtext.validation.checks.ThingsUsage.lambda$5(ThingsUsage.java:171) at java.util.ArrayList.forEach(ArrayList.java:1249) at org.thingml.xtext.validation.checks.ThingsUsage.checkPSM(ThingsUsage.java:174)

[jakhog]

Aha, that is probably the cause of some issues we've had while trying it out. That means, that we need to double-check what resource an object belongs to before posting any errors...

I think, in nature, the checker runs only on a single resource (file) when it is edited or saved. (It doesn't really make sense to highlight things in files that are not open in the editor).

But it might mean that we need to move some of the checks

[jakhog]

By the way, the checker that is run at compilation, runs through all the resources. So it will find errors and warnings in imported files as well...

[ffleurey]

Yes and better we should avoid cheking object which do not belong to the current resources because it is a wast of CPU. That is why I did not fix it by adding an if before posting the error.

[ffleurey]

Yes that is why only the current ressource should be considered by the checker

[brice-morin]

Not sure I fully understood (or agree) with the last comment. Before compilation, it makes sense to check everything that is used directly or not by the instances defined in the configuration to be compiled, which can involve imported things in many files, so that we do not produce code that will anyway crash. In many cases, the file we compile only contains the configuration, where not so many things can go wrong. We could of course report errors on the instances when it is an error on its type, but then it might be several errors/warning on each instances, which will not be easy to trace back. The check before compilation should give the line and path to the file where the error actually is.

But for the live checks in the editor, I agree.

We will look into that.

[brice-morin]

I had a look at the different rules and it now seems OK, though it might still be some surprises here or there. We should just continue using ThingML and report any checker error :-)

[ffleurey]

Very good. I think we agree that all the program has to be checked before compiling. What I want to stress is that we should make sure that it is check once and only once. I think that this is (was?) one of the main cause for the checker to be slow: It was checking everything all over again for each file/resource in the project.

[brice-morin]

Once and only once, yes and no. Most checks are done whenever we save the file (@check(NORMAL)), and some checks are "live" (@check(FAST)), typically to detect duplicated elements in a "namespace", so that it can be fixed on the fly. Those checks should have a file scope. So it can potentially be checked multiple times, depending on XText strategies and/or how often you save files.

As for the checks before compilation, they check everything once and only once, navigating the model once and only once (or at least that is what we assume, unless XText guys really did something stupid here). Previously, the model was checked once and only once before compilation, but each check was basically re-browsing the whole model all over again from the top to find the element that were relevant.

We did not benchmark old versus new, but we can easily feel on big project that the new checker is much faster (no lag when typing, and quite fast to save files). When checking the whole model before compilation of a big project, it takes about 200ms, while the compilation itself takes about 500ms.