| Summary: | Wrong generated java due to null-safe feature call (prop?.foo) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Serano Colameo <serano.colameo> | ||||
| Component: | Xtext | Assignee: | Project Inbox <tmf.xtext-inbox> | ||||
| Status: | CLOSED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | hheitkoetter, sebastian.zarnekow, sven.efftinge | ||||
| Version: | 2.0.1 | Flags: | sebastian.zarnekow:
indigo+
|
||||
| Target Milestone: | SR2 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
bug is marked in the generated java code with // <=== Bug actually there are two bugs:
public String error() {
String _xblockexpression = null;
{
String value = "test";
this==null?(String)null:this.doSomething(value); // <=== Bug
value = null;
String _doSomething = this==null?(String)null:this.doSomething(value); // <==
// this would be correct:
String _doSomething = value==null?(String)null:this.doSomething(value);
_xblockexpression = (_doSomething);
}
return _xblockexpression;
}
however, to me it looks that the null-safe feature call operator does not work at all.
~serano
Created attachment 201375 [details]
Proposed patch
As I've hit the same problem, I took a stab at fixing the issue.
Note that there are two problems apparent in your example: the first being that a statement made up with the conditional operator ("a ? b : c") is no valid expression in Java, leading to the compilation error at the first line marked with "<=== Bug". While the transformation of the null-safe feature call should probably address this, too, you can work around that by assigning the expression to a variable ("val tmp = value?.doSomething").
Thus, I did not further investigate this part of this issue.
The second problem, the null-check on the "wrong" variable ("this" instead of "value"), is due to the fact that FeatureCallCompiler.appendReceiver generates a check on the actual receiver in Java, which does not have to be the same as the receiver in Xbase. For example, the method "doSomething" in the example is an instance method of class Test, so an instance of this class ("this") will be the actual receiver in Java, instead of "value".
I believe the present behavior does not conform with the intention of the null-safe feature call in Xbase, because it should operate on the logical model present in Xbase, not on how it's eventually transformed to Java - thus, I'd prefer the check "value == null" as well; "x.member?.feature" should always generate a null-check against "x.member".
Thus, I've created a patch that fixes this issue (see attachment). Note that it fixes a related issue with static methods as well: for the reasons outlined above, from a Xtend perspective, it should not make a difference if the call is made to a static feature. All that matters is the receiver in Xbase.
An Xbase expression like "value.bytes?.map(b | b.intValue)" should check the return value of "value.bytes", not silently ignore the null-safe operator because "map" is a static method.
Scheduled for SR2 to make sure we take a look at the patch. Thanks Henning! pushed the change and added a test case. Still not working with xtext 2.0.1
serano
---------------------------------
xtend2:
def StringConcatenation equalsSection(Structure s) '''
«s?.superIfStructure?.equalsSection»
...
'''
expected java:
public StringConcatenation constructorParameterSection(final Structure s) throws RuntimeException {
StringConcatenation _builder = new StringConcatenation();
Structure _superIfStructure = s==null?(Structure)null:this.getSuperIfStructure(s);
StringConcatenation _constructorParameterSection =
_superIfStructure==null?(StringConcatenation)null:
_superIfStructure.constructorParameterSection(_superIfStructure);
...
generated java:
public StringConcatenation constructorParameterSection(final Structure s) throws RuntimeException {
StringConcatenation _builder = new StringConcatenation();
Structure _superIfStructure = this==null?(Structure)null:this.getSuperIfStructure(s);
StringConcatenation _constructorParameterSection =
this==null?(StringConcatenation)null:this.constructorParameterSection(_superIfStructure);
...
another example:
def dispatch boolean isMulti(Parameter p) {
p?.cardinality?.isMulti
}
protected boolean _isMulti(final Parameter p) {
Cardinality _cardinality = p==null?(Cardinality)null:p.getCardinality();
boolean _isMulti = this==null?false:this.isMulti(_cardinality);
return _isMulti;
}
It's not in 2.0.1, but in HEAD and will be contained in the upcoming 2.1.0 Closing all bugs that were set to RESOLVED before Neon.0 Closing all bugs that were set to RESOLVED before Neon.0 |
Build Identifier: Xtend2 2.0.0.v201105241033 xtend2: class Test { def error() { var value = "test" value?.doSomething value = null value?.doSomething } def doSomething(String value) { println('Val= '+value) } } generated java: package com.csg.cs.tools.mdgen.idl.importer; import org.eclipse.xtext.xbase.lib.InputOutput; import org.eclipse.xtext.xbase.lib.StringExtensions; @SuppressWarnings("all") public class Test { public String error() { String _xblockexpression = null; { String value = "test"; this==null?(String)null:this.doSomething(value); // <=== Bug value = null; String _doSomething = this==null?(String)null:this.doSomething(value); _xblockexpression = (_doSomething); } return _xblockexpression; } public String doSomething(final String value) { String _operator_plus = StringExtensions.operator_plus("Str = ", value); String _println = InputOutput.<String>println(_operator_plus); return _println; } } Reproducible: Always Steps to Reproduce: 1. see details 2. 3.