diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 57c29e28..5997a022 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -104,6 +104,8 @@ phutil_register_library_map(array( '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', @@ -553,6 +555,8 @@ phutil_register_library_map(array( 'ArcanistConsoleLintRendererTestCase' => 'PhutilTestCase', 'ArcanistConstructorParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistConstructorParenthesesXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistContinueInsideSwitchXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistControlStatementSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistControlStatementSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistCoverWorkflow' => 'ArcanistWorkflow', diff --git a/src/configuration/ArcanistConfiguration.php b/src/configuration/ArcanistConfiguration.php index 80ebf6fa..aa7ea406 100644 --- a/src/configuration/ArcanistConfiguration.php +++ b/src/configuration/ArcanistConfiguration.php @@ -7,15 +7,15 @@ * pointing to your subclass in your project configuration. * * When specified as the **arcanist_configuration** class in your project's - * ##.arcconfig##, your subclass will be instantiated (instead of this class) + * `.arcconfig`, your subclass will be instantiated (instead of this class) * and be able to handle all the method calls. In particular, you can: * - * - create, replace, or disable workflows by overriding buildWorkflow() - * and buildAllWorkflows(); + * - create, replace, or disable workflows by overriding `buildWorkflow()` + * and `buildAllWorkflows()`; * - add additional steps before or after workflows run by overriding - * willRunWorkflow() or didRunWorkflow() or didAbortWorkflow(); and + * `willRunWorkflow()` or `didRunWorkflow()` or `didAbortWorkflow()`; and * - add new flags to existing workflows by overriding - * getCustomArgumentsForCommand(). + * `getCustomArgumentsForCommand()`. * * @concrete-extensible */ diff --git a/src/lint/linter/ArcanistFutureLinter.php b/src/lint/linter/ArcanistFutureLinter.php index d14fe49a..e9df8371 100644 --- a/src/lint/linter/ArcanistFutureLinter.php +++ b/src/lint/linter/ArcanistFutureLinter.php @@ -11,7 +11,7 @@ abstract class ArcanistFutureLinter extends ArcanistLinter { return 8; } - final public function willLintPaths(array $paths) { + public function willLintPaths(array $paths) { $limit = $this->getFuturesLimit(); $this->futures = id(new FutureIterator(array()))->limit($limit); foreach ($this->buildFutures($paths) as $path => $future) { @@ -23,7 +23,7 @@ abstract class ArcanistFutureLinter extends ArcanistLinter { return; } - final public function didLintPaths(array $paths) { + public function didLintPaths(array $paths) { if (!$this->futures) { return; } diff --git a/src/lint/linter/xhpast/rules/ArcanistContinueInsideSwitchXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistContinueInsideSwitchXHPASTLinterRule.php new file mode 100644 index 00000000..be071fac --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistContinueInsideSwitchXHPASTLinterRule.php @@ -0,0 +1,53 @@ +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/ArcanistPHPCompatibilityXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php index d73d1fae..b2fa6770 100644 --- a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php @@ -70,7 +70,7 @@ final class ArcanistPHPCompatibilityXHPASTLinterRule $symbol = $params->getChildByIndex(0); if (!$symbol->isStaticScalar()) { - continue; + break; } $symbol_name = $symbol->evalStatic(); diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..031e18f8 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistContinueInsideSwitchXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +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 index 00000000..6a9f040f --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/continue-inside-switch/continue-inside-switch-1-valid.lint-test @@ -0,0 +1,21 @@ + $line) { - if (!strlen($line)) { - unset($lines[$key]); - continue; - } - if ($line[0] == '#') { + if (preg_match('/^#/', $line)) { unset($lines[$key]); continue; } + break; } + $lines = array_reverse($lines); - return implode("\n", $lines)."\n"; + $lines = implode('', $lines); + $lines = rtrim($lines)."\n"; + + return $lines; } } diff --git a/src/parser/__tests__/ArcanistCommentRemoverTestCase.php b/src/parser/__tests__/ArcanistCommentRemoverTestCase.php index d89a3d08..7de70934 100644 --- a/src/parser/__tests__/ArcanistCommentRemoverTestCase.php +++ b/src/parser/__tests__/ArcanistCommentRemoverTestCase.php @@ -24,6 +24,21 @@ Here is a list: The end. +EOTEXT; + + $this->assertEqual($expect, ArcanistCommentRemover::removeComments($test)); + + $test = <<assertEqual($expect, ArcanistCommentRemover::removeComments($test));