[2.7.0 target] Fix issue where mixing different types of conditionals (…

…SkriptLang#5932)

Fix issue where mixing different types of conditionals sometimes isn't allowed (SkriptLang#5872)

(cherry picked from commit 639bcf7)

Co-authored-by: Pikachu920 <[email protected]>

src/main/java/ch/njol/skript/sections/SecConditional.java

@@ -118,21 +118,34 @@ public boolean init(Expression<?>[] exprs,
 		multiline = parseResult.regexes.size() == 0 && type != ConditionalType.ELSE;
 
 		// ensure this conditional is chained correctly (e.g. an else must have an if)
-		SecConditional lastIf;
 		if (type != ConditionalType.IF) {
-			lastIf = getClosestIf(triggerItems);
-			if (lastIf == null) {
-				if (type == ConditionalType.ELSE_IF) {
-					Skript.error("'else if' has to be placed just after another 'if' or 'else if' section");
-				} else if (type == ConditionalType.ELSE) {
-					Skript.error("'else' has to be placed just after another 'if' or 'else if' section");
-				} else if (type == ConditionalType.THEN) {
+			if (type == ConditionalType.THEN) {
+				/*
+				 * if this is a 'then' section, the preceding conditional has to be a multiline conditional section
+				 * otherwise, you could put a 'then' section after a non-multiline 'if'. for example:
+				 *  if 1 is 1:
+				 *    set {_example} to true
+				 *  then: # this shouldn't be possible
+				 *    set {_uh oh} to true
+				 */
+				SecConditional precedingConditional = getPrecedingConditional(triggerItems, null);
+				if (precedingConditional == null || !precedingConditional.multiline) {
 					Skript.error("'then' has to placed just after a multiline 'if' or 'else if' section");
+					return false;
+				}
+			} else {
+				// find the latest 'if' section so that we can ensure this section is placed properly (e.g. ensure a 'if' occurs before an 'else')
+				SecConditional precedingIf = getPrecedingConditional(triggerItems, ConditionalType.IF);
+				if (precedingIf == null) {
+					if (type == ConditionalType.ELSE_IF) {
+						Skript.error("'else if' has to be placed just after another 'if' or 'else if' section");
+					} else if (type == ConditionalType.ELSE) {
+						Skript.error("'else' has to be placed just after another 'if' or 'else if' section");
+					} else if (type == ConditionalType.THEN) {
+						Skript.error("'then' has to placed just after a multiline 'if' or 'else if' section");
+					}
+					return false;
 				}
-				return false;
-			} else if (!lastIf.multiline && type == ConditionalType.THEN) {
-				Skript.error("'then' has to placed just after a multiline 'if' or 'else if' section");
-				return false;
 			}
 		} else {
 			// if this is a multiline if, we need to check if there is a "then" section after this
@@ -150,7 +163,6 @@ public boolean init(Expression<?>[] exprs,
 					return false;
 				}
 			}
-			lastIf = null;
 		}
 
 		// if this an "if" or "else if", let's try to parse the conditions right away
@@ -231,9 +243,11 @@ public boolean init(Expression<?>[] exprs,
 			return true;
 
 		if (type == ConditionalType.ELSE) {
+			SecConditional precedingIf = getPrecedingConditional(triggerItems, ConditionalType.IF);
+			assert precedingIf != null; // at this point, we've validated the section so this can't be null
 			// In an else section, ...
 			if (hasDelayAfter.isTrue()
-					&& lastIf.hasDelayAfter.isTrue()
+					&& precedingIf.hasDelayAfter.isTrue()
 					&& getElseIfs(triggerItems).stream().map(SecConditional::getHasDelayAfter).allMatch(Kleenean::isTrue)) {
 				// ... if the if section, all else-if sections and the else section have definite delays,
 				//  mark delayed as TRUE.
@@ -314,21 +328,28 @@ private Kleenean getHasDelayAfter() {
 		return hasDelayAfter;
 	}
 
+	/**
+	 * Gets the closest conditional section in the list of trigger items
+	 * @param triggerItems the list of items to search for the closest conditional section in
+	 * @param type the type of conditional section to find. if null is provided, any type is allowed.
+	 * @return the closest conditional section
+	 */
 	@Nullable
-	private static SecConditional getClosestIf(List<TriggerItem> triggerItems) {
+	private static SecConditional getPrecedingConditional(List<TriggerItem> triggerItems, @Nullable ConditionalType type) {
 		// loop through the triggerItems in reverse order so that we find the most recent items first
 		for (int i = triggerItems.size() - 1; i >= 0; i--) {
 			TriggerItem triggerItem = triggerItems.get(i);
 			if (triggerItem instanceof SecConditional) {
-				SecConditional secConditional = (SecConditional) triggerItem;
+				SecConditional conditionalSection = (SecConditional) triggerItem;
 
-				if (secConditional.type == ConditionalType.IF)
-					// if the condition is an if, we found our most recent preceding "if"
-					return secConditional;
-				else if (secConditional.type == ConditionalType.ELSE)
+				if (conditionalSection.type == ConditionalType.ELSE) {
 					// if the conditional is an else, return null because it belongs to a different condition and ends
 					// this one
 					return null;
+				} else if (type == null || conditionalSection.type == type) {
+					// if the conditional matches the type argument, we found our most recent preceding conditional section
+					return conditionalSection;
+				}
 			} else {
 				return null;
 			}

src/test/skript/tests/syntaxes/sections/SecConditional.sk

@@ -143,3 +143,38 @@ test "SecConditional - else if all false":
 	then:
 		set {_a} to false
 	assert {_a} is not set with "'else if all' ran even though all conditions were false"
+
+test "SecConditional - starting with a non-multiline conditional":
+	if 1 is 1:
+		set {_a} to true
+	else if:
+		1 is 8
+		2 is 7
+	then:
+		set {_b} to true
+	assert {_a} is set with "non-multiline 'if' didn't run when before a multiline 'else if'"
+
+test "SecConditional - non-multiline conditional in the middle":
+	if:
+		1 is 2
+		3 is 4
+	then:
+		set {_b} to true
+	else if 1 is 1:
+		set {_a} to true
+	else if:
+		5 is 6
+		7 is 8
+	then:
+		set {_c} to true
+	assert {_a} is set with "non-multiline 'if' didn't run when used in the middle of a multiline 'else if'"
+
+test "SecConditional - non-multiline conditional at the end":
+	if:
+		1 is 2
+		3 is 4
+	then:
+		set {_b} to true
+	else if 1 is 1:
+		set {_a} to true
+	assert {_a} is set with "non-multiline 'if' didn't run used at the end of a multiline 'if'"