ghostiam / protogetter

Protobuf golang linter - use getters instead of fields.
MIT License
20 stars 0 forks source link

Error when applying to pointer field in fix mode #11

Open eyasy1217 opened 1 month ago

eyasy1217 commented 1 month ago

Hi.

When I apply a pointer field in fix mode, I get a compile error(IncompatibleAssign). Because the getter in the pointer field returns a value. For example, protoc generates the following code.

type Foo struct {
    // ...

    CancelPolicy *string `protobuf:"bytes,3,opt,name=cancel_policy,json=cancelPolicy,proto3,oneof" json:"cancel_policy,omitempty"`
}

func (x *Foo) GetCancelPolicy() string {
    if x != nil && x.CancelPolicy != nil {
        return *x.CancelPolicy
    }
    return ""
}

version

ghostiam commented 1 month ago

Hello, thank you for your interest in the project.

Could you show the code (before the fix), which after the fix no longer works? Since you provided code from a generated proto file, which should not be processed by the linter.

You might also want to look at: https://github.com/ghostiam/protogetter/issues/5#issuecomment-2072799729

eyasy1217 commented 1 month ago

Could you show the code (before the fix), which after the fix no longer works?

See bellow.

type MyFoo struct 
    CancelPolicy *string
}

// f's type is *Foo

m := MyFoo{
-   CancelPolicy: f.CancelPolicy,
+   CancelPolicy: f.GetCancelPolicy(), // cannot use f.GetCancelPolicy() (value of type string) as *string value in struct literal compiler(IncompatibleAssign)
}

Therefore, I modified it myself as follows.

+c := res.GetCancelPolicy()
m := MyFoo{
-   CancelPolicy: f.CancelPolicy,
+   CancelPolicy: &c,
}
ghostiam commented 1 month ago

Now I understand. I’ll think about how to solve this problem, but here the field is already beyond what protogetter scans.

At this point, if you really need nil in a given field (rather than zero value), I would suggest adding //nolint:protogetter

m := MyFoo{
    CancelPolicy: f.CancelPolicy, //nolint:protogetter
}
jalaziz commented 3 weeks ago

I've run into this and would love to see it work out of the box.

It seems to me that this linter should allow assigning the field directly when the destination type is known to be a pointer type. A potentially trickier thing to solve for is accessing a nested field safely but then assigning the raw field to a pointer field.