Added support for short variable name exceptions

[kirillsablin]

Sometimes we need exclude variables like 'id' from short name checking.

New property for rule was introduced - 'exceptions'. It is comma-separated list of variables names to be excluded from short name checking.

Example usage:

    <rule ref="rulesets/naming.xml/ShortVariable" >
            <properties>
                <property name="exceptions" value="id,em" />
            </properties>
    </rule>

[Dynom]

+1 Seems like a good addition, please include it.

[Dynom]

It doesn't fully work, I currently get the exception "Property $exceptions does not exist."

My ruleset looks like this:

    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/cleancode.xml">
        <!-- Excluding static-access since it generates a lot of false-positives on namespaced classes -->
        <exclude name="StaticAccess" />
    </rule>
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/codesize.xml" />
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/controversial.xml" />
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/design.xml">
    </rule>
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/naming.xml">
        <property name="minimum" value="3" />
        <property name="maximum" value="30" />
        <property name="checkParameterizedMethods" value="true" />
    </rule>
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/naming.xml/ShortVariable">
        <properties>
            <!--
            * em - Doctrine entity manager
            * e - Short exceptions variable
            * id - short identifier
            -->
            <property name="exceptions" value="id,em,e" />
        </properties>
    </rule>

    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/unusedcode.xml" />

[Dynom]

When I leave out the following section, it seems to work:

    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/naming.xml">
        <property name="minimum" value="3" />
        <property name="maximum" value="30" />
        <property name="checkParameterizedMethods" value="true" />
    </rule>

My guess is that there is a design problem when using multiple rulesets (in this example naming.xml is included twice). I'm probably doing something wrong here, any suggestions to improve my definition would be appreciated.

[kirillsablin]

This problem arises when using entire ruleset (naming in this case). I'll make patch for this in near future.
And it seems you should use individual rules to set properties

[kirillsablin]

I've made pull request with fix #81

[Dynom]

PR #81 doesn't fix the problem, it only suppresses the error.

There are more problems, when I debug the $properties property it's clear that not only "exceptions" isn't passed along, but also "maximum" and "checkParameterizedMethods" are voided. Only "minimum" is available, but that's the default with value "3". When I override it in the properties section, it's ignored.

To reproduce:

<ruleset name="foo"
         xmlns="http://pmd.sf.net/ruleset/1.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0
                      http://pmd.sf.net/ruleset_xml_schema.xsd"
         xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd">
    <description>
        Foo.
    </description>

    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/cleancode.xml">
        <!-- Excluding static-access since it generates a lot of false-positives on namespaced classes -->
        <exclude name="StaticAccess" />
    </rule>
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/codesize.xml" />
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/controversial.xml" />
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/design.xml"></rule>
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/naming.xml">
        <properties>
            <!--
            * em - Doctrine entity manager
            * e - Short exceptions variable
            * id - short identifier
            -->
            <property name="exceptions" value="id,em,e" />
            <property name="minimum" value="4" /> <!-- default: 3 -->
            <property name="maximum" value="30" />
            <property name="checkParameterizedMethods" value="true" />
        </properties>
    </rule>
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/unusedcode.xml" />
</ruleset>

[Dynom]

@manuelpichler Please re-open PR #66

[kirillsablin]

To use parameters you have to assign those params to individual rules instead of to ruleset.

<rule ref="rulesets/naming.xml/ShortVariable" >
        <properties>
            <property name="exceptions" value="id" />
        </properties>
</rule>

So if you use whole ruleset as in your example then default values will be used.
#81 fixes default value for "whole ruleset" mode.

[Dynom]

Ah thanks @kirillsablin. I find it very counter-intuitive, but at least it works now. Thanks for your time!

For reference, a working example:

<ruleset name="foo"
         xmlns="http://pmd.sf.net/ruleset/1.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0
                      http://pmd.sf.net/ruleset_xml_schema.xsd"
         xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd">
    <description>
        Foo.
    </description>
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/cleancode.xml">
        <!-- Excluding static-access since it generates a lot of false-positives on namespaced classes -->
        <exclude name="StaticAccess" />
    </rule>
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/codesize.xml" />
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/controversial.xml" />
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/design.xml"></rule>
    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/naming.xml/ShortVariable">
        <properties>
            <!--
            * em - Doctrine entity manager
            * e - Short exceptions variable
            * id - short identifier
            -->
            <property name="exceptions" value="id,em,e" />
            <property name="minimum" value="3" />
        </properties>
    </rule>

    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/naming.xml/LongVariable">
        <properties>
            <property name="maximum" value="30" />
        </properties>
    </rule>

    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/naming.xml/BooleanGetMethodName">
        <properties>
            <property name="checkParameterizedMethods" value="true" />
        </properties>
    </rule>

    <rule ref="vendor/phpmd/phpmd/src/main/resources/rulesets/unusedcode.xml" />
</ruleset>

[str]

I don't follow. Are there exceptions added by default? (like id, e, em) or do I need to define them myself?

If there are exceptions added by default, can we suggest more by default? ($to, $db for example).

[ravage84]

@str yes, yes you can and yes you may. But please do not misuse old issues. Thanks.