google
`MemberName` checks class names too
As of 43ed6f3b9de2b6cb4d93488792287a945eeb5a1d, the MemberName check also flags type names. Resolving such violations is often much more involved than renaming (private) field names, possibly even prohibitive due to compatibility requirements.
Additionally, some of the suggestions are possibly controversial:
- To rename
MoreASTHelperstoMoreAstHelpers, while it relates toASTHelpers. - To rename
AssertThatThrownByIOExceptiontoAssertThatThrownByIoException, while this Refaster rule relates to Java's built-inIOExceptiontype. - To rename
TestNGToAssertJRulestoTestNgToAssertJRules, while the associated testing framework is named TestNG, not TestNg. - Similarly, to rename
MongoDbTextFilterUsagetoMongoDbTextFilterUsage, while the database is named MongoDB, not MongoDb.
Suggestion: perhaps this type name logic can be moved to a separate check, with a more suitable name. Otherwise, what about allowing the type name logic to be disabled using a flag?
CC @graememorgan.
None of those should, in principle, be controversial inside Google. Google's Java style guide has very well-defined (and as you can tell from ASTHelpers, not always followed!) rules about camelcasing. I think all of your "before" examples would be unstylish by those standards.
Of course, though, external users are not at all bound by our style guide. Would it be useful to just disable this entire check for third-party users? The entire thing is enforcing the style guide, which really ought to be opt-in.
Would it be useful to just disable this entire check for third-party users?
Hmm, that would be a shame, as for actual member names I so far have never seen it emit a suggestion I'd consider suppressing. Additionally, users who disagree with the rule as a whole can also just set it to OFF.
(While I'd still prefer one of the other suggestions made, ) I suppose the minimal change this issue advocates for would be to rename the check, as it now focusses on more than members. On our side I'll review whether we should turn the check back on and (perhaps) add selective suppressions instead.
NB: It'd be nice to see the style guide take an explicit stance of type names derived from IOException: I'd expect that most readers would find it surprising to see SomeIoException. But maybe it just takes getting used to. 🤔
@graememorgan I see that MemberName was renamed to IdentifierName in 4f630fc668d107b7801281ce527e1a83a782dc1b. Should I therefore close this issue, or are there plans to e.g. implement an allowlist for cases such as IOException? (I suppose I can live with either.)
I don't think we'd be too motivated to implement carved-out exemptions to the style guide. Internally, the style guide is meant to be viewed as entirely non-negotiable.
I'd be happy to accept a PR to flag class checking though.
NB: It'd be nice to see the style guide take an explicit stance of type names derived from IOException: I'd expect that most readers would find it surprising to see SomeIoException.
I think it does take a stance on it by not providing an exemption. URI is another obvious one. A field should be called something like URI fooUri; you don't get to carry forward the violation to URI fooURI. I think I share your view that classes are somehow different though, but I can't quite work out why.
I'd be happy to accept a PR to flag class checking though. [..] I think I share your view that classes are somehow different though, but I can't quite work out why.
Tnx! Based on this "somehow different but not sure why" feeling I proposed a PR that only allows users to opt-out of initialism checking in type names: #4646. I realize that this is rather niche, but there seems value in having the check unconditionally flag type names with underscores and those not starting with a capital letter.