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

Using compare plugin does not work for oneof #221

Open
johanbrandhorst opened this issue Oct 25, 2016 · 7 comments
Open

Using compare plugin does not work for oneof #221

johanbrandhorst opened this issue Oct 25, 2016 · 7 comments

Comments

@johanbrandhorst
Copy link
Member

johanbrandhorst commented Oct 25, 2016

Hi,

When using a message which contains a message with a oneof variable and using the setting to generate compare methods, the generated interface for the variable does not have the Compare() function generated as part of the interface.

Here is a gist which reproduces the error:

https://gist.github.com/johanbrandhorst/783e0908940a2f81d7aef7b1ac93472e

The offending line is my.pb.go:44:

type isValue_Type interface {
    isValue_Type()
}

Should be

type isValue_Type interface {
    isValue_Type()
    Compare(interface{}) int
}
@johanbrandhorst
Copy link
Member Author

It also generates invalid tests, but I will raise that as a separate issue

@awalterschulze
Copy link
Member

I think this is the same issue

@awalterschulze
Copy link
Member

The compare plugin was probably not tested with a oneof message

@johanbrandhorst
Copy link
Member Author

Ok, I'll skip that issue

@johanbrandhorst
Copy link
Member Author

This might also apply to the ProtoSize plugin, should I make a separate issue for that?

@awalterschulze
Copy link
Member

Do you get a compile error, then yes.

@awalterschulze awalterschulze changed the title Using compare_all with proto oneof generates invalid Interface Using compare plugin does not work for oneof Nov 6, 2016
@awalterschulze
Copy link
Member

Here is a design for a fix:

We have to generated a type switch for each possible one of type for this and that.
This will then assign the oneof field number for each type to a thisType+NumGen.Next() and thatType+NumGen.Next() variable.
Then we compare the numbers, if they are equal, the Compare method for that field is called.
If they differ we return 1 or -1 as appropriate.

Anybody up for implementing this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants