Skip to content

Commit

Permalink
Merge pull request #7 from snprajwal/fix-proto2
Browse files Browse the repository at this point in the history
fix: handle dereference in proto2 bindings
  • Loading branch information
ghostiam authored Nov 1, 2023
2 parents b213f22 + 07bc9c1 commit 1bc8154
Show file tree
Hide file tree
Showing 8 changed files with 285 additions and 2 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.PHONY: test
test:
cd testdata && make vendor
$(MAKE) -C testdata vendor
go test -v ./...

.PHONY: install
Expand Down
17 changes: 17 additions & 0 deletions processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,23 @@ func (c *processor) process(n ast.Node) (*Result, error) {

c.processInner(x)

case *ast.StarExpr:
f, ok := x.X.(*ast.SelectorExpr)
if !ok {
return &Result{}, nil
}

if !isProtoMessage(c.info, f.X) {
return &Result{}, nil
}

// proto2 generates fields as pointers. Hence, the indirection
// must be removed when generating the fix for the case.
// The `*` is retained in `c.from`, but excluded from the fix
// present in the `c.to`.
c.writeFrom("*")
c.processInner(x.X)

default:
return nil, fmt.Errorf("not implemented for type: %s (%s)", reflect.TypeOf(x), formatNode(n))
}
Expand Down
1 change: 1 addition & 0 deletions protogetter.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ func Run(pass *analysis.Pass, cfg *Config) ([]Issue, error) {
(*ast.AssignStmt)(nil),
(*ast.CallExpr)(nil),
(*ast.SelectorExpr)(nil),
(*ast.StarExpr)(nil),
(*ast.IncDecStmt)(nil),
(*ast.UnaryExpr)(nil),
}
Expand Down
2 changes: 1 addition & 1 deletion testdata/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ protoc:
--go_opt paths=source_relative \
--go-grpc_out proto \
--go-grpc_opt paths=source_relative \
proto/test.proto
proto/test.proto proto/test_proto2.proto
216 changes: 216 additions & 0 deletions testdata/proto/test_proto2.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions testdata/proto/test_proto2.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
syntax = "proto2";

option go_package = "github.com/ghostiam/protogetter/testdata/proto";

message TestProto2 {
required double d = 1;
required float f = 2;
required int32 i32 = 3;
required int64 i64 = 4;
optional uint32 u32 = 5;
optional uint64 u64 = 6;
optional bool t = 7;
optional bytes b = 8;
optional string s = 9;
}
17 changes: 17 additions & 0 deletions testdata/test_proto2.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package testdata

import (
"github.com/ghostiam/protogetter/testdata/proto"
)

func testInvalidProto2(t *proto.TestProto2) {
_ = *t.D // want `avoid direct access to proto field \*t\.D, use t\.GetD\(\) instead`
_ = *t.F // want `avoid direct access to proto field \*t\.F, use t\.GetF\(\) instead`
_ = *t.I32 // want `avoid direct access to proto field \*t\.I32, use t\.GetI32\(\) instead`
_ = *t.I64 // want `avoid direct access to proto field \*t\.I64, use t\.GetI64\(\) instead`
_ = *t.U32 // want `avoid direct access to proto field \*t\.U32, use t\.GetU32\(\) instead`
_ = *t.U64 // want `avoid direct access to proto field \*t\.U64, use t\.GetU64\(\) instead`
_ = *t.T // want `avoid direct access to proto field \*t\.T, use t\.GetT\(\) instead`
_ = t.B // want `avoid direct access to proto field t\.B, use t\.GetB\(\) instead`
_ = *t.S // want `avoid direct access to proto field \*t\.S, use t\.GetS\(\) instead`
}
17 changes: 17 additions & 0 deletions testdata/test_proto2.go.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package testdata

import (
"github.com/ghostiam/protogetter/testdata/proto"
)

func testInvalidProto2(t *proto.TestProto2) {
_ = t.GetD() // want `avoid direct access to proto field \*t\.D, use t\.GetD\(\) instead`
_ = t.GetF() // want `avoid direct access to proto field \*t\.F, use t\.GetF\(\) instead`
_ = t.GetI32() // want `avoid direct access to proto field \*t\.I32, use t\.GetI32\(\) instead`
_ = t.GetI64() // want `avoid direct access to proto field \*t\.I64, use t\.GetI64\(\) instead`
_ = t.GetU32() // want `avoid direct access to proto field \*t\.U32, use t\.GetU32\(\) instead`
_ = t.GetU64() // want `avoid direct access to proto field \*t\.U64, use t\.GetU64\(\) instead`
_ = t.GetT() // want `avoid direct access to proto field \*t\.T, use t\.GetT\(\) instead`
_ = t.GetB() // want `avoid direct access to proto field t\.B, use t\.GetB\(\) instead`
_ = t.GetS() // want `avoid direct access to proto field \*t\.S, use t\.GetS\(\) instead`
}

0 comments on commit 1bc8154

Please sign in to comment.