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
Resolve more java field accessor name conflicts #8198
Merged
zhangskz
merged 11 commits into
protocolbuffers:master
from
philsttr:resolve_java_field_name_accessor_conflict
Feb 3, 2022
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
af6ce22
Remove javanano from .gitignore.
philsttr 2529c60
Resolve more java field accessor name conflicts.
philsttr 7da00a5
Prevent java accessor name conflicts for fields with leading undersco…
philsttr 1f8baae
Merge branch 'master' into resolve_java_field_name_accessor_conflict
philsttr 4038de8
Improve comments/documentation for conversion from snake case to came…
philsttr 7c0c1fc
Merge branch 'resolve_java_field_name_accessor_conflict' of github.co…
philsttr 9f88f0c
Fix indents and typo
philsttr 8663cff
Unnest <pre> tag
philsttr bbd504b
improve grammar in comments
philsttr ce16ade
Remove ternary operator and improve comments
philsttr 7427439
Fix typo in comment
philsttr File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think snakeCaseToUpperCamelCase is only ever used for checking against specialFieldNames. Can we just keep specialFieldNames in lower camel case instead? This would simplify our code and IMO is a bit more intuitive anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The specialFieldNames are in UpperCamelCase to handle proto fields named
_my_field
, since_my_field
will beMyField
when it is converted to camel case (either upper or lower). It seemed more straightforward to keep the special field names in UpperCamelCase for comparison (since it handles proto fields named bothmy_field
,_my_field
, and__my_field
), rather than adding additional logic to handle proto fields starting with_
.Note the tests that use
ForbiddenWordsLeadingUnderscoreMessage
test this case (pun intended ;)).