Bugzilla – Full Text Bug Listing
| Summary: | NullPointerException in DefaultEcoreElementFactory | ||
|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Nikolay Krasko <goodwinnk> |
| Component: | Xtext | Assignee: | Jan Koehnlein <jan> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P3 | CC: | jan, knut.wannheden, sebastian.zarnekow, sven.efftinge, tmf.xtext-inbox |
| Version: | 1.0.1 | Flags: | jan:
indigo+
|
| Target Milestone: | M3 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
|
Description
Nikolay Krasko
The exception also happens if the first int is missing, as both ints are mandatory. The default value converter used for EInt (EFactoryValueConverter) is not null safe. We should change that for the primitive types. I guess the problem is, that the value converter actually returns null but null may not be passed to eSet(..) if the data type is backed by a primitive. Autoboxing will fail with a NPE. Yes, I was not clear in my previous comment. I added a guard in EFactoryValueConverter and a test in HEAD. We don't have a clear contract for empty strings/null passed into an IValueConverter. Debugging shows the parser has already determined up the syntax error. It is somehow strange the value converter is called here anyway. We should try to avoid this on the caller's side to get rid of the second error message. See also bug 280873 where this was discussed and the current strategy was implemented. I think we shouldn't call the value converter for null values in DefaultEcoreElementFactory at all, because they indicate some problem which was already found by the parser. Still ValueConverters for primitives shouldn't return null values. I meant don't call the converter and don't set/add the value. I agree, if the parser is recovering we should not call add / set on the factory. Furthermore we should ensure that no default implementation of the value converter interface returns null for primitives. The problem is how to detect the parser is in recovery mode. When parsing a datatype rule, we instantiate an AntlrDatatypeRuleToken that aggregates the parsed tokens in the rule. Returning with an empty token can either mean that a datatype rule has only optional tokens and so represent a valid result, or that the parser failed to match any token and is recovering. We should also be careful about the meaning of null returned by IValueConverters, as they must be symmetrically implemented. I am not sure what the serializer expects here. We should decide * Is null or "" a valid result of a datatype rule? * Is null a valid input for IValueConverter.toValue()? Results of internal discussion: Null or "" are valid results of a datatype rule ad therefore null is a valid input for IValueConverter.toValue(), too. I added a guard in the DefaultEcoreElementFactory to not set primitive features to null. Closing all bugs that were set to RESOLVED before Neon.0 Closing all bugs that were set to RESOLVED before Neon.0 |