Page MenuHomePhabricator
Files F14750910

D19931.id.diff
No OneTemporary
Actions

D19931.id.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -91,6 +91,8 @@
'ArcanistConsoleLintRendererTestCase' => 'lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php',
'ArcanistConstructorParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistConstructorParenthesesXHPASTLinterRule.php',
'ArcanistConstructorParenthesesXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistConstructorParenthesesXHPASTLinterRuleTestCase.php',
+ 'ArcanistContinueInsideSwitchXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistContinueInsideSwitchXHPASTLinterRule.php',
+ 'ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase.php',
'ArcanistControlStatementSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistControlStatementSpacingXHPASTLinterRule.php',
'ArcanistControlStatementSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistControlStatementSpacingXHPASTLinterRuleTestCase.php',
'ArcanistCoverWorkflow' => 'workflow/ArcanistCoverWorkflow.php',
@@ -511,6 +513,8 @@
'ArcanistConsoleLintRendererTestCase' => 'PhutilTestCase',
'ArcanistConstructorParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistConstructorParenthesesXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
+ 'ArcanistContinueInsideSwitchXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
+ 'ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistControlStatementSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistControlStatementSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistCoverWorkflow' => 'ArcanistWorkflow',
diff --git a/src/lint/linter/xhpast/rules/ArcanistContinueInsideSwitchXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistContinueInsideSwitchXHPASTLinterRule.php
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/ArcanistContinueInsideSwitchXHPASTLinterRule.php
@@ -0,0 +1,53 @@
+<?php
+
+final class ArcanistContinueInsideSwitchXHPASTLinterRule
+ extends ArcanistXHPASTLinterRule {
+
+ const ID = 128;
+
+ public function getLintName() {
+ return pht('Continue Inside Switch');
+ }
+
+ public function process(XHPASTNode $root) {
+ $continues = $root->selectDescendantsOfType('n_CONTINUE');
+
+ $valid_containers = array(
+ 'n_WHILE' => true,
+ 'n_FOREACH' => true,
+ 'n_FOR' => true,
+ 'n_DO_WHILE' => true,
+ );
+
+ foreach ($continues as $continue) {
+ // If this is a "continue 2;" or similar, assume it's legitimate.
+ $label = $continue->getChildByIndex(0);
+ if ($label->getTypeName() !== 'n_EMPTY') {
+ continue;
+ }
+
+ $node = $continue->getParentNode();
+ while ($node) {
+ $node_type = $node->getTypeName();
+
+ // If we hit a valid loop which you can actually "continue;" inside,
+ // this is legitimate and we're done here.
+ if (isset($valid_containers[$node_type])) {
+ break;
+ }
+
+ if ($node_type === 'n_SWITCH') {
+ $this->raiseLintAtNode(
+ $continue,
+ pht(
+ 'In a "switch" statement, "continue;" is equivalent to "break;" '.
+ 'but causes compile errors beginning with PHP 7.0.0.'),
+ 'break');
+ }
+
+ $node = $node->getParentNode();
+ }
+ }
+ }
+
+}
diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase.php
@@ -0,0 +1,11 @@
+<?php
+
+final class ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase
+ extends ArcanistXHPASTLinterRuleTestCase {
+
+ public function testLinter() {
+ $this->executeTestsInDirectory(
+ dirname(__FILE__).'/continue-inside-switch/');
+ }
+
+}
diff --git a/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-1-valid.lint-test b/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-1-valid.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-1-valid.lint-test
@@ -0,0 +1,21 @@
+<?php
+
+// These are valid loops which you can "continue;" inside.
+
+for ($x = 0; $x < 3; $x++) {
+ continue;
+}
+
+foreach ($x as $y) {
+ continue;
+}
+
+while ($x) {
+ continue;
+}
+
+do {
+ continue;
+} while ($x);
+~~~~~~~~~~
+~~~~~~~~~~
diff --git a/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-2-n.lint-test b/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-2-n.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-2-n.lint-test
@@ -0,0 +1,24 @@
+<?php
+
+// It's okay to "continue N;" into a loop, although this is usually not the
+// cleanest way to write code and might be discouraged in the future.
+
+while ($x) {
+ switch ($y) {
+ case 1:
+ continue 2;
+ }
+}
+
+// This is invalid, but we don't detect it for now.
+
+switch ($x) {
+ case 1:
+ switch ($y) {
+ case 2:
+ continue 2;
+ }
+ break;
+}
+~~~~~~~~~~
+~~~~~~~~~~
diff --git a/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-3-rewrite.lint-test b/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-3-rewrite.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-3-rewrite.lint-test
@@ -0,0 +1,26 @@
+<?php
+
+switch ($x) {
+ case 1:
+ continue;
+}
+
+switch ($x) {
+ case 1:
+ continue /* CRITICAL: Nuclear launch code is 1234. */ ;
+}
+~~~~~~~~~~
+error:5:5
+error:10:5
+~~~~~~~~~~
+<?php
+
+switch ($x) {
+ case 1:
+ break;
+}
+
+switch ($x) {
+ case 1:
+ break /* CRITICAL: Nuclear launch code is 1234. */ ;
+}

File Metadata

Mime Type
text/plain
Expires
Wed, Jan 22, 12:02 PM (5 h, 5 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7032682
Default Alt Text
D19931.id.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility