aik099 / codingstandard Goto Github PK
View Code? Open in Web Editor NEWThe PHP_CodeSniffer coding standard I'm using on all of my projects
License: BSD 3-Clause "New" or "Revised" License
The PHP_CodeSniffer coding standard I'm using on all of my projects
License: BSD 3-Clause "New" or "Revised" License
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.
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.
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.
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
.
Maybe we should remove empty DocBlocks when we find them.
Make "AbstractSniffUnitTest" compatible with PHPCS 2.x and at same time working with PHPCS 1.x as well.
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));
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.
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
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.
Validate, that when present these tags must be in following order:
@param
@return
@throw
@throws
@expectedException
@expectedExceptionCode
@expectedExceptionMessage
@dataProvider
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.
We need to validate that method name after parent::
indeed matches the current method name.
function nameOne() {
parent::nameTwo();
...
}
The object operator (->
) should be checked for:
$object->a->b->c;
$object->a->b()->c
->d;
When @return $this
is detected in PHPDoc, then suggest using self
or static
instead.
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.
Don't require callable
type hint since using it will prevent fixed code from working on PHP 5.3 and below.
Following must be checked:
thePropertyName
or _thePrivatePropertyName
)_
used for word separation (e.g. $local_variable_name
and not $_local_variable_name
or even not $localVariableName
)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.
Need to validate/fix, that:
!
!
(maybe don't check initially because of edge cases with IF statements where conditions split across several lines)Decide on whatever the:
are functions or not and enforce corresponding syntax. Maybe Squiz standard already have a sniff for that.
Make sure, that an empty line right after opening brace and right before closing brace of a class/interface/trait is required.
DocBlock:
/**
* Allows to override standard permission mapping
*
* @return void
* @access protected
* @see kEventHandler::$permMapping
*/
protected function mapPermissions()
Following scenarios (for reporting) are possible:
@access
tag present - do nothing@access
tag present, but doesn't match method/property visibility (consider missing visibility as public
) - report mismatchFixing scenarios:
@access
tag and remove @access
tag@access
tag, then remove @access
tagEnsure 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.
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' => ''),
),
);
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.
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).
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):
Related to: #78
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.
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
Check that all class constants are uppercased.
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.
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';
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.
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.
Add Sniff for:
There is a Squiz.WhiteSpace.FunctionSpacing
sniff, that checks, that:
I mistakenly assumed that check was made to ensure 1 empty line after class opening brace.
Need to write sniff that will:
{
(basically line after line where {
is placed should be empty)}
(basically line before line where }
is placed should be empty)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).
Validate that:
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)We need to validate that for cases when:
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.
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.
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.
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.
See if https://github.com/squizlabs/PHP_CodeSniffer/blob/phpcs-fixer/CodeSniffer/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php can be included into standard.
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);
}
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:
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.
There must be:
Tricky cases (can cheat by checking for following closing parenthesis):
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.