Improve switch continue warning

Don't suggest "continue N+1" if there is no wrapping loop. The resulting code would be illegal.
php · Jan 25, 2021 · 1850785 · 1850785
1 parent d319098
commit 1850785
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 10 deletions.
diff --git a/Zend/tests/continue_targeting_switch_warning.phpt b/Zend/tests/continue_targeting_switch_warning.phpt
@@ -22,6 +22,21 @@ function test() {
         }
     }
 
+    switch ($bar) {
+        case 0:
+            while ($xyz) {
+                continue 2; // INVALID
+            }
+        case 1:
+            while ($xyz) {
+                continue;
+            }
+        case 2:
+            while ($xyz) {
+                break 2;
+            }
+    }
+
     while ($foo) {
         switch ($bar) {
             case 0:
@@ -42,8 +57,10 @@ function test() {
 
 ?>
 --EXPECTF--
-Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in %s on line 6
+Warning: "continue" targeting switch is equivalent to "break" in %s on line 6
 
 Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"? in %s on line 14
 
-Warning: "continue 2" targeting switch is equivalent to "break 2". Did you mean to use "continue 3"? in %s on line 26
+Warning: "continue 2" targeting switch is equivalent to "break 2" in %s on line 25
+
+Warning: "continue 2" targeting switch is equivalent to "break 2". Did you mean to use "continue 3"? in %s on line 41
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -4891,15 +4891,26 @@ void zend_compile_break_continue(zend_ast *ast) /* {{{ */
 
 		if (CG(context).brk_cont_array[cur].is_switch) {
 			if (depth == 1) {
-				zend_error(E_WARNING,
-					"\"continue\" targeting switch is equivalent to \"break\". " \
-					"Did you mean to use \"continue " ZEND_LONG_FMT "\"?",
-					depth + 1);
+				if (CG(context).brk_cont_array[cur].parent == -1) {
+					zend_error(E_WARNING,
+						"\"continue\" targeting switch is equivalent to \"break\"");
+				} else {
+					zend_error(E_WARNING,
+						"\"continue\" targeting switch is equivalent to \"break\". " \
+						"Did you mean to use \"continue " ZEND_LONG_FMT "\"?",
+						depth + 1);
+				}
 			} else {
-				zend_error(E_WARNING,
-					"\"continue " ZEND_LONG_FMT "\" targeting switch is equivalent to \"break " ZEND_LONG_FMT "\". " \
-					"Did you mean to use \"continue " ZEND_LONG_FMT "\"?",
-					depth, depth, depth + 1);
+				if (CG(context).brk_cont_array[cur].parent == -1) {
+					zend_error(E_WARNING,
+						"\"continue " ZEND_LONG_FMT "\" targeting switch is equivalent to \"break " ZEND_LONG_FMT "\"",
+						depth, depth);
+				} else {
+					zend_error(E_WARNING,
+						"\"continue " ZEND_LONG_FMT "\" targeting switch is equivalent to \"break " ZEND_LONG_FMT "\". " \
+						"Did you mean to use \"continue " ZEND_LONG_FMT "\"?",
+						depth, depth, depth + 1);
+				}
 			}
 		}
 	}