Checking empty variables is reported as an error

sonarlint
javascript

(Tobias Schmidt) #1
  • versions used ( SonarLint for Eclipse 3.6.0.201806071228 )
    When checking if a variable is still initial, SonarLint will report it as a bug:
    “Refactor this code so that this expression does not always evaluate to true.”
    This occurs in the following code:
    		getParentPage : function(){
    			var pageName = this.oView;
    			var result
    			if(this.oView.mProperties.viewName == 'view.timeline.eventTimeline'){
    				return 'timelineOuter';
    			}
    			do {
    				//get the parent ID			
    				if(pageName.sId){
    			    	if(pageName.sId.includes('events.page')){
    						   result = 'events';
    					   }else if(pageName.sId.includes('timeline.page')){
    						   result = 'timeline';   
    					   }else if(pageName.sId.includes('eventdetails.page')){
    						   result = 'eventdetails';   
    					   }
    			    }
    				//get the parent object
    				if(pageName.oParent && !result){
    					pageName = pageName.oParent; 
    				}else{
    					return result; // no parent page set yet
    				}

    			}
    			-->while (!result); <--
    			return result;
    		},

Is there a prefered method to check that a variable is not initial? Otherwise, this appears to be a false positive.


(Pierre-Yves Nicolas) #2

Hi,

Why do you think it’s a false positive?
My understanding is that while (!result) is evaluated only when pageName.oParent && !result is truthy, otherwise return result exits the function. So !result is always true when evaluating while (!result).

Am I missing something?

Pierre-Yves


(Tobias Schmidt) #3

It is correct that the content of the variable “result” is checked twice.
What this function does, it retrieves the name of the parent page in a UI5 app.
The variable can stay empty multiple iterations of this loop since it goes p one level at a time and only stops once it could determine the name of the parent page.

(!result) is true until the variable “result” is set, which will end the loop.
The code does work, which is the reason why I posted this as a false positive.
In ABAP I would write “while result is initial”, not sure if the sonar addon prefers a different syntax for this check, I’d be happy to work with that.


(Pierre-Yves Nicolas) #4

The issue tells you “Refactor this code so that this expression does not always evaluate to true.”.
If you know that the condition of the while is always true, why don’t you change it to while(true)?
To me, it would be much more explicit, and wouldn’t look like a bug (and SonarLint would raise no issue).
That would also show that the last return statement (at the very end of the function) is never executed…


(Tobias Schmidt) #5

Well, the SonarLint message says " Refactor this code so that this expression does not always evaluate to true" But that is actually false.
The statement while(!result) is true until the variable “result” is populated.
As soon as the variable Result is populated, while(!result) will return false.
This is my exit condition for the loop. If I change it to while(true) I’d obviously have created an endless loop.
I’m not sure how I can clarify it better, the only thing that I could think of is recording how I debug this piece of code to prove that the statement works as designed and the message from sonarLint is actually false.

To illustrate this, use the following code, and test it (I know, the alert statement is depreciated, it is just to illustrate my point):
‘’’
var result;
if(!result) // this will return true, because result === null
{result = ‘Hello World!’}
if(!result) // this will return false, because result === ‘Hello World!’
{alert(‘OK, I was wrong’)}
else
{alert(‘I was right after all’)}’’’


(Tobias Schmidt) #6

Pierre-Yves Nicolas, you were right, I changed the method slightly, now the plugin does not mark it as a problem anymore, thanks four your patience :slight_smile:
This ticket can be considered closed.
Here is the final code:

getParentPage : function(){
		var pageName = this.oView;
		var result
		do {
			//get the parent ID			
			if(pageName.sId){
		    	if(pageName.sId.includes('events.page')){
					   result = 'events';
				   }else if(pageName.sId.includes('timeline.page')){
					   result = 'timeline';   
				   }else if(pageName.sId.includes('eventdetails.page')){
					   result = 'eventdetails';   
				   }
		    }
			//get the parent object
			if(pageName.oParent && !result){
				pageName = pageName.oParent; 
			}
		}
		while (!result);
		return result;
	}