Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 347296

Summary: Wrong generated java due to null-safe feature call (prop?.foo)
Product: [Modeling] TMF Reporter: Serano Colameo <serano.colameo>
Component: XtextAssignee: Project Inbox <tmf.xtext-inbox>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: hheitkoetter, sebastian.zarnekow, sven.efftinge
Version: 2.0.1Flags: sebastian.zarnekow: indigo+
Target Milestone: SR2   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Proposed patch sven.efftinge: iplog+

Description Serano Colameo CLA 2011-05-26 07:46:27 EDT
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.
Comment 1 Serano Colameo CLA 2011-05-26 07:47:11 EDT
bug is marked in the generated java code with // <=== Bug
Comment 2 Serano Colameo CLA 2011-05-26 08:01:03 EDT
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
Comment 3 Henning Heitkoetter CLA 2011-08-12 04:12:13 EDT
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.
Comment 4 Sebastian Zarnekow CLA 2011-08-12 04:21:27 EDT
Scheduled for SR2 to make sure we take a look at the patch. Thanks Henning!
Comment 5 Sven Efftinge CLA 2011-08-15 04:28:33 EDT
pushed the change and added a test case.
Comment 6 Serano Colameo CLA 2011-08-29 08:38:57 EDT
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;
  }
Comment 7 Sven Efftinge CLA 2011-08-29 09:06:42 EDT
It's not in 2.0.1, but in HEAD and will be contained in the upcoming 2.1.0
Comment 8 Karsten Thoms CLA 2017-09-19 17:34:59 EDT
Closing all bugs that were set to RESOLVED before Neon.0
Comment 9 Karsten Thoms CLA 2017-09-19 17:46:03 EDT
Closing all bugs that were set to RESOLVED before Neon.0