[Phpmyadmin-devel] New PHPCS rule
Madhura Jayaratne
madhura.cj at gmail.com
Wed Dec 18 06:10:02 CET 2013
On Wed, Dec 18, 2013 at 3:09 AM, Hugues Peccatte
<hugues.peccatte at gmail.com>wrote:
> 2013/12/17 Hugues Peccatte <hugues.peccatte at gmail.com>
>
>> 2013/12/16 Marc Delisle <marc at infomarc.info>
>>
>>> Le 2013-12-16 13:12, Hugues Peccatte a écrit :
>>> > 2013/12/16 Marc Delisle <marc at infomarc.info <mailto:marc at infomarc.info
>>> >>
>>> >
>>> > Le 2013-12-15 14:36, Hugues Peccatte a écrit :
>>> > > Hi,
>>> > >
>>> > > What would you think about using the "nesting level" sniff
>>> please?
>>> > > This sniff permits to detect when there are too much nested
>>> levels
>>> > like:
>>> > > if (...) {
>>> > > if (...) {
>>> > > if (...) {
>>> > > if (...) {
>>> > > if (...) {
>>> > > ....
>>> > > }
>>> > > }
>>> > > }
>>> > > }
>>> > > }
>>> > >
>>> > > This kind of syntax often means that a refactoring is needed or a
>>> > rewrite.
>>> > >
>>> > > I can add it myself if you agree.
>>> > >
>>> >
>>> > Good idea but how many levels is considered "too many"? Can you
>>> give an
>>> > example of nesting levels you found?
>>> >
>>> >
>>> > --
>>> > Marc Delisle
>>> > http://infomarc.info | http://phpmyadmin.net
>>> >
>>> >
>>> ------------------------------------------------------------------------------
>>> > Rapidly troubleshoot problems before they affect your business.
>>> Most IT
>>> > organizations don't have a clear picture of how application
>>> performance
>>> > affects their revenue. With AppDynamics, you get 100% visibility
>>> > into your
>>> > Java,.NET, & PHP application. Start your 15-day FREE TRIAL of
>>> > AppDynamics Pro!
>>> >
>>> http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
>>> > _______________________________________________
>>> > Phpmyadmin-devel mailing list
>>> > Phpmyadmin-devel at lists.sourceforge.net
>>> > <mailto:Phpmyadmin-devel at lists.sourceforge.net>
>>> > https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>>> >
>>> >
>>> >
>>> > Hi,
>>> >
>>> > To add this new rule won't add to many PHPCS errors. I already test
>>> this
>>> > rule and there are currently not so much this errors in the sources. (I
>>> > don't have an exact count. I would say around 20-30.)
>>> >
>>> > See:
>>> http://www.squizlabs.com/php-codesniffer/php_codesniffer-ruleset.xml-support-in-svn
>>> > See:
>>> http://pear.php.net/manual/en/package.php.php-codesniffer.annotated-ruleset.php
>>> >
>>> > The rule could be:
>>> > <rule ref="Generic.Metrics.NestingLevel">
>>> > <properties>
>>> > <property name="nestingLevel" value="2" />
>>> > <property name="absoluteNestingLevel" value="5" />
>>> > </properties>
>>> > </rule>
>>> >
>>> > We don't need to define both properties.
>>> > "nestingLevel" is the number of nested level which throw a warning.
>>> > (default = 5)
>>> > "absoluteNestingLevel" is the maximum of allowed nested levels.
>>> (default
>>> > = 10)
>>> >
>>> > I think that the default values don't need to be overridden, so the
>>> rule
>>> > would be:
>>> > <rule ref="Generic.Metrics.NestingLevel" />
>>> >
>>> > Hugues.
>>>
>>> Yes, let's try that.
>>>
>>>
>>> --
>>> Marc Delisle
>>> http://infomarc.info | http://phpmyadmin.net
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Rapidly troubleshoot problems before they affect your business. Most IT
>>> organizations don't have a clear picture of how application performance
>>> affects their revenue. With AppDynamics, you get 100% visibility into
>>> your
>>> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of
>>> AppDynamics Pro!
>>>
>>> http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
>>> _______________________________________________
>>> Phpmyadmin-devel mailing list
>>> Phpmyadmin-devel at lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>>>
>>
>> Hi,
>>
>> I just committed the add of the new rule. Jenkins should find new errors
>> now.
>> [ ... wait for Jenkins result ... ]
>> My estimate was almost good. There are 27 new errors because of the
>> nested levels. I think that many of them could be fixed.
>>
>> I'll look at this.
>>
>> Hugues.
>>
>
> Hi,
>
> I've just found this page:
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples<https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#For-loop-with-test-function-call>
> You can find here a huge list of sniffs and their descriptions. I think
> that some of those sniffs could be useful for the phpMyAdmin source code.
> Let think about:
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#For-loop-with-test-function-call
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Unused-function-parameter
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Inline-control-structure
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Control-signature
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Method-scope
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Always-return
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Non-executable-code
>
> And there are more sniffs that I like:
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Function-call-argument-spacing
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Concatenation-must-be-surrounded-by-spaces
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Semicolon-spacing
> -
> https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Array-bracket-spacing
>
> (Yes, I'm a little bit annoying with code style...)
>
> TYPO3 seems to be another standard. I believe that I selected 1 or 2 of
> their sniffs. Maybe we could imagine to use it, but I never used it, for
> now at least...
>
> Please, let me know what you think about those sniffs, if some seems
> useful or useless.
> I'll try to use it locally and give you more information about number of
> errors/warnings thrown by those sniffs.
>
> Thanks,
> Hugues.
>
Hi,
These sniffs are definitely useful.
Even though not enforced by PHPCS, I believe we already comply to most of
them except probably for [1] & [2].
PMAStandard already include [3] & [4], and [5] is covered
by PEAR.ControlStructures.ControlSignature sniff. So I guess there is no
need to include them again.
However I am not too sure about [6]. Don't we sometimes have to have
methods with unused parameters because we are implementing interfaces?
[1]
https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Method-scope
[2]
https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Always-return
[3]
https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Function-call-argument-spacing
[4]
https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Inline-control-structure
[5]
https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Control-signature
[6]
https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Unused-function-parameter
<https://forge.typo3.org/projects/team-php_codesniffer/wiki/TYPO3v4_Sniffs_with_code_examples#Method-scope>
--
Thanks and Regards,
Madhura Jayaratne
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20131218/a6022f0c/attachment.html>
More information about the Developers
mailing list