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

Can not create mapper for protobufs with repeated field named field #23

Open
jjttjj opened this issue Nov 8, 2023 · 1 comment
Open

Comments

@jjttjj
Copy link

jjttjj commented Nov 8, 2023

If I use java classes generated from this proto file:

syntax = "proto3";

package example;

// Message with a repeated field of one type
message ResponseTypeOne {
  repeated MessageTypeOne field = 1;
}

// Message with a repeated field of another type
message ResponseTypeTwo {
  repeated MessageTypeTwo field = 1;
}

message MessageTypeOne {
  int32 id = 1;
}

message MessageTypeTwo {
  int32 id = 1;
}

Then try to define a mapper for one of the types:

(pronto.core/defmapper M [example.Example$ResponseTypeOne])

I get:

IllegalArgumentException Must hint overloaded method: getField
	clojure.lang.Compiler$NewInstanceMethod.parse (Compiler.java:8497)
	clojure.lang.Compiler$NewInstanceExpr.build (Compiler.java:8059)
	clojure.lang.Compiler$NewInstanceExpr$DeftypeParser.parse (Compiler.java:7935)
	clojure.lang.Compiler.analyzeSeq (Compiler.java:7107)
	clojure.lang.Compiler.analyze (Compiler.java:6789)
{:clojure.error/phase :compile-syntax-check,
 :clojure.error/line 1,
 :clojure.error/column 1,
 :clojure.error/source "NO_SOURCE_PATH",
 :clojure.error/symbol deftype*}

I'm pretty sure this only happens in cases where the field is named field, and it's repeated. But it's possible I've confused myself with some detail of this during looking into this.

The relevant java setter is this:

    public example.Example.MessageTypeOne getField(int index) {
      return field_.get(index);
    }

This seems to stem from this code (and similar other excerpts) from macroexpanding that defmapper call:

(clojure.core/deftype
     transient_example_Example__ResponseTypeOne
     [pojo2369 editable?]
   example.Example$ResponseTypeOneOrBuilder
   (getField
     [this__2302__auto__ G__44449]
     (. pojo2369 getField G__44449)))

I can get this to eval by type hinting both the method name and the integer param:

  (clojure.core/deftype
      transient_example_Example__ResponseTypeOne
      [pojo2369 editable?]
    example.Example$ResponseTypeOneOrBuilder
    (^example.Example$MessageTypeOne getField ;;added return type hint on method symbol
     [this__2302__auto__ ^int G__44449] ;;added type hint to int arg
     (. pojo2369 getField G__44449)))

I'm not immediately sure how to fit this into the pronto code generation though. I might take a shot at it later in the week, but wanted to report this first

@jjttjj
Copy link
Author

jjttjj commented Nov 8, 2023

Actually I realized there was some commented out code that seemed somewhat related to this. I had to tweak it slightly to add type hints to all args instead of non-integer args, but after doing that I was able to fix my issue in the example above, and it also didn't break anything in the relatively large real project I'm working on.

This is what I changed:
jjttjj@49ca872

There might still be some issues with this, and it could possibly break other things. I'm not fully sure what I'm doing. But feedback is welcome and I can do a PR on request.

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

1 participant