From 560e4ae4919b9a7eb10b9625cb66b99ac50cee12 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Thu, 3 Dec 2015 08:11:22 +1100 Subject: [PATCH] Add a linter rule for invalid octals Summary: PHP doesn't handle octals very well. Basically, it seems that any numeric scalar matching `/^0\d+$/` will be treated as an octal, whereas this should be `/^0[0-7]+$/`. As a result, `08` and `09` are both treated as `0` (because they are invalid octals. This diff adds a linter rule to detect this abnormality. Test Plan: Added unit tests. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D14604 --- src/__phutil_library_map__.php | 4 ++ ...alidOctalNumericScalarXHPASTLinterRule.php | 44 +++++++++++++++++++ ...lNumericScalarXHPASTLinterRuleTestCase.php | 11 +++++ .../binary.lint-test | 3 ++ .../decimal.lint-test | 5 +++ .../hexadecimal.lint-test | 4 ++ .../octal.lint-test | 7 +++ 7 files changed, 78 insertions(+) create mode 100644 src/lint/linter/xhpast/rules/ArcanistInvalidOctalNumericScalarXHPASTLinterRule.php create mode 100644 src/lint/linter/xhpast/rules/__tests__/ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase.php create mode 100644 src/lint/linter/xhpast/rules/__tests__/invalid-octal-numeric-scalar/binary.lint-test create mode 100644 src/lint/linter/xhpast/rules/__tests__/invalid-octal-numeric-scalar/decimal.lint-test create mode 100644 src/lint/linter/xhpast/rules/__tests__/invalid-octal-numeric-scalar/hexadecimal.lint-test create mode 100644 src/lint/linter/xhpast/rules/__tests__/invalid-octal-numeric-scalar/octal.lint-test diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3c859155..f2f2d43f 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -190,6 +190,8 @@ phutil_register_library_map(array( 'ArcanistInvalidDefaultParameterXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInvalidDefaultParameterXHPASTLinterRuleTestCase.php', 'ArcanistInvalidModifiersXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInvalidModifiersXHPASTLinterRule.php', 'ArcanistInvalidModifiersXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInvalidModifiersXHPASTLinterRuleTestCase.php', + 'ArcanistInvalidOctalNumericScalarXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistInvalidOctalNumericScalarXHPASTLinterRule.php', + 'ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase.php', 'ArcanistJSHintLinter' => 'lint/linter/ArcanistJSHintLinter.php', 'ArcanistJSHintLinterTestCase' => 'lint/linter/__tests__/ArcanistJSHintLinterTestCase.php', 'ArcanistJSONLintLinter' => 'lint/linter/ArcanistJSONLintLinter.php', @@ -590,6 +592,8 @@ phutil_register_library_map(array( 'ArcanistInvalidDefaultParameterXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistInvalidModifiersXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistInvalidModifiersXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', + 'ArcanistInvalidOctalNumericScalarXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistJSHintLinter' => 'ArcanistExternalLinter', 'ArcanistJSHintLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistJSONLintLinter' => 'ArcanistExternalLinter', diff --git a/src/lint/linter/xhpast/rules/ArcanistInvalidOctalNumericScalarXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistInvalidOctalNumericScalarXHPASTLinterRule.php new file mode 100644 index 00000000..3bff9e05 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistInvalidOctalNumericScalarXHPASTLinterRule.php @@ -0,0 +1,44 @@ +getOctalNumericScalars($root); + + foreach ($octals as $octal) { + if (!preg_match('/^0[0-7]*$/', $octal->getConcreteString())) { + $this->raiseLintAtNode( + $octal, + pht( + 'Invalid octal numeric scalar. `%s` is not a '. + 'valid octal and will be interpreted as `%d`.', + $octal->getConcreteString(), + intval($octal->getConcreteString(), 8))); + } + } + } + + private function getOctalNumericScalars(XHPASTNode $root) { + $numeric_scalars = $root->selectDescendantsOfType('n_NUMERIC_SCALAR'); + $octal_numeric_scalars = array(); + + foreach ($numeric_scalars as $numeric_scalar) { + $number = $numeric_scalar->getConcreteString(); + + if (preg_match('/^0\d+$/', $number)) { + $octal_numeric_scalars[] = $numeric_scalar; + } + } + + return $octal_numeric_scalars; + + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..6224115e --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistInvalidOctalNumericScalarXHPASTLinterRuleTestCase.php @@ -0,0 +1,11 @@ +executeTestsInDirectory( + dirname(__FILE__).'/invalid-octal-numeric-scalar/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/invalid-octal-numeric-scalar/binary.lint-test b/src/lint/linter/xhpast/rules/__tests__/invalid-octal-numeric-scalar/binary.lint-test new file mode 100644 index 00000000..5876c99e --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/invalid-octal-numeric-scalar/binary.lint-test @@ -0,0 +1,3 @@ +