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

Google style: allow single character variables #3702

Closed
romani opened this Issue Jan 9, 2017 · 11 comments

Comments

Projects
None yet
3 participants
@romani
Member

romani commented Jan 9, 2017

Based on google/styleguide#214

Local variables and catch arguments, method parameters should be allowed to be one characters

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jan 25, 2017

Contributor

Chapter 5.2.6 Parameter Names will be partially (for methods parameters names) covered by #3722

Should be covered in scope of this issue
https://google.github.io/styleguide/javaguide.html#s5.2.7-local-variable-names
https://google.github.io/styleguide/javaguide.html#s5.2.6-parameter-names (for catch parameters)

Contributor

MEZk commented Jan 25, 2017

Chapter 5.2.6 Parameter Names will be partially (for methods parameters names) covered by #3722

Should be covered in scope of this issue
https://google.github.io/styleguide/javaguide.html#s5.2.7-local-variable-names
https://google.github.io/styleguide/javaguide.html#s5.2.6-parameter-names (for catch parameters)

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jan 25, 2017

Contributor

@romani

  1. I do not find any information about identifiers like aParam. Should we allow them. Google Style guide does not prohibit their usage.

Local variable names are written in lowerCamelCase.
Even when final and immutable, local variables are not considered to be constants, and should not be styled as constants.

Parameter names are written in lowerCamelCase.
One-character parameter names in public methods should be avoided.

lowerCamelCase section does not say anything about such parameters names.

  1. Google Style Guide also states that XmlHttpRequest is correct but XMLHTTPRequest is not. We need to update check's regexpes to disallow consequences of upper case characters. For example AAA.
Contributor

MEZk commented Jan 25, 2017

@romani

  1. I do not find any information about identifiers like aParam. Should we allow them. Google Style guide does not prohibit their usage.

Local variable names are written in lowerCamelCase.
Even when final and immutable, local variables are not considered to be constants, and should not be styled as constants.

Parameter names are written in lowerCamelCase.
One-character parameter names in public methods should be avoided.

lowerCamelCase section does not say anything about such parameters names.

  1. Google Style Guide also states that XmlHttpRequest is correct but XMLHTTPRequest is not. We need to update check's regexpes to disallow consequences of upper case characters. For example AAA.
@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 25, 2017

Member

@MEZk

I do not find any information about identifiers like aParam.

What about this?
https://google.github.io/styleguide/javaguide.html#s5.1-identifier-names

In Google Style special prefixes or suffixes, like those seen in the examples name_, mName, s_name and kName, are not used.

Member

rnveach commented Jan 25, 2017

@MEZk

I do not find any information about identifiers like aParam.

What about this?
https://google.github.io/styleguide/javaguide.html#s5.1-identifier-names

In Google Style special prefixes or suffixes, like those seen in the examples name_, mName, s_name and kName, are not used.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jan 25, 2017

Contributor

@rnveach
Sorry, I did not notice that on the top and thought that identifiers did not refer to parameters) My fault.
What about point number 2?

Contributor

MEZk commented Jan 25, 2017

@rnveach
Sorry, I did not notice that on the top and thought that identifiers did not refer to parameters) My fault.
What about point number 2?

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jan 25, 2017

Member

thought that identifiers did not refer to parameters

We name parameter name as IDENT in CS tree, so I assume identifiers means all variable names.
5.1 says to all identifiers, while 5.2 and sub-sections say identifiers by type. Parameters are listed in 5.2.6. So yes, I believe everything is an identifier.

We need to update check's regexpes to disallow consequences of upper case characters.

It sounds reasonable. AbbreviationAsWordInName should be set to 0 too like we are planning to do in our own config at #3721 (comment).

Member

rnveach commented Jan 25, 2017

thought that identifiers did not refer to parameters

We name parameter name as IDENT in CS tree, so I assume identifiers means all variable names.
5.1 says to all identifiers, while 5.2 and sub-sections say identifiers by type. Parameters are listed in 5.2.6. So yes, I believe everything is an identifier.

We need to update check's regexpes to disallow consequences of upper case characters.

It sounds reasonable. AbbreviationAsWordInName should be set to 0 too like we are planning to do in our own config at #3721 (comment).

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jan 25, 2017

Contributor

https://google.github.io/styleguide/javaguide.html#s5.1-identifier-names is a bit controversial.

They says:

Identifiers use only ASCII letters and digits, and, in a small number of cases noted below, underscores. Thus each valid identifier name is matched by the regular expression \w+ .

\w+ matches any word character (equal to [a-zA-Z0-9_])
+ quantifier — Matches between one and unlimited times, as many times as possible, giving back as needed.

So one can think that identifiers with underscore are allowed: parameter_number1, parameter_number. But then Google Style states that

In Google Style special prefixes or suffixes, like those seen in the examples name_, mName, s_name and kName, are not used.

and

Parameter names are written in lowerCamelCase.

and

Local variable names are written in lowerCamelCase.

They should have prohibited underscore usage in the first sentence.

Contributor

MEZk commented Jan 25, 2017

https://google.github.io/styleguide/javaguide.html#s5.1-identifier-names is a bit controversial.

They says:

Identifiers use only ASCII letters and digits, and, in a small number of cases noted below, underscores. Thus each valid identifier name is matched by the regular expression \w+ .

\w+ matches any word character (equal to [a-zA-Z0-9_])
+ quantifier — Matches between one and unlimited times, as many times as possible, giving back as needed.

So one can think that identifiers with underscore are allowed: parameter_number1, parameter_number. But then Google Style states that

In Google Style special prefixes or suffixes, like those seen in the examples name_, mName, s_name and kName, are not used.

and

Parameter names are written in lowerCamelCase.

and

Local variable names are written in lowerCamelCase.

They should have prohibited underscore usage in the first sentence.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jan 26, 2017

Member

@MEZk , do you still need comments from me ?

Member

romani commented Jan 26, 2017

@MEZk , do you still need comments from me ?

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jan 26, 2017

Contributor

@romani
No.I'm OK to start resolving the issues.

Contributor

MEZk commented Jan 26, 2017

@romani
No.I'm OK to start resolving the issues.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Jan 28, 2017

Contributor

@romani @rnveach
I came up with the following regexp pattern:
^[a-z]([a-z0-9]+[A-Z]?)*$

Examples:
a - MATCHES
a1 - MATCHES
camelCase - MATCHES
param1 - MATCHES
A - DOES NOT MATCH
A1 - DOES NOT MATCH
name_ - DOES NOT MATCH
s_name - DOES NOT MATCH
aParam - DOES NOT MATCH
Param1 - DOES NOT MATCH
singleLineCComment - DOES NOT MATCH
paramABBB - DOES NOT MATCH
1asd - DOES NOT MATCH

paramA1 - MATCHES (?)
paramA1Asdas - MATCHES (?)
asdqA1A1 - MATCHES (?)

(?) - Google Style guide allows digits in identifiers but says nothing about their placement in Camel Case.

As you can see there are no underscores in the regexp as Google disallow s_name and name_.
I assume that _name should also be allowed as it it not prohibited by the guide. In this case the regexp will be ^_?[a-z]([a-z0-9]+[A-Z]?)*$
What do you think?

You can check both regexpses here
https://regex101.com/

Contributor

MEZk commented Jan 28, 2017

@romani @rnveach
I came up with the following regexp pattern:
^[a-z]([a-z0-9]+[A-Z]?)*$

Examples:
a - MATCHES
a1 - MATCHES
camelCase - MATCHES
param1 - MATCHES
A - DOES NOT MATCH
A1 - DOES NOT MATCH
name_ - DOES NOT MATCH
s_name - DOES NOT MATCH
aParam - DOES NOT MATCH
Param1 - DOES NOT MATCH
singleLineCComment - DOES NOT MATCH
paramABBB - DOES NOT MATCH
1asd - DOES NOT MATCH

paramA1 - MATCHES (?)
paramA1Asdas - MATCHES (?)
asdqA1A1 - MATCHES (?)

(?) - Google Style guide allows digits in identifiers but says nothing about their placement in Camel Case.

As you can see there are no underscores in the regexp as Google disallow s_name and name_.
I assume that _name should also be allowed as it it not prohibited by the guide. In this case the regexp will be ^_?[a-z]([a-z0-9]+[A-Z]?)*$
What do you think?

You can check both regexpses here
https://regex101.com/

MEZk added a commit to MEZk/checkstyle that referenced this issue Jan 28, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Jan 30, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Jan 30, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Jan 31, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Feb 2, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Feb 2, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Feb 4, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Feb 4, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Feb 7, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Feb 9, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

MEZk added a commit to MEZk/checkstyle that referenced this issue Feb 9, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide

rnveach added a commit that referenced this issue Feb 9, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide
@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Feb 9, 2017

Member

Fix is merged

Member

rnveach commented Feb 9, 2017

Fix is merged

@rnveach rnveach closed this Feb 9, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Feb 9, 2017

Member

It should be noted that Google Style never responded to our requests to update their document.

Issue: google/styleguide#226
PR: google/styleguide#227

Member

rnveach commented Feb 9, 2017

It should be noted that Google Style never responded to our requests to update their document.

Issue: google/styleguide#226
PR: google/styleguide#227

@rnveach rnveach added this to the 7.6 milestone Feb 9, 2017

bamapookie added a commit to bamapookie/checkstyle that referenced this issue Feb 10, 2017

Issue #3702: Allow single character names in local variables, method …
…and catch-blocks parameters names in accordance with Google Style Guide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment