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

ignoring single-quoted reserved field names #94

Closed
nilslice opened this issue Nov 6, 2018 · 4 comments
Closed

ignoring single-quoted reserved field names #94

nilslice opened this issue Nov 6, 2018 · 4 comments

Comments

@nilslice
Copy link

nilslice commented Nov 6, 2018

func TestReservedFieldNamesSingleQuote(t *testing.T) {
	r := new(Reserved)
	p := newParserOn(`reserved 'foo', 'bar';`)
	_, _, _ = p.next()
	err := r.parse(p)
	if err != nil {
		t.Fatal(err)
	}
	if got, want := len(r.FieldNames), 2; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := r.FieldNames[0], "'foo'"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := r.FieldNames[1], "'bar'"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
}

I'm not certain about the expected "'bar'" or "'foo'" literals in the test, but the len(r.FieldNames) should be 2.

This is in reference to a user report from one of my repos: nilslice/protolock#65

@nilslice
Copy link
Author

nilslice commented Nov 6, 2018

My PR (#95) seems to make the reserved w/ single-quote test pass, but when actually parsing a full message, I get this failure from the scanner pkg:

go scanner error at <input>:7:12 = illegal char literal        
go scanner error at <input>:7:21 = illegal char literal        
go scanner error at <input>:8:20 = illegal char literal

The message in question is:

syntax = "proto3";

package test;

message Channel {
  reserved 'thing', 'another';
  reserved "more", 'mixed';
  int64 id = 1;
  string name = 2;
  string description = 3;
}

@emicklei
Copy link
Owner

emicklei commented Nov 6, 2018

thank you for reporting this issue. I will investigate it and review the PR

emicklei pushed a commit that referenced this issue Nov 6, 2018
@emicklei
Copy link
Owner

emicklei commented Nov 6, 2018

the parser is not (completely) validating the proto file so empty reserved names should not be ignored. Tools such as protolint and protoc will.
Please have a look at PR #96 which uses your fix to add a scanner mode and adds your message test.

@nilslice
Copy link
Author

nilslice commented Nov 6, 2018

the parser is not (completely) validating the proto file so empty reserved names should not be ignored. Tools such as protolint and protoc will.

makes sense -- great callout.

Please have a look at PR #96 which uses your fix to add a scanner mode and adds your message test.

I will run through the test and build my tool against this version and try it out. Thanks!

emicklei added a commit that referenced this issue Nov 10, 2018
* test for message in issue #94
* handle single quoted strings, fixes issue #94
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

No branches or pull requests

2 participants