4.0 | Proposal: add parameter types to methods in PHPCS

[jrfnl]

Setting the scene

PHPCS 4.0 will have a minimum supported PHP version of PHP 7.2.

PHP 7.2 allows for:

• Adding parameter type declarations, with the exception of union/intersection/DNF/mixed types.
• Adding return type declarations, with the exception of union/intersection/DNF/mixed types.

PHP 7.2 does not support:

• Property type declarations (PHP 7.4+)
• Constant type declarations (PHP 8.3+)
• Union/intersection/DNF types (PHP 8.0, 8.1, 8.2 respectively).
• Mixed type (PHP 8.0)
• Null/true/false as in union types or as stand-alone types (PHP 8.0/8/2)

Parameter type declarations in PHP are contravariant, which means that overloaded methods in child classes can widen the type. Return type declarations in PHP are covariant, which means that overloaded methods in child classes can make the type more narrow.

Full support for contravariance and covariance was only added in PHP 7.4, but for the purposes of this proposal, the partial support as added in PHP 7.2 should be sufficient.

Proposal

I propose to update the signatures of the following public/protected methods in PHPCS 4.0, adding (additional) parameter type declarations as per the list below.

Detailed proposal

Notes about the detailed proposal:

• Signatures of private methods methods are excluded from the below list as they don't affect the public API, but these will be updated too.
• Signatures which would not change (no parameters, all parameters already typed, or no typed parameters possible with the current minimum PHP version) have been left out of the below list.
• Methods which will need to be updated by extension and do not deviate from the change to the "parent" method, are not listed repetitively.
• The parameter types have largely been based on the currently documented types, which may not be correct (in which case the type will be changed to the real type).
• The list is based on the current state of the codebase and is subject to change.

In PHP_CodeSniffer\Config:

• [ ] public function __get(string $name)
• [ ] public function __set(string $name, $value)
• [ ] public function __isset(string $name)
• [ ] public function __unset(string $name)
• [ ] public function setSettings(array $settings)
• [ ] public function __construct(array $cliArgs=[], bool $dieOnUnknownArg=true)
• [ ] public function setCommandLineValues(array $args)
• [ ] public function processShortArgument(string $arg, int $pos)
• [ ] public function processLongArgument(string $arg, int $pos)
• [ ] public function processUnknownArgument(string $arg, int $pos)
• [ ] public function processFilePath(string $path)
• [ ] public function printShortUsage(bool $return=false)
• [ ] public static function getConfigData(string $key)
• [ ] public static function getExecutablePath(string $name)
• [ ] public function setConfigData(string $key, ?string $value, bool $temp=false)
• [ ] public function printConfigData(array $data)

In PHP_CodeSniffer\Fixer:

• [ ] public function generateDiff(?string $filePath=null, bool $colors=true)
• [ ] public function getTokenContent(int $stackPtr)
• [ ] public function replaceToken(int $stackPtr, string $content)
• [ ] public function revertToken(int $stackPtr)
• [ ] public function substrToken(int $stackPtr, int $start, ?int $length=null)
• [ ] public function addNewline(int $stackPtr)
• [ ] public function addNewlineBefore(int $stackPtr)
• [ ] public function addContent(int $stackPtr, string $content)
• [ ] public function addContentBefore(int $stackPtr, string $content)
• [ ] public function changeCodeBlockIndent(int $start, int $end, int $change)

In PHP_CodeSniffer\Reporter:

• [ ] public function printReport(string $report)

In PHP_CodeSniffer\Ruleset:

• [ ] public function processRuleset(string $rulesetPath, int $depth=0)
• [ ] public function registerSniffs(array $files, array $restrictions, array $exclusions)
• [ ] public function setSniffProperty(string $sniffClass, string $name, array $settings)
• [ ] public function getIgnorePatterns(?string $listener=null)
• [ ] public function getIncludePatterns(?string $listener=null)

In PHP_CodeSniffer\Runner:

• [ ] public function handleErrors(int $code, string $message, string $file, int $line)
• [ ] public function processFile(\PHP_CodeSniffer\Files\File $file)
• [ ] public function printProgress(File $file, int $numFiles, int $numProcessed)

In PHP_CodeSniffer\Files\File:

• [ ] public function __construct(string $path, Ruleset $ruleset, Config $config)
• [ ] public function setContent(string $content)
• [ ] public function addError(string $error, int $stackPtr, string $code, array $data=[], int $severity=0, bool $fixable=false)
• [ ] public function addWarning(string $warning, int $stackPtr, string $code, array $data=[], int $severity=0, bool $fixable=false)
• [ ] public function addErrorOnLine(string $error, int $line, string $code, array $data=[], int $severity=0)
• [ ] public function addWarningOnLine(string $warning, int $line, string $code, array $data=[], int $severity=0)
• [ ] public function addFixableError(string $error, int $stackPtr, string $code, array $data=[], int $severity=0)
• [ ] public function addFixableWarning(string $warning, int $stackPtr, string $code, array $data=[], int $severity=0)
• [ ] protected function addMessage(bool $error, string $message, int $line, int $column, string $code, array $data, int $severity, bool $fixable)
• [ ] public function recordMetric(int $stackPtr, string $metric, string $value)
• [ ] public function getDeclarationName(int $stackPtr)
• [ ] public function getMethodParameters(int $stackPtr)
• [ ] public function getMethodProperties(int $stackPtr)
• [ ] public function getMemberProperties(int $stackPtr)
• [ ] public function getClassProperties(int $stackPtr)
• [ ] public function isReference(int $stackPtr)
• [ ] public function getTokensAsString(int $start, int $length, bool $origContent=false)
• [ ] public function findPrevious($types, int $start, ?int $end=null, bool $exclude=false, ?string $value=null, bool $local=false)
• [ ] public function findNext($types, int $start, ?int $end=null, bool $exclude=false, ?string $value=null, bool $local=false)
• [ ] public function findStartOfStatement(int $start, $ignore=null)
• [ ] public function findEndOfStatement(int $start, $ignore=null)
• [ ] public function findFirstOnLine($types, int $start, bool $exclude=false, ?string $value=null)
• [ ] public function hasCondition(int $stackPtr, $types)
• [ ] public function getCondition(int $stackPtr, $type, bool $first=true)
• [ ] public function findExtendedClassName(int $stackPtr)
• [ ] public function findImplementedInterfaceNames(int $stackPtr)

In PHP_CodeSniffer\Files\DummyFile:

• [ ] public function __construct(string $content, Ruleset $ruleset, Config $config)
• [ ] public function setErrorCounts(int $errorCount, int $warningCount, int $fixableCount, int $fixedCount)

In PHP_CodeSniffer\Files\FileList:

• [ ] public function addFile(string $path, ?\PHP_CodeSniffer\Files\File $file=null)

In PHP_CodeSniffer\Filters\Filter:

• [ ] public function __construct(\RecursiveIterator $iterator, string $basedir, Config $config, Ruleset $ruleset)
• [ ] protected function shouldProcessFile(string $path)
• [ ] protected function shouldIgnorePath(string $path)

In PHP_CodeSniffer\Filters\Git*:

• [ ] protected function exec(string $cmd)

In PHP_CodeSniffer\Generators\Generator:

• [ ] protected function getTitle(\DOMElement $doc) (was: protected function getTitle(\DOMNode $doc))
• [ ] abstract protected function processSniff(\DOMElement $doc) (was: abstract protected function processSniff(\DOMNode $doc))

In PHP_CodeSniffer\Generators\*:

All methods taking a $node parameter will be updated to expect DOMElement instead of DOMNode.

In PHP_CodeSniffer\Reports\Report:

• [ ] public function generateFileReport(array $report, File $phpcsFile, bool $showSources=false, int $width=80)
• [ ] public function generate(string $cachedData, int $totalFiles, int $totalErrors, int $totalWarnings, int $totalFixable, bool $showSources=false, int $width=80, bool $interactive=false, bool $toScreen=true)

In PHP_CodeSniffer\Sniffs\Sniff:

• [ ] public function process(File $phpcsFile, int $stackPtr)

In PHP_CodeSniffer\Sniffs\AbstractArraySniff:

• [ ] abstract protected function processSingleLineArray(File $phpcsFile, int $stackPtr, int $arrayStart, int $arrayEnd, array $indices)
• [ ] abstract protected function processMultiLineArray(File $phpcsFile, int $stackPtr, int $arrayStart, int $arrayEnd, array $indices)

In PHP_CodeSniffer\Sniffs\AbstractPatternSniff:

• [ ] public function __construct(bool $ignoreComments=false) (was __construct($ignoreComments=null))
• [ ] protected function processPattern(array $patternInfo, File $phpcsFile, int $stackPtr)
• [ ] protected function prepareError(string $found, string $patternCode)
• [ ] protected function processSupplementary(File $phpcsFile, int $stackPtr)

In PHP_CodeSniffer\Sniffs\AbstractScopeSniff:

• [ ] public function __construct(array $scopeTokens, array $tokens, bool $listenOutside=false)
• [ ] abstract protected function processTokenWithinScope(File $phpcsFile, int $stackPtr, int $currScope)
• [ ] abstract protected function processTokenOutsideScope(File $phpcsFile, int $stackPtr)

In PHP_CodeSniffer\Sniffs\AbstractVariableSniff:

• [ ] abstract protected function processMemberVar(File $phpcsFile, int $stackPtr)
• [ ] abstract protected function processVariable(File $phpcsFile, int $stackPtr)
• [ ] abstract protected function processVariableInString(File $phpcsFile, int $stackPtr)

In PHP_CodeSniffer\Tokenizers\Tokenizer:

• [ ] public function __construct(string $content, ?\PHP_CodeSniffer\Config $config, string $eolChar='\n')
• [ ] protected function isMinifiedContent(string $content, string $eolChar='\n')
• [ ] abstract protected function tokenize(string $string)
• [ ] public function replaceTabsInToken(array &$token, string $prefix=' ', string $padding=' ', int $tabWidth=null)

In PHP_CodeSniffer\Tokenizers\Comment:

• [ ] public function tokenizeString(string $string, string $eolChar, int $stackPtr)

In PHP_CodeSniffer\Tokenizers\PHP:

• [ ] public static function resolveSimpleToken(string $token)

In PHP_CodeSniffer\Util\Cache:

• [ ] public static function get(?string $key=null)
• [ ] public static function set(string $key, $value) $value (mixed) cannot be typed until there is a PHP 8.0 minimum.

In PHP_CodeSniffer\Util\Common:

• [ ] public static function isPharFile(string $path)
• [ ] public static function isReadable(string $path)
• [ ] public static function realpath(string $path)
• [ ] public static function stripBasepath(string $path, string $basepath)
• [ ] public static function detectLineEndings(string $contents)
• [ ] public static function escapeshellcmd(string $cmd)
• [ ] public static function prepareForOutput(string $content, array $exclude=[])
• [ ] public static function isCamelCaps(string $string, bool $classFormat=false, bool $public=true, bool $strict=true)
• [ ] public static function isUnderscoreName(string $string)
• [ ] public static function suggestType(string $varType)
• [ ] public static function getSniffCode(string $sniffClass)
• [ ] public static function cleanSniffClass(string $sniffClass)

In PHP_CodeSniffer\Util\Standards:

• [ ] public static function getInstalledStandardDetails(bool $includeGeneric=false, string $standardsDir='')
• [ ] public static function getInstalledStandards(bool $includeGeneric=false, string $standardsDir='')
• [ ] public static function isInstalledStandard(string $standard)
• [ ] public static function getInstalledStandardPath(string $standard)

In PHP_CodeSniffer\Util\Timing:

• [ ] public static function getHumanReadableDuration(float $duration)
• [ ] public static function printRunTime(bool $force=false)

In PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest:

• [ ] protected function getTestFiles(string $testFileBase)
• [ ] public function setCliValues(string $filename, \PHP_CodeSniffer\Config $config)

Regarding strict types

This proposal explicitly does NOT include adding strict_types=1 declarations to the classes/interfaces in PHP_CodeSniffer.

PHPCS can not enforce for external standards to add strict_types, which means that strict_types offers a false sense of security as it is only effective when both the file in which the method call is made, as well as the file containing the method being called has a strict_types declaration.

In all other situations, it will still end up juggling the type of a passed parameter for scalar types and throw a TypeError for non-scalar types.

Additionally, for users using external standards which do already use strict_types, it is likely to lead to more problems with files not being scanned correctly/completely due to type issues when exceptions are not being caught. Also see #30.

I propose discussing enabling strict_types for PHPCS in a separate issue with a target PHPCS version of 5.0 or higher.

Future scope

After this proposal there will still be a few parameters untyped due to those needing the mixed type or a union/intersection/DNF type.

After this proposal has been actioned, a follow-up issue should be opened to keep track of what remains and to action adding types to those parameters depending on when PHPCS will raise the minimum supported PHP version to the version needed for those parameters.

BC considerations

As parameter types are contravariant, adding these type declarations in PHPCS 4.0 does not break backward compatibility.

An overloaded/implemented method in a child class which does not have a type declaration, effectively widens the type to mixed.

Suggested Migration Path for External Standards

During the lifetime of PHPCS 4.x, external standards are encouraged to:

• Add parameter type declarations - in line with the above proposal - to their code.
• Add return type declarations to their code, in line with the proposal in #391
• Add strict_types=1 declarations to their files.
• Use tooling like PHPStan and/or Psalm to find further type juggling issues.

None of this can/will be enforced, but external standards using the lifetime of PHPCS 4.x to action the above will be better prepared for supporting PHPCS 5.x when the time comes.

Opinions ?

/cc @asispts @dingo-d @fredden @GaryJones @greg0ire @kukulich @michalbundyra @Ocramius @sirbrillig @stronk7 @weierophinney @wimg

[sirbrillig]

As with the return types issue, I think that the more types, the safer the code and the more clear the API, so I vote in favor of this.

[stronk7]

I like it. Just, side comment… maybe this could be considered as an opportunity to advance marking what’s public API and what’s not, in case #392 is delayed too much? It could help identifying stuff then.

Ciao :)

[asispts]

I like the idea :+1:. PHP 7 will show a warning about the incompatibility, which can be used by CS owners/maintainers to fix their code.

maybe this could be considered as an opportunity to advance marking what’s public API and what’s not, in case #392 is delayed too much? It could help identifying stuff then.

Agreed.

[weierophinney]

@jrfnl One note: you can add parameter types in a minor release without being a BC break. Extensions/implementations can widen argument parameters without violating the LSP, and if they were previously leaving them untyped, the implied type is mixed.

(In FIG, we even codified this in the PSR Evolution bylaw.)

So you might be able to introduce these in v3, and then use v4 as the BC break introducing return types.

[jrfnl]

So you might be able to introduce these in v3, and then use v4 as the BC break introducing return types.

@weierophinney PHPCS 3.x still has a minimum of PHP 5.4, so that's not an option. The minimum PHP version will be raised to PHP 7.2 in PHPCS 4.0, which is why this proposal targets that version for parameter types and #391 targets 5.0 for return types.