Skip to content
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.

Don't throw NPE if accessing null references #10

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

abstraktor
Copy link
Contributor

Given I have a concept `Car` which has a nullable reference `owner`
And my `aCar` has no owner
When I call `aCar.getOwner()`, I erroneously got a NullPointerException
Because it is calling `MPSLanguageRegistry.Companion.getInstance(getINode().getReferenceTarget("owner"))`
It now returns null instead of throwing the NPE

thanks to christoph and sameh for debugging this together

Given I have a concept `Car` which has a nullable reference `owner`
And my `aCar` has no owner
When I call `aCar.getOwner()`, I erroneously got a NullPointerException
Because it is calling `MPSLanguageRegistry.Companion.getInstance(getINode().getReferenceTarget("owner"))`
It now returns null instead of throwing the NPE
@coolya
Copy link
Contributor

coolya commented Aug 22, 2022

I don't think we should solve this in the registry. You must never pass null into the registry. I think we should change the code generator.

Right now it emits this code when the reference has a cardinality of 1 if the reference is 0..1 it would do a null check before passing it to the registry. In your case the model is in an incomplete state. Right now the generated code for a reference with cardinality of 1 is annotated with @NotNull even with your change you would get an NPE just a little later in the code.

I think we need a switch in the code generator which allows for incomplete models and then makes basically everything @Nullable which is super annoying to work with that's why I think it should be a switch in the API Manifest.

@slisson
Copy link
Member

slisson commented Aug 22, 2022

The cardinality is just a convenient way to specify a constraint that can be used by the editor or model checker, but nothing you can enforce on API level. For convenience you could generate an additional method that does the null check and throws a meaningful exception, but I think you always need the method that allows you to read the "raw" value of a reference.
While editing the model it will most of the time be in an inconsistent state. You still want to run your analyses on it. Having to handle these null references is not inconvenient, but necessary.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants