From af7744781f12f512e4b508d8651906a2dc3a7808 Mon Sep 17 00:00:00 2001
From: Daimona Eaytoy <daimona.wiki@gmail.com>
Date: Tue, 20 Aug 2019 18:19:31 +0200
Subject: [PATCH] Allow if without else

Bug: T230727
Depends-On: I8e7f7710b8cb37ada8531b631456a3ce7b27ee45
Change-Id: I3b85087677607573f4fa68681735dc35348dcd87
---
 includes/parser/AFPTreeNode.php              |  4 +--
 includes/parser/AFPTreeParser.php            | 17 ++++--------
 includes/parser/AbuseFilterCachingParser.php |  9 ++++--
 includes/parser/AbuseFilterParser.php        | 29 ++++++++------------
 tests/parserTests/conditional-shortcircuit.t |  2 --
 tests/parserTests/ifthen.t                   |  4 ++-
 6 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/includes/parser/AFPTreeNode.php b/includes/parser/AFPTreeNode.php
index dea66b613..520ff60dc 100644
--- a/includes/parser/AFPTreeNode.php
+++ b/includes/parser/AFPTreeNode.php
@@ -23,8 +23,8 @@ class AFPTreeNode {
 	const ARRAY_APPEND = 'ARRAY_APPEND';
 
 	// CONDITIONAL represents both a ternary operator and an if-then-else-end
-	// construct.  The format is (condition, evaluated-if-true,
-	// evaluated-in-false), all tree nodes.
+	// construct.  The format is (condition, evaluated-if-true, evaluated-in-false).
+	// The first two are tree nodes, the last one can be a node, or null if there's no else.
 	const CONDITIONAL = 'CONDITIONAL';
 
 	// LOGIC is a logic operator accepted by AFPData::boolOp.  The format is
diff --git a/includes/parser/AFPTreeParser.php b/includes/parser/AFPTreeParser.php
index f66d82fc5..c7cc87079 100644
--- a/includes/parser/AFPTreeParser.php
+++ b/includes/parser/AFPTreeParser.php
@@ -236,19 +236,12 @@ class AFPTreeParser {
 
 			$valueIfTrue = $this->doLevelConditions();
 
-			if ( !( $this->mCur->type === AFPToken::TKEYWORD && $this->mCur->value === 'else' ) ) {
-				throw new AFPUserVisibleException( 'expectednotfound',
-					$this->mPos,
-					[
-						'else',
-						$this->mCur->type,
-						$this->mCur->value
-					]
-				);
+			if ( $this->mCur->type === AFPToken::TKEYWORD && $this->mCur->value === 'else' ) {
+				$this->move();
+				$valueIfFalse = $this->doLevelConditions();
+			} else {
+				$valueIfFalse = null;
 			}
-			$this->move();
-
-			$valueIfFalse = $this->doLevelConditions();
 
 			if ( !( $this->mCur->type === AFPToken::TKEYWORD && $this->mCur->value === 'end' ) ) {
 				throw new AFPUserVisibleException( 'expectednotfound',
diff --git a/includes/parser/AbuseFilterCachingParser.php b/includes/parser/AbuseFilterCachingParser.php
index b984af084..11aae5b48 100644
--- a/includes/parser/AbuseFilterCachingParser.php
+++ b/includes/parser/AbuseFilterCachingParser.php
@@ -240,11 +240,16 @@ class AbuseFilterCachingParser extends AbuseFilterParser {
 				list( $condition, $valueIfTrue, $valueIfFalse ) = $node->children;
 				$condition = $this->evalNode( $condition );
 				if ( $condition->toBool() ) {
-					$this->discardWithHoisting( $valueIfFalse );
+					if ( $valueIfFalse !== null ) {
+						$this->discardWithHoisting( $valueIfFalse );
+					}
 					return $this->evalNode( $valueIfTrue );
 				} else {
 					$this->discardWithHoisting( $valueIfTrue );
-					return $this->evalNode( $valueIfFalse );
+					return $valueIfFalse !== null
+						? $this->evalNode( $valueIfFalse )
+						// We assume null as default if the else is missing
+						: new AFPData( AFPData::DNULL );
 				}
 
 			case AFPTreeNode::ASSIGNMENT:
diff --git a/includes/parser/AbuseFilterParser.php b/includes/parser/AbuseFilterParser.php
index d3263f9e6..141ba07af 100644
--- a/includes/parser/AbuseFilterParser.php
+++ b/includes/parser/AbuseFilterParser.php
@@ -508,7 +508,8 @@ class AbuseFilterParser {
 			$this->move();
 
 			$r1 = new AFPData( AFPData::DEMPTY );
-			$r2 = new AFPData( AFPData::DEMPTY );
+			// DNULL is assumed as default in case of a missing else
+			$r2 = new AFPData( AFPData::DNULL );
 
 			$isTrue = $result->toBool();
 
@@ -520,24 +521,16 @@ class AbuseFilterParser {
 				$this->mShortCircuit = $scOrig;
 			}
 
-			if ( !( $this->mCur->type === AFPToken::TKEYWORD && $this->mCur->value === 'else' ) ) {
-				throw new AFPUserVisibleException( 'expectednotfound',
-					$this->mCur->pos,
-					[
-						'else',
-						$this->mCur->type,
-						$this->mCur->value
-					]
-				);
-			}
-			$this->move();
+			if ( $this->mCur->type === AFPToken::TKEYWORD && $this->mCur->value === 'else' ) {
+				$this->move();
 
-			if ( $isTrue ) {
-				$scOrig = wfSetVar( $this->mShortCircuit, $this->mAllowShort, true );
-			}
-			$this->doLevelConditions( $r2 );
-			if ( $isTrue ) {
-				$this->mShortCircuit = $scOrig;
+				if ( $isTrue ) {
+					$scOrig = wfSetVar( $this->mShortCircuit, $this->mAllowShort, true );
+				}
+				$this->doLevelConditions( $r2 );
+				if ( $isTrue ) {
+					$this->mShortCircuit = $scOrig;
+				}
 			}
 
 			if ( !( $this->mCur->type === AFPToken::TKEYWORD && $this->mCur->value === 'end' ) ) {
diff --git a/tests/parserTests/conditional-shortcircuit.t b/tests/parserTests/conditional-shortcircuit.t
index 10438cbb5..39b2f8aea 100644
--- a/tests/parserTests/conditional-shortcircuit.t
+++ b/tests/parserTests/conditional-shortcircuit.t
@@ -57,8 +57,6 @@ false & (
 var9 := 'foo';
 if ( 1=== 1 ) then
   ( false & ( var9 := 'foobar' ) )
-else
-( 0 )
 end;
 
 var1 === 1 &
diff --git a/tests/parserTests/ifthen.t b/tests/parserTests/ifthen.t
index c22a39470..ce3e0aa34 100644
--- a/tests/parserTests/ifthen.t
+++ b/tests/parserTests/ifthen.t
@@ -1,2 +1,4 @@
 (if 1 then 2 else 3 end) === 2 &
-(if false then 2 else 3 end) === 3
+(if false then 2 else 3 end) === 3 &
+(if true then 3 end) === 3 &
+(if false then 3 end) === null