aik099/codingstandard

The PHP_CodeSniffer coding standard I'm using on all of my projects

codingstandard's Issues

Include PHPCS 2.x in tests

Update Travis CI build matrix to use both PHPCS 1.x (the ~1.5 version in composer.json) and PHPCS 2.x (the dev-phpcs-fixer version in composer.json) in tests.

This way we can see if standard really works with PHPCS 2.x

Blank line inside IF detection with inline comments

For code below I'm having strange error: Expected 0 blank lines at start of "if" control structure; -1 found:

if ( count($list->Records) > $list->GetSelectedCount() ) { // Has more records for next page.
    $next_block_params = $this->prepareTagParams($params);
}

Technically comment after { isn't on new line, so error message is wrong. I guess we can make an exception and allow such comments that follow { or } on same line.

Empty lines at multi-line array declaration edges

We should disallow empty lines right after start and right before end of multi-line array.

$config = array(

    'Prefix' => 'custom-helpers',

    'EventHandlerClass' => array('class' => 'kEventHandler', 'file' => '', 'build_event' => 'OnBuild'),

    'RegisterClasses' => array(
        array('pseudo' => 'SampleHelper', 'class' => 'SampleHelper', 'file' => 'sample_helper.php', 'build_event' => ''),
    ),

);

Validate comma spacing in arrays

We need to validate that for cases when:

1. there are several elements of array placed in a single line (not exactly inline array)
2. not for last element in a row

we need to ensure that a space is present after comma.

Valid:

$something = array(1 => 'aa', 'bb' => 'cc');

Invalid:

$something = array(1 => 'aa','bb' => 'cc');

The ArrayDeclaration sniff from Squiz coding standard has some good code for checking what we need, but it only applies to inline arrays.

Check for empty content inside function

Same as with content inside IF we need to check that no empty line exists right after function opening brace (starting from next line of course) and before function closing brace (starting from previous line).

Drop support for PHP_CodeSniffer 1.5.x

The PHP_CodeSniffer 2.0.0 version was released in December 2014 and that's why supporting both PHP_CodeSniffer 1.5.x and PHP_CodeSniffer 2.0 is no longer cost effective.

Shorthand ternary operator validation

The shorthand ternary operator was introduced in PHP 5.3 and is regular ternary operator with TRUE part missing: $value ?: $default. Need to validate/fix, that no whitespace are present between ? and : if whitespace is the only content between them.

Decide on require/include faith

Decide on whatever the:

• require
• require_once
• include
• include_once

are functions or not and enforce corresponding syntax. Maybe Squiz standard already have a sniff for that.

[In-Portal] File Header Validator

Validate that:

1. file comment is present
2. file comment has proper license text
3. the defined('FULL_PATH') or die('restricted access!'); is present and has 1 line empty before/after it (only for files with class/interface/trait in them)

Disallow type names in variable names

Disallow type names in variable names. For example $user_object or $categories_string.

List of not allowed types (see more at http://phpdoc.org/docs/latest/guides/types.html):

• string
• int, integer
• float
• bool, boolean
• array
• resource
• null
• callable
• object
• array

Related to: #78

Remove empty DocBlocks

Maybe we should remove empty DocBlocks when we find them.

Support for @inheritdoc

Add support for @inheritdoc to FunctionCommentSniff. Look at example from https://github.com/opensky/Symfony2-coding-standard/blob/master/Sniffs/Commenting/FunctionCommentSniff.php

Don't forget about unit tests.

Magnetic inline comments

Currently an inline comment must be placed right above code line to be valid. This creates a problems, where you need to mark a piece of code with comments, e.g.

echo '';

// CUSTOM: begin
$this_code = 5;
// CUSTOM: end (warning here)

echo 'something';

I propose this:

• if inline comment goes after another code in same line, then it's not an inline comment
• inline comment can be connected (no empty line between them) to code line above it
• inline comment can be connected (no empty line between them) to code line below it

Deal with @access tags

DocBlock:

/**
 * Allows to override standard permission mapping
 *
 * @return void
 * @access protected
 * @see    kEventHandler::$permMapping
 */
protected function mapPermissions()

Following scenarios (for reporting) are possible:

1. no @access tag present - do nothing
2. @access tag present, but doesn't match method/property visibility (consider missing visibility as public) - report mismatch

Fixing scenarios:

1. if method/property visibility not explicitly set, then set method/property visibility based on @access tag and remove @access tag
2. if method/property visibility set and matches the @access tag, then remove @access tag

Add sub-optimal variable sniffs

Add Sniff for:

• detecting unused variables
• detecting undefined variables (even 2 or more mentions of the variable)
• detect if variable value is assigned several times (e.g. reused temp variable)

Resources

• https://www.google.com/search?q=php_codesniffer+variable+case&oq=php_codesniffer+variable+case&aqs=chrome..69i57.7045j0j7&sourceid=chrome&espv=210&es_sm=119&ie=UTF-8#q=php_codesniffer+variable+name&safe=off
• https://github.com/AlanOne/PHP_Codesniffer-Plugins

Check for assignment in codition

Check if = is used instead of == in the conditional constructs (if, elseif). Don't touch while and other cycles before ($row = mysql_fetch_row) seems to be pretty common approach there.

Validate correct variable names

Following must be checked:

1. the class/interface/trait property name must be in camel case (e.g. thePropertyName or _thePrivatePropertyName)
2. the function/method parameter names and variables declared within the function/method must be named in lowercase with _ used for word separation (e.g. $local_variable_name and not $_local_variable_name or even not $localVariableName)

https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Zend/Sniffs/NamingConventions/ValidVariableNameSniff.php

Compare "InPortal" PhpStorm standard to what PHP_CodeSniffer checks

Compare "InPortal" PhpStorm standard to what PHP_CodeSniffer checks to determine if PhpStorm fixes something that PHP_CodeSniffer doesn't check for.

For example #45 task was created because of such inconsistency.

Validate comma spacing

There must be:

• no whitespace before comma
• 1 whitespace (with length of 1) after comma

Tricky cases (can cheat by checking for following closing parenthesis):

• array declaration end
• function call end

Validate tag order in PHPDoc function comments

Validate, that when present these tags must be in following order:

1. @param
2. @return
3. @throw
4. @throws
5. @expectedException
6. @expectedExceptionCode
7. @expectedExceptionMessage
8. @dataProvider

Avoid excessive errors during try/catch validation

Code:

try {
    1;
}catch ( Exception $e ) {
    1;
}

Errors:

--------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------------
 20 | ERROR | [x] Closing brace of the "try" control structure must be last
    |       |     content on the line
    |       |     (CodingStandard.WhiteSpace.ControlStructureSpacing.LineAfterClose)
 20 | ERROR | [ ] Expected "}\ncatch (...) {\n"; found "}}catch (...) {\n"
    |       |     (CodingStandard.ControlStructures.ControlSignature)
 20 | ERROR | [x] Beginning of the "catch" control structure must be first
    |       |     content on the line
    |       |     (CodingStandard.WhiteSpace.ControlStructureSpacing.LineBeforeOpen)

Problematic place: no space before catch instead of catch placed on new line.

Add fixing capabilities to the sniffs

Don't include the Generic standard

The ruleset.xml currently includes the entire Generic standard and then selectively excludes some sniffs. But the Generic standard is not actually a standard, but a dumping ground for all generic sniffs. It also includes a lot of conflicting sniffs.

Including the whole standard in 2.x will now include the new DocComment generic sniff, which will cause problems with trying to make a backwards compatible FunctionComment sniff because you can't reference Generic.Commenting.DocComment in the ruleset.xml (to exclude the error message you don't want) because it doesn't exist in PHPCS 1.x.

I suggest changing the ruleset.xml file to only include the sniffs from the Generic standard that you want.

Keep an eye on the PHPCS release notes and you'll see when new sniffs are added to standards. If you like them, add them to your ruleset.xml over time as well.

Include PSR2.SwitchDeclaration sniff

See if https://github.com/squizlabs/PHP_CodeSniffer/blob/phpcs-fixer/CodeSniffer/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php can be included into standard.

Validate "boolean not" spacing

Need to validate/fix, that:

• no whitespace present after !
• single space present before ! (maybe don't check initially because of edge cases with IF statements where conditions split across several lines)

Create keyword order for class/method

Ensure that keywords abstract, final, static, public/protected/private are used in following order:

[abstract] [final] public/protected/private [static]

This applies to classes, traits, methods and class/trait properties.

Error message from PHP-CS-Fixer: Visibility MUST be declared on all properties and methods; abstract and final MUST be declared before the visibility; static MUST be declared after the visibility. (see https://github.com/fabpot/PHP-CS-Fixer/blob/54b01c018bffe3ed20d1e138f2bc885597c59f50/Symfony/CS/Fixer/PSR2/VisibilityFixer.php).

I think there are sniffs already that checks for private/protected/public presence, so here we just need to assert their order.

Assignment by reference formatting

The =& should look exactly like that, so any of below is wrong:

$a = & $b;
$a = &$b;

Some info on topic: https://github.com/wimg/PHPCompatibility/tree/master/Sniffs/PHP

Require empty line inside class/interface/trait declaration

Make sure, that an empty line right after opening brace and right before closing brace of a class/interface/trait is required.

In inline function call no spaces required inside braces

Incorrect:

$this->setParams( array_merge($parsedParams, $params) );
$this->setParams(array_merge($parsedParams, $params) );
$this->setParams( array_merge($parsedParams, $params));
some_function( array_merge($parsedParams, $params) );
some_function(array_merge($parsedParams, $params) );
some_function( array_merge($parsedParams, $params));

Correct:

$this->setParams(array_merge($parsedParams, $params));
some_function(array_merge($parsedParams, $params));

ValidVariableName assert only declaration

Currently ValidaVariableName sniff asserts both variable declaration and usage. This will show 5 warnings for cases when variable is declared once and used 4 times.

Instead we should only assert variable declaration.

Allow blank lines after IF, when next statement is return

We're inside switch statement here if that matters.

Don't require `callable` typehint

Don't require callable type hint since using it will prevent fixed code from working on PHP 5.3 and below.

Allow PHP4 constructors in "WrongParentCall" sniff

Right now following code is reported as error:

<?php

class ClassC extends ClassA {

    function ClassC() {
        parent::ClassB();
    }

}

I think that we should totally ignore parent:: call validation, that happens from function having same name as class name.

Class constant declaration

Ensure that class constant declaration looks like this:

class SomeClass
{

const CONSTANT_NAME = 'value';

}

Rule: "const" word > one space > constant name > one space > "=" > one space > value;

Maybe allow indentation of several constants defined in a row.

Check for uppercase constants

Check that all class constants are uppercased.

The "ConcatenationSpacingSniff" should validate extra spaces

Right now "ConcatenationSpacingSniff" sniff validates, that at least one space is used to concatenate parts of a string. We should change it to be exactly one space.

In case if previous/next string ends up on another line we shouldn't validate other half:

// check both spaces
$s1 = 'a' . 'b';

// only check space before dot
$s2 = 'a' .
'b';

// only check space after dot
$s3 = 'a'
. 'b';

File: https://github.com/aik099/CodingStandard/blob/master/CodingStandard/Sniffs/Strings/ConcatenationSpacingSniff.php

Exclude several sniff errors on extended classes

If class name starts from E then it's safe to assume that it extends something. Let's ignore following errors in such files:

• Member variable "PerPage" is not in valid camel caps format (CodingStandard.NamingConventions.ValidVariableName.MemberNotCamelCaps)
• Public method name "EUserList::GetTotalPages" is not in camel caps format (CodingStandard.NamingConventions.ValidFunctionName.ScopeNotCamelCaps)

Maybe we can cheat a bit and exclude files that starts with e_ from checks right in the ruleset.xml.

Empty line between class/method and it's PHPDoc

Empty line between class/method and it's PHPDoc should be reported as error. Tricky case if instead of finding /** or */ we'll find } or { then it's ok, because this means, that method has no PHPDoc.

/**
 * @expectedException \aik099\QATools\PageObject\Exception\UrlException
 * @expectedExceptionCode \aik099\QATools\PageObject\Exception\UrlException::TYPE_INVALID_URL
 */

public function testConstructorMissingProtocol()
{
    $builder = new Builder($normalized_components);
}

Create license validation sniff

Create license validation sniff. There is something already created for PHPCS coding standard.

We need to validate that there is license/file comment in each PHP file and it looks about right (don't validate exact phrases to allow sniff to be used on projects with different licenses).

Change error code in ArraySniff

The ArraySniff among other things check that no space is permitted before closing parenthesis of an inline array. The code for this error is SpaceAfterClose, but it should be SpaceBeforeClose instead.

This doesn't really break anything, but makes things harder to understand if you see error code only without corresponding error message.

Don't require one space after "//" for inline comment sniff

Right now exactly 1 space is required after // for inline comments. PhpStorm puts // at the start of the line (ignoring indentation) and that makes sniff report it as error instantly.

I propose that we require at least one whitespace and exactly one whitespace.

Create sniff to ensure empty after { and before } in class/interface/trait

There is a Squiz.WhiteSpace.FunctionSpacing sniff, that checks, that:

• there 1 line before/after function declaration
• there is an empty line after last function declaration in a class (only since PHPCS 2.2.0, squizlabs/PHP_CodeSniffer@4bc2418)

I mistakenly assumed that check was made to ensure 1 empty line after class opening brace.

---

Need to write sniff that will:

• ensure 1 empty line after { (basically line after line where { is placed should be empty)
• ensure 1 empty line before } (basically line before line where } is placed should be empty)

Add tab indentation support to sniffs

Currently some of PEAR/PSR2 standard sniffs are used as-is, but they only support 4 space indentation fixing. We need to add support for tab indentation as well.

Make "AbstractSniffUnitTest" compatible with PHPCS 2.x

Make "AbstractSniffUnitTest" compatible with PHPCS 2.x and at same time working with PHPCS 1.x as well.

Report write operation to function parameters as error

When inside the method/function a write operation is detected on given parameter (e.g. somebody overwriting it's value), then report that as an error.

File licensing comment must be first content in file

Validate that DocBlock is the the first content (excluding whitespaces) after <?php tag in every PHP file.

Don't validate DocBlock content at this stage.

Prohibit usage of "$this" in "@return"

When @return $this is detected in PHPDoc, then suggest using self or static instead.

Validate multi-line object operator ("->") call indentation

The object operator (->) should be checked for:

• be first symbol on a line if it's a chained call
• chained call on a new line must be indented

$object->a->b->c;
$object->a->b()->c
    ->d;

Ensure 2 empty lines after namespace declaration

If a namespace declaration exists in a file, then the next construct (e.g. class/trait/interface/comment, etc.) should be after 2 lines only.

Validate "parent::" method name

We need to validate that method name after parent:: indeed matches the current method name.

function nameOne() {
    parent::nameTwo();

   ...
}