Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 326364 - NullPointerException in DefaultEcoreElementFactory
Summary: NullPointerException in DefaultEcoreElementFactory
Status: CLOSED FIXED
Alias: None
Product: TMF
Classification: Modeling
Component: Xtext (show other bugs)
Version: 1.0.1   Edit
Hardware: PC Windows 7
: P3 major (vote)
Target Milestone: M3   Edit
Assignee: Jan Koehnlein CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-27 19:02 EDT by Nikolay Krasko CLA
Modified: 2017-09-19 17:41 EDT (History)
5 users (show)

See Also:
jan: indigo+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolay Krasko CLA 2010-09-27 19:02:22 EDT
I have the grammar file:

grammar eclihx.HXML hidden(WS)

generate hXML "http://www.eclihx.org/hxml/HXML"

import "http://www.eclipse.org/emf/2002/Ecore" as ecore

File:
	TwoIntsOption;

TwoIntsOption :
	'-ints' (first = IntValue) COLON (second = IntValue);
	
IntValue returns ecore::EInt:
	DIGIT+;

terminal DIGIT: '0'..'9';
terminal COLON: ':';
terminal WS : (' '|'\t'|'\r'|'\n')+;

Parser and lexer are generated using default project mwe2 file (I can post it if neccessary).

When launching eclipse with the generated plug-in included I'm trying to type the following file:

-ints 12:14

But after typing
-ints 12:

The parser throws a NullReferenceException:

1697202 [Worker-9] WARN  org.eclipse.xtext.parser.DefaultEcoreElementFactory  - 
java.lang.NullPointerException
	at eclihx.hXML.impl.TwoIntsOptionImpl.eSet(TwoIntsOptionImpl.java:173)
	at org.eclipse.emf.ecore.impl.BasicEObjectImpl.eSet(BasicEObjectImpl.java:1081)
	at org.eclipse.xtext.parser.DefaultEcoreElementFactory.set(DefaultEcoreElementFactory.java:67)
	at org.eclipse.xtext.parser.antlr.AbstractInternalAntlrParser.set(AbstractInternalAntlrParser.java:285)
	at eclihx.parser.antlr.internal.InternalHXMLParser.ruleTwoIntsOption(InternalHXMLParser.java:267)
	at eclihx.parser.antlr.internal.InternalHXMLParser.ruleFile(InternalHXMLParser.java:124)
	at eclihx.parser.antlr.internal.InternalHXMLParser.entryRuleFile(InternalHXMLParser.java:85)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at org.eclipse.xtext.parser.antlr.AbstractInternalAntlrParser.parse(AbstractInternalAntlrParser.java:502)
	at eclihx.parser.antlr.HXMLParser.parse(HXMLParser.java:32)
	at org.eclipse.xtext.parser.antlr.AbstractAntlrParser.parse(AbstractAntlrParser.java:82)
	at org.eclipse.xtext.parser.impl.PartialParsingHelper.reparse(PartialParsingHelper.java:90)
	at org.eclipse.xtext.parser.antlr.AbstractAntlrParser.doReparse(AbstractAntlrParser.java:100)
	at org.eclipse.xtext.parser.AbstractParser.reparse(AbstractParser.java:77)
	at org.eclipse.xtext.resource.XtextResource.update(XtextResource.java:197)
	at org.eclipse.xtext.ui.editor.reconciler.XtextReconcilerUnitOfWork.process(XtextReconcilerUnitOfWork.java:50)
	at org.eclipse.xtext.ui.editor.reconciler.XtextReconcilerUnitOfWork.process(XtextReconcilerUnitOfWork.java:1)
	at org.eclipse.xtext.util.concurrent.IUnitOfWork$Void.exec(IUnitOfWork.java:36)
	at org.eclipse.xtext.util.concurrent.IStateAccess$AbstractImpl.modify(IStateAccess.java:57)
	at org.eclipse.xtext.ui.editor.model.XtextDocument$XtextDocumentLocker.modify(XtextDocument.java:161)
	at org.eclipse.xtext.ui.editor.model.XtextDocument.modify(XtextDocument.java:74)
	at org.eclipse.xtext.ui.editor.reconciler.XtextDocumentReconcileStrategy.reconcile(XtextDocumentReconcileStrategy.java:27)
	at org.eclipse.xtext.ui.editor.reconciler.XtextReconciler.run(XtextReconciler.java:233)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)

And writes and comment to the error:

A NullPointerException occured. This indicates a missing value converter or a bug in its implementation.

For me there shouldn't be no attempt to convert second int value.
Comment 1 Jan Koehnlein CLA 2010-09-28 12:02:59 EDT
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.
Comment 2 Sebastian Zarnekow CLA 2010-09-28 12:05:14 EDT
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.
Comment 3 Jan Koehnlein CLA 2010-09-29 04:04:32 EDT
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.
Comment 4 Knut Wannheden CLA 2010-09-29 06:23:09 EDT
See also bug 280873 where this was discussed and the current strategy was implemented.
Comment 5 Sven Efftinge CLA 2010-09-29 07:27:01 EDT
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.
Comment 6 Sven Efftinge CLA 2010-09-29 07:27:29 EDT
I meant don't call the converter and don't set/add the value.
Comment 7 Sebastian Zarnekow CLA 2010-09-29 16:52:08 EDT
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.
Comment 8 Jan Koehnlein CLA 2010-09-30 07:25:46 EDT
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()?
Comment 9 Jan Koehnlein CLA 2010-10-13 09:48:13 EDT
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.
Comment 10 Karsten Thoms CLA 2017-09-19 17:29:36 EDT
Closing all bugs that were set to RESOLVED before Neon.0
Comment 11 Karsten Thoms CLA 2017-09-19 17:41:01 EDT
Closing all bugs that were set to RESOLVED before Neon.0