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

[ticket/12721] Improve the code sniffer: simple rules #2603

Merged
merged 9 commits into from Jun 16, 2014

Conversation

@Nicofuma Nicofuma changed the title Ticket/12721 [ticket/12717] Improve the code sniffer: simple rules Jun 15, 2014
@@ -318,7 +318,7 @@ function sphUnpackI64 ( $v )
return sprintf ( "%u", $lo );
}
// x32, int
elseif ( $hi==-1 )
else if ( $hi==-1 )

This comment has been minimized.

Copy link
@bantu

bantu Jun 15, 2014

Member

This file needs to be ignored completely (vendor).

This comment has been minimized.

Copy link
@Nicofuma

Nicofuma Jun 15, 2014

Author Member

updated

@bantu

This comment has been minimized.

Copy link
Member

commented Jun 15, 2014

The file phpBB/includes/sphinxapi.php is from a vendor and needs to be ignored completely.
The descriptions in the xml files need to be wordwrapped to 80 or 100 chars per line.
I would prefer if the description of the Sniffs would be in MUST or MUST NOT format.

@@ -22,6 +22,9 @@
<!-- PHP keywords MUST be in lower case. -->
<rule ref="Generic.PHP.LowerCaseKeyword" />

<!-- Constructors should be named __construct, not after the class. -->

This comment has been minimized.

Copy link
@bantu

bantu Jun 15, 2014

Member

"Constructors MUST be called __construct() instead of after the class."

This comment has been minimized.

Copy link
@Nicofuma

Nicofuma Jun 15, 2014

Author Member

updated

@Nicofuma Nicofuma changed the title [ticket/12717] Improve the code sniffer: simple rules [ticket/12721] Improve the code sniffer: simple rules Jun 15, 2014
@nickvergessen

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2014

Tests can not be run, please rebase

@@ -1,4 +1,5 @@
<?php
// @codingStandardsIgnoreFile

This comment has been minimized.

Copy link
@bantu

bantu Jun 16, 2014

Member

This file must not be modified at all. Please add it to the ignore in build.xml instead.

This comment has been minimized.

Copy link
@Nicofuma

Nicofuma Jun 16, 2014

Author Member

the file is already in the ignore list.

@@ -20,6 +20,13 @@
<!-- Call-time pass-by-reference MUST not be used. -->
<rule ref="Generic.Functions.CallTimePassByReference.NotAllowed" />

<!-- Lowercase filenames are required. -->

This comment has been minimized.

Copy link
@bantu

bantu Jun 16, 2014

Member

Filenames MUST be lowercase.

This comment has been minimized.

Copy link
@Nicofuma

Nicofuma Jun 16, 2014

Author Member

updated

<!-- Lowercase filenames are required. -->
<rule ref="Generic.Files.LowercasedFilename" />

<!-- Function declarations follow the "BSD/Allman style".

This comment has been minimized.

Copy link
@bantu

bantu Jun 16, 2014

Member

Remove the BSD/Allman style reference. "The function brace MUST be on the line following the function declaration and MUST be indented to the same column as the start of the function declaration."

This comment has been minimized.

Copy link
@bantu

bantu Jun 16, 2014

Member

Wordwrap to 80 chars or 100 or so

This comment has been minimized.

Copy link
@bantu

bantu Jun 16, 2014

Member

Looks like we are using 80 for Squiz.Functions.FunctionDeclarationArgumentSpacing

This comment has been minimized.

Copy link
@Nicofuma

Nicofuma Jun 16, 2014

Author Member

updated

@@ -44,6 +63,9 @@
</rule>
<rule ref="Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingAfterHint" />

<!-- All PHP built-in functions should be lowercased when called. -->

This comment has been minimized.

Copy link
@bantu

bantu Jun 16, 2014

Member

All built-in PHP functions MUST be called lowercased.

This comment has been minimized.

Copy link
@Nicofuma

Nicofuma Jun 16, 2014

Author Member

updated

<!-- When referencing arrays you should not put whitespace around the opening bracket or before the closing bracket. -->
<rule ref="Squiz.Arrays.ArrayBracketSpacing" />

<!-- Verifies that there are not elseif statements. The else and the if should be separated by a space. -->

This comment has been minimized.

Copy link
@bantu

bantu Jun 16, 2014

Member

The "else if" statement MUST be written with a space between the words else and if.

@@ -35,6 +42,18 @@
<!-- Each file MUST end with exactly one newline character -->
<rule ref="PSR2.Files.EndFileNewline" />

<!-- When referencing arrays you should not put whitespace around the opening bracket or before the closing bracket. -->

This comment has been minimized.

Copy link
@bantu

bantu Jun 16, 2014

Member

When referencing arrays there MUST NOT be any whitespace around the opening bracket or before the closing bracket.

<!-- Verifies that there are not elseif statements. The else and the if should be separated by a space. -->
<rule ref="Squiz.ControlStructures.ElseIfDeclaration" />

<!-- There should be a space between each element of a foreach loop and the as keyword should be lowercase. -->

This comment has been minimized.

Copy link
@bantu

bantu Jun 16, 2014

Member

There MUST be a space between each element of a foreach loop.
The "as" keyword MUST be written in lowercase.

PHPBB3-12721
bantu added a commit to bantu/phpbb that referenced this pull request Jun 16, 2014
[ticket/12721] Improve the code sniffer: simple rules

* Nicofuma/ticket/12721:
  [ticket/12721] Update rules descriptions
  [ticket/12721] Add Generic.NamingConventions.ConstructorName in strict
  [ticket/12721] Add Squiz.Functions.LowercaseFunctionKeywords in legacy
  [ticket/12721] Add Squiz.ControlStructures.ForLoopDeclaration in legacy
  [ticket/12721] Add Squiz.ControlStructures.ForEachLoopDeclaration
  [ticket/12721] Add Squiz.ControlStructures.ElseIfDeclaration in legacy
  [ticket/12721] Add Generic.Functions.OpeningFunctionBraceBsdAllman
  [ticket/12721] Add Squiz.Arrays.ArrayBracketSpacing in the legacy ruleset
  [ticket/12721] Add Generic.Files.LowercasedFilename in the legacy ruleset
@bantu bantu merged commit afcca62 into phpbb:develop-ascraeus Jun 16, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@Sajaki

This comment has been minimized.

Copy link
Contributor

commented on 1aec0d2 Mar 20, 2016

It should be mentioned in the coding guidelines aswell. Currently in my ext, Class files are capitalised, but travis fails because of this rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.