From af119af393d59283cc14ef49faa10ef410db50ee Mon Sep 17 00:00:00 2001 From: tfrommen Date: Wed, 28 Oct 2020 15:03:53 +0100 Subject: [PATCH 1/3] Add sniff to check unnecessary ternaries --- HM/Sniffs/PHP/TernarySniff.php | 70 ++++++++++++++++++++++++++++ tests/FixtureTests.php | 1 + tests/fixtures/fail/ternary.php | 11 +++++ tests/fixtures/fail/ternary.php.json | 32 +++++++++++++ tests/fixtures/pass/ternary.php | 14 ++++++ 5 files changed, 128 insertions(+) create mode 100644 HM/Sniffs/PHP/TernarySniff.php create mode 100644 tests/fixtures/fail/ternary.php create mode 100644 tests/fixtures/fail/ternary.php.json create mode 100644 tests/fixtures/pass/ternary.php diff --git a/HM/Sniffs/PHP/TernarySniff.php b/HM/Sniffs/PHP/TernarySniff.php new file mode 100644 index 00000000..79a70745 --- /dev/null +++ b/HM/Sniffs/PHP/TernarySniff.php @@ -0,0 +1,70 @@ +getTokens(); + + $inline_then_token = $phpcsFile->findPrevious( T_INLINE_THEN, $stackPtr ); + + $value_if_true_tokens = $this->get_nonempty_tokens( $tokens, $inline_then_token + 1, $stackPtr - 1 ); + if ( count( $value_if_true_tokens ) !== 1 ) { + // No single value. + return; + } + + if ( ! $this->is_boolean_token( $value_if_true_tokens[0] ) ) { + // No Boolean value. + return; + } + + $ternary_end_token = $phpcsFile->findNext( array( T_CLOSE_CURLY_BRACKET, T_CLOSE_PARENTHESIS, T_SEMICOLON ), $stackPtr + 1 ); + + $value_if_false_tokens = $this->get_nonempty_tokens( $tokens, $stackPtr + 1, $ternary_end_token - 1 ); + if ( count( $value_if_false_tokens ) !== 1 ) { + // No single value. + return; + } + + if ( ! $this->is_boolean_token( $value_if_false_tokens[0] ) ) { + // No Boolean value. + return; + } + + $warning = 'Unnecessary ternary found: Instead of "$expr ? %s : %s", use "%s"'; + $data = [ + $value_if_true_tokens[0]['content'], + $value_if_false_tokens[0]['content'], + $value_if_true_tokens[0]['content'] === 'true' ? '(bool) $expr' : '! $expr' + ]; + $phpcsFile->addWarning( $warning, $stackPtr, 'UnnecessaryTernary', $data ); + } + + private function get_nonempty_tokens( array $tokens, $start, $end ) { + $tokens = array_slice( $tokens, $start, $end - $start + 1 ); + + return array_values( array_filter( + $tokens, + [ $this, 'is_nonempty_token' ] + ) ); + } + + private function is_boolean_token( array $token ) { + return in_array( $token['code'], [ T_FALSE, T_TRUE ], true ); + } + + private function is_nonempty_token( array $token ) { + return ! in_array( $token['code'], Tokens::$emptyTokens, true ); + } +} diff --git a/tests/FixtureTests.php b/tests/FixtureTests.php index bad88454..40e39151 100644 --- a/tests/FixtureTests.php +++ b/tests/FixtureTests.php @@ -106,6 +106,7 @@ public function setUp() { 'HM.Namespaces.NoLeadingSlashOnUse', 'HM.Performance.SlowMetaQuery', 'HM.Performance.SlowOrderBy', + 'HM.PHP.Ternary', 'HM.Security.EscapeOutput', 'HM.Security.NonceVerification', 'HM.Security.ValidatedSanitizedInput', diff --git a/tests/fixtures/fail/ternary.php b/tests/fixtures/fail/ternary.php new file mode 100644 index 00000000..a9dfec73 --- /dev/null +++ b/tests/fixtures/fail/ternary.php @@ -0,0 +1,11 @@ + Date: Wed, 28 Oct 2020 15:12:44 +0100 Subject: [PATCH 2/3] Add comma as ternary end for inline usage --- HM/Sniffs/PHP/TernarySniff.php | 2 +- tests/fixtures/fail/ternary.php | 2 ++ tests/fixtures/fail/ternary.php.json | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/HM/Sniffs/PHP/TernarySniff.php b/HM/Sniffs/PHP/TernarySniff.php index 79a70745..a1756f87 100644 --- a/HM/Sniffs/PHP/TernarySniff.php +++ b/HM/Sniffs/PHP/TernarySniff.php @@ -29,7 +29,7 @@ public function process( File $phpcsFile, $stackPtr ) { return; } - $ternary_end_token = $phpcsFile->findNext( array( T_CLOSE_CURLY_BRACKET, T_CLOSE_PARENTHESIS, T_SEMICOLON ), $stackPtr + 1 ); + $ternary_end_token = $phpcsFile->findNext( array( T_CLOSE_CURLY_BRACKET, T_CLOSE_PARENTHESIS, T_COMMA, T_SEMICOLON ), $stackPtr + 1 ); $value_if_false_tokens = $this->get_nonempty_tokens( $tokens, $stackPtr + 1, $ternary_end_token - 1 ); if ( count( $value_if_false_tokens ) !== 1 ) { diff --git a/tests/fixtures/fail/ternary.php b/tests/fixtures/fail/ternary.php index a9dfec73..0d753a1b 100644 --- a/tests/fixtures/fail/ternary.php +++ b/tests/fixtures/fail/ternary.php @@ -9,3 +9,5 @@ // Missing semicolon on purpose. return $expr ? true : false } + +print( $expr ? true : false, 'foo' ); diff --git a/tests/fixtures/fail/ternary.php.json b/tests/fixtures/fail/ternary.php.json index 2a103108..83c9b794 100644 --- a/tests/fixtures/fail/ternary.php.json +++ b/tests/fixtures/fail/ternary.php.json @@ -28,5 +28,11 @@ "source": "HM.PHP.Ternary.UnnecessaryTernary", "type": "warning" } + ], + "13": [ + { + "source": "HM.PHP.Ternary.UnnecessaryTernary", + "type": "warning" + } ] } \ No newline at end of file From 5734ed51407cf2f1ad18f14e8bfa075a2a781c3e Mon Sep 17 00:00:00 2001 From: tfrommen Date: Wed, 28 Oct 2020 15:20:22 +0100 Subject: [PATCH 3/3] Use long array syntax for consistency --- HM/Sniffs/PHP/TernarySniff.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/HM/Sniffs/PHP/TernarySniff.php b/HM/Sniffs/PHP/TernarySniff.php index a1756f87..ad881f09 100644 --- a/HM/Sniffs/PHP/TernarySniff.php +++ b/HM/Sniffs/PHP/TernarySniff.php @@ -43,11 +43,11 @@ public function process( File $phpcsFile, $stackPtr ) { } $warning = 'Unnecessary ternary found: Instead of "$expr ? %s : %s", use "%s"'; - $data = [ + $data = array( $value_if_true_tokens[0]['content'], $value_if_false_tokens[0]['content'], $value_if_true_tokens[0]['content'] === 'true' ? '(bool) $expr' : '! $expr' - ]; + ); $phpcsFile->addWarning( $warning, $stackPtr, 'UnnecessaryTernary', $data ); } @@ -56,12 +56,12 @@ private function get_nonempty_tokens( array $tokens, $start, $end ) { return array_values( array_filter( $tokens, - [ $this, 'is_nonempty_token' ] + array( $this, 'is_nonempty_token' ) ) ); } private function is_boolean_token( array $token ) { - return in_array( $token['code'], [ T_FALSE, T_TRUE ], true ); + return in_array( $token['code'], array( T_FALSE, T_TRUE ), true ); } private function is_nonempty_token( array $token ) {