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

ParameterNameCheck: new scope and excludeScope properties #3473

Closed
agcuda opened this Issue Sep 28, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@agcuda
Contributor

agcuda commented Sep 28, 2016

Google Style Guide says that one-character parameter names in public methods should be avoided. There is no such restriction for methods at different visibility scopes.
It seems necessary to add scope and excludeScope (of type http://checkstyle.sourceforge.net/property_types.html#scope) to ParameterName, so that the rule above could be implemented with:

        <module name="ParameterName">
            <property name="format" value="^[a-z][a-zA-Z0-9]*$"/>
            <property name="excludeScope" value="public"/>
            <message key="name.invalidPattern"
             value="Parameter name ''{0}'' must match pattern ''{1}''."/>
        </module>
        <module name="ParameterName">
            <property name="format" value="^[a-z][a-zA-Z0-9]+$"/>
            <property name="scope" value="public"/>
            <message key="name.invalidPattern"
             value="Public parameter name ''{0}'' must match pattern ''{1}''."/>
        </module>

@romani romani added the approved label Sep 29, 2016

@romani romani changed the title from Add scope selection attributes to ParameterNameCheck to ParameterNameCheck: new scope and excludeScope properties Sep 29, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Sep 29, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Sep 30, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Sep 30, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Oct 3, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Oct 13, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Oct 26, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Oct 26, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Oct 30, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Nov 3, 2016

romani added a commit that referenced this issue Nov 5, 2016

@romani romani added the new feature label Nov 5, 2016

@romani romani added this to the 7.3 milestone Nov 5, 2016

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Nov 5, 2016

Member

@agcuda ,

we need to update google-style.xml and mapping table - http://checkstyle.sourceforge.net/google_style.html (/src/xdocs/google_style.xml)

Member

romani commented Nov 5, 2016

@agcuda ,

we need to update google-style.xml and mapping table - http://checkstyle.sourceforge.net/google_style.html (/src/xdocs/google_style.xml)

@agcuda

This comment has been minimized.

Show comment
Hide comment
@agcuda

agcuda Nov 7, 2016

Contributor

@romani Ok. Should I prepare a PR referencing this issue or #3381?

Contributor

agcuda commented Nov 7, 2016

@romani Ok. Should I prepare a PR referencing this issue or #3381?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Nov 7, 2016

Member

You are right, lets reference #3381 .
This issue is done.

Member

romani commented Nov 7, 2016

You are right, lets reference #3381 .
This issue is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment