Skip to content
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

verify that reserved names are valid identifiers #65

Merged
merged 2 commits into from
Oct 13, 2022

Conversation

jhump
Copy link
Member

@jhump jhump commented Oct 12, 2022

Currently (latest on master), protoc just warns when a reserved name is not a valid identifier. (Current release just ignores invalid names, just like buf does.) But I am hoping to land a change soon that will make it an actual error in protoc.

(For the whole saga of the changes in protoc see protocolbuffers/protobuf#6335 and protocolbuffers/protobuf#10586.)

This PR will make it an actual error to use an invalid name. This would be one of two known cases where buf is more strict than protoc (the other being that buf validates custom JSON names, but protoc does not... at least not yet: protocolbuffers/protobuf#5063). But both of these checks should be finding their way into a protoc release soon-ish.

@jhump jhump requested a review from pkwarren October 13, 2022 02:04
Copy link
Member

@pkwarren pkwarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

parser/validate.go Show resolved Hide resolved
parser/validate.go Show resolved Hide resolved
@jhump jhump enabled auto-merge (squash) October 13, 2022 19:15
@jhump jhump merged commit e606fac into main Oct 13, 2022
@jhump jhump deleted the jh/warn-bad-reserved-names branch October 13, 2022 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants