Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new sniff to ban the use of compact() function #3795

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,159 @@
<?php

/**
* Bans the use of compact() function
*
* @author Paweł Bogut <pbogut@pbogut.me>
* @copyright 2006-2023 Squiz Pty Ltd (ABN 77 084 670 600)
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
*/

namespace PHP_CodeSniffer\Standards\Generic\Sniffs\Arrays;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think Arrays is the correct category as that only contains sniffs which are looking for array syntax.

I'd suggest using the PHP category.


use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Util\Tokens;

class DisallowCompactArrayBuilderSniff implements Sniff
{
protected const VARIABLE_NAME_PATTERN = '/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/';


/**
* Registers the tokens that this sniff wants to listen for.
*
* @return int[]
*/
public function register()
{
return [T_STRING];

}//end register()


/**
* Processes this test, when one of its tokens is encountered.
*
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the current token
* in the stack passed in $tokens.
*
* @return void
*/
public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

$content = $tokens[$stackPtr]['content'];

if (strtolower($content) !== 'compact') {
return;
}

// Make sure this is a function call.
$next = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), null, true);
if ($next === false || $tokens[$next]['code'] !== T_OPEN_PARENTHESIS) {
// Not a function call.
return;
}

$prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true);
$prevPrev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev - 1), null, true);

$ignorePrev = [
T_BITWISE_AND,
T_NS_SEPARATOR,
];

$excludedPrev = [
T_NULLSAFE_OBJECT_OPERATOR,
T_OBJECT_OPERATOR,
T_DOUBLE_COLON,
T_NEW,
T_NAMESPACE,
T_STRING,
T_FUNCTION,
];

$significantPrev = $prev;
if (in_array($tokens[$prev]['code'], $ignorePrev) === true) {
$significantPrev = $prevPrev;
}

// Make sure it is built-in function call.
if (in_array($tokens[$significantPrev]['code'], $excludedPrev) === true) {
// Not a built-in function call.
return;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about namespaced functions ?

MyNamespace\compact('a');
namespace\compact('a');

Also what about these ?

class Foo {
    public function compact( $param = 'a' ) {}
    public function &compact( $param = 'a' ) {}
}

$obj = new Compact('a');

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added them to the test case, they should not produce sniff errors.
I've also added \compact() as this is valid function call and should produce error.

$error = 'Array must not be created with compact() function';

// Make sure it is not prepended by bitwise operator.
if ($tokens[$prev]['code'] === T_BITWISE_AND) {
// Can not be fixed as &[] is not valid syntax.
$phpcsFile->addError($error, $stackPtr, 'Found');
return;
}

$fixable = false;
$toExpand = [];
$openPtr = $next;
$closePtr = null;
// Find all params in compact() function call, and check if it is fixable.
while (($next = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true)) !== false) {
if ($tokens[$next]['code'] === T_CONSTANT_ENCAPSED_STRING) {
$variableName = substr($tokens[$next]['content'], 1, -1);
$isValid = preg_match(self::VARIABLE_NAME_PATTERN, $variableName);

if ($isValid === false || $isValid === 0) {
break;
}

$toExpand[] = $next;
continue;
}

if ($tokens[$next]['code'] === T_CLOSE_PARENTHESIS) {
$fixable = true;
$closePtr = $next;
break;
}

if ($tokens[$next]['code'] !== T_COMMA) {
break;
}
}//end while

if ($fixable === false) {
$phpcsFile->addError($error, $stackPtr, 'Found');
return;
}

$fix = $phpcsFile->addFixableError($error, $stackPtr, 'Found');

if ($fix === true) {
$phpcsFile->fixer->beginChangeset();

if ($tokens[$prev]['code'] === T_NS_SEPARATOR) {
$phpcsFile->fixer->replaceToken($prev, '');
}

$phpcsFile->fixer->replaceToken($stackPtr, '');
$phpcsFile->fixer->replaceToken($openPtr, '[');
$phpcsFile->fixer->replaceToken($closePtr, ']');

foreach ($toExpand as $ptr) {
$variableName = substr($tokens[$ptr]['content'], 1, -1);
$phpcsFile->fixer->replaceToken(
$ptr,
$tokens[$ptr]['content'].' => $'.$variableName
);
}

$phpcsFile->fixer->endChangeset();
}//end if

}//end process()


}//end class
@@ -0,0 +1,47 @@
<?php
$var = compact();
$var = compact('a','b','c');
$foo = compact($var[1],$var[2]);
$foo = compact(
'a',
"b",
'c'
);
$var = compact/*comment*/('a', 'b', "c");
$var = compact(['aa', 'bb' => 'cc']);
$var = compact(array('aa', 'bb' => 'cc'));

function foo($compact) {}
$compact = function ($a, $b, $c) use ($foo): array {};
$compact('a', 'b', 'c');

view('some.view', compact("a", 'b', 'c'));
view('some.view', compact(
'a',
'b',
'c'
));

$var = compact('aa', 'invalid-var.name');
COMPACT('a');
Compact('a');
$var = Bazz::compact('a', 'b');
$ver = $foo->compact('a', 'b');
$obj?->compact('a');
class compact {
public function compact( $param = 'a' ) {}
public function &compact( $param = 'a' ) {}
}
new compact('a');
MyNamespace\compact('a');
namespace\compact('a');
\compact('a');
compact(...$names);
compact( 'prefix' . $name, '$name' . 'suffix', "some$name");
compact(...get_names('category1', 'category2'));
$bar = @compact('a', 'b');
$foo = true && compact('a', 'b');
$baz = &compact('a', 'b');
func(compact('a', 'b'));
// Live coding/parse error.
compact( 'a', 'b'
@@ -0,0 +1,47 @@
<?php
$var = [];
$var = ['a' => $a,'b' => $b,'c' => $c];
$foo = compact($var[1],$var[2]);
$foo = [
'a' => $a,
"b" => $b,
'c' => $c
];
$var = /*comment*/['a' => $a, 'b' => $b, "c" => $c];
$var = compact(['aa', 'bb' => 'cc']);
$var = compact(array('aa', 'bb' => 'cc'));

function foo($compact) {}
$compact = function ($a, $b, $c) use ($foo): array {};
$compact('a', 'b', 'c');

view('some.view', ["a" => $a, 'b' => $b, 'c' => $c]);
view('some.view', [
'a' => $a,
'b' => $b,
'c' => $c
]);

$var = compact('aa', 'invalid-var.name');
['a' => $a];
['a' => $a];
$var = Bazz::compact('a', 'b');
$ver = $foo->compact('a', 'b');
$obj?->compact('a');
class compact {
public function compact( $param = 'a' ) {}
public function &compact( $param = 'a' ) {}
}
new compact('a');
MyNamespace\compact('a');
namespace\compact('a');
['a' => $a];
compact(...$names);
compact( 'prefix' . $name, '$name' . 'suffix', "some$name");
compact(...get_names('category1', 'category2'));
$bar = @['a' => $a, 'b' => $b];
$foo = true && ['a' => $a, 'b' => $b];
$baz = &compact('a', 'b');
func(['a' => $a, 'b' => $b]);
// Live coding/parse error.
compact( 'a', 'b'
@@ -0,0 +1,73 @@
<?php

/**
* Unit test class for the DisallowCompactArrayBuilder sniff.
*
* @author Paweł Bogut <pbogut@pbogut.me>
* @copyright 2006-2023 Squiz Pty Ltd (ABN 77 084 670 600)
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
*/

namespace PHP_CodeSniffer\Standards\Generic\Tests\Arrays;

use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;

class DisallowCompactArrayBuilderUnitTest extends AbstractSniffUnitTest
{


/**
* Returns the lines where errors should occur.
*
* The key of the array should represent the line number and the value
* should represent the number of errors that should occur on that line.
*
* @param string $testFile The name of the file being tested.
*
* @return array<int, int>
*/
public function getErrorList($testFile='')
{
return [
2 => 1,
3 => 1,
4 => 1,
5 => 1,
10 => 1,
11 => 1,
12 => 1,
18 => 1,
19 => 1,
25 => 1,
26 => 1,
27 => 1,
38 => 1,
39 => 1,
40 => 1,
41 => 1,
42 => 1,
43 => 1,
44 => 1,
45 => 1,
47 => 1,
];

}//end getErrorList()


/**
* Returns the lines where warnings should occur.
*
* The key of the array should represent the line number and the value
* should represent the number of warnings that should occur on that line.
*
* @return array<int, int>
*/
public function getWarningList()
{
return [];

}//end getWarningList()


}//end class