Merge pull request #240 from mercari/show-warning-file-import

Show warning about file import
mercari · Aug 23, 2024 · 8128e53 · 8128e53
2 parents 5b6a227 + 2b52501
commit 8128e53
Show file tree

Hide file tree

Showing 58 changed files with 843 additions and 460 deletions.
diff --git a/_examples/11_multi_service/federation/federation.pb.go b/_examples/11_multi_service/federation/federation.pb.go
diff --git a/_examples/11_multi_service/federation/federation_grpc_federation.pb.go b/_examples/11_multi_service/federation/federation_grpc_federation.pb.go
diff --git a/_examples/11_multi_service/federation/other_grpc_federation.pb.go b/_examples/11_multi_service/federation/other_grpc_federation.pb.go
diff --git a/_examples/11_multi_service/proto/federation/federation.proto b/_examples/11_multi_service/proto/federation/federation.proto
@@ -7,8 +7,8 @@ import "federation/reaction.proto";
 
 option go_package = "example/federation;federation";
 
-option (grpc.federation.file)= {
-  import: ["comment/comment.proto", "favorite/favorite.proto"]
+option (grpc.federation.file) = {
+  import: ["favorite/favorite.proto"]
 };
 
 service FederationService {

diff --git a/compiler/compiler.go b/compiler/compiler.go
@@ -28,6 +28,7 @@ import (
 type Compiler struct {
 	importPaths  []string
 	manualImport bool
+	importRule   bool
 }
 
 // Option represents compiler option.
@@ -50,6 +51,13 @@ func ManualImportOption() Option {
 	}
 }
 
+// ImportRuleOption used to reference proto files imported by grpc.federation.file.import rule.
+func ImportRuleOption() Option {
+	return func(c *Compiler) {
+		c.importRule = true
+	}
+}
+
 // New creates compiler instance.
 func New() *Compiler {
 	return &Compiler{}
@@ -183,6 +191,9 @@ func (c *Compiler) Compile(ctx context.Context, file *source.File, opts ...Optio
 	}
 	files := []string{relPath}
 	files = append(files, file.Imports()...)
+	if c.importRule {
+		files = append(files, file.ImportsByImportRule()...)
+	}
 	linkedFiles, err := compiler.Compile(ctx, files...)
 	if err != nil {
 		return nil, &CompilerError{Err: err, ErrWithPos: r.errs}

diff --git a/generator/generator.go b/generator/generator.go
@@ -639,9 +639,11 @@ func CreateCodeGeneratorResponse(ctx context.Context, req *pluginpb.CodeGenerato
 	}
 	outputPathResolver := resolver.NewOutputFilePathResolver(opt.Path)
 	result, err := resolver.New(req.GetProtoFile(), resolver.ImportPathOption(opt.Path.ImportPaths...)).Resolve()
-	outs := validator.New().ToValidationOutputByResolverResult(result, err, validator.ImportPathOption(opt.Path.ImportPaths...))
-	if validator.ExistsError(outs) {
-		return nil, errors.New(validator.Format(outs))
+	if outs := validator.New().ToValidationOutputByResolverResult(result, err, validator.ImportPathOption(opt.Path.ImportPaths...)); len(outs) > 0 {
+		if validator.ExistsError(outs) {
+			return nil, errors.New(validator.Format(outs))
+		}
+		fmt.Fprint(os.Stderr, validator.Format(outs))
 	}
 
 	var outDir string

diff --git a/grpc/federation/cel/bind.go b/grpc/federation/cel/bind.go
@@ -78,19 +78,3 @@ func MemberOverloadFunc(name string, self *cel.Type, args []*cel.Type, result *c
 		),
 	}
 }
-
-func toSelectorName(v ast.Expr) string {
-	switch v.Kind() {
-	case ast.SelectKind:
-		sel := v.AsSelect()
-		parent := toSelectorName(sel.Operand())
-		if parent != "" {
-			return parent + "." + sel.FieldName()
-		}
-		return sel.FieldName()
-	case ast.IdentKind:
-		return v.AsIdent()
-	default:
-		return ""
-	}
-}
diff --git a/grpc/federation/cel/conv.go b/grpc/federation/cel/conv.go
@@ -0,0 +1,63 @@
+package cel
+
+import "github.com/google/cel-go/common/ast"
+
+func ToIdentifiers(expr ast.Expr) []string {
+	var idents []string
+	switch expr.Kind() {
+	case ast.CallKind:
+		call := expr.AsCall()
+		idents = append(idents, ToIdentifiers(call.Target())...)
+		for _, arg := range call.Args() {
+			idents = append(idents, ToIdentifiers(arg)...)
+		}
+	case ast.IdentKind:
+		idents = append(idents, expr.AsIdent())
+	case ast.ListKind:
+		l := expr.AsList()
+		for _, e := range l.Elements() {
+			idents = append(idents, ToIdentifiers(e)...)
+		}
+	case ast.MapKind:
+		m := expr.AsMap()
+		for _, entry := range m.Entries() {
+			idents = append(idents, toEntryNames(entry)...)
+		}
+	case ast.SelectKind:
+		idents = append(idents, toSelectorName(expr))
+	case ast.StructKind:
+		idents = append(idents, expr.AsStruct().TypeName())
+		for _, field := range expr.AsStruct().Fields() {
+			idents = append(idents, toEntryNames(field)...)
+		}
+	}
+	return idents
+}
+
+func toEntryNames(entry ast.EntryExpr) []string {
+	var ident []string
+	switch entry.Kind() {
+	case ast.MapEntryKind:
+		ident = append(ident, ToIdentifiers(entry.AsMapEntry().Key())...)
+		ident = append(ident, ToIdentifiers(entry.AsMapEntry().Value())...)
+	case ast.StructFieldKind:
+		ident = append(ident, ToIdentifiers(entry.AsStructField().Value())...)
+	}
+	return ident
+}
+
+func toSelectorName(v ast.Expr) string {
+	switch v.Kind() {
+	case ast.SelectKind:
+		sel := v.AsSelect()
+		parent := toSelectorName(sel.Operand())
+		if parent != "" {
+			return parent + "." + sel.FieldName()
+		}
+		return sel.FieldName()
+	case ast.IdentKind:
+		return v.AsIdent()
+	default:
+		return ""
+	}
+}
diff --git a/internal/testutil/cmpopt.go b/internal/testutil/cmpopt.go
@@ -11,10 +11,10 @@ func ResolverCmpOpts() []cmp.Option {
 	return []cmp.Option{
 		cmpopts.IgnoreUnexported(resolver.VariableDefinition{}),
 		cmpopts.IgnoreFields(resolver.File{}, "Messages", "Services", "Enums", "Desc", "CELPlugins", "ImportFiles"),
-		cmpopts.IgnoreFields(resolver.Service{}, "CELPlugins"),
+		cmpopts.IgnoreFields(resolver.Service{}, "CELPlugins", "Desc"),
 		cmpopts.IgnoreFields(resolver.Package{}, "Files"),
-		cmpopts.IgnoreFields(resolver.Method{}, "Service"),
-		cmpopts.IgnoreFields(resolver.Message{}, "File", "ParentMessage"),
+		cmpopts.IgnoreFields(resolver.Method{}, "Service", "Desc"),
+		cmpopts.IgnoreFields(resolver.Message{}, "File", "ParentMessage", "Desc"),
 		cmpopts.IgnoreFields(resolver.Enum{}, "File", "Message.Rule"),
 		cmpopts.IgnoreFields(resolver.EnumValue{}, "Enum"),
 		cmpopts.IgnoreFields(resolver.EnumValueAlias{}, "EnumAlias"),
@@ -31,7 +31,7 @@ func ResolverCmpOpts() []cmp.Option {
 		cmpopts.IgnoreFields(resolver.AutoBindField{}, "VariableDefinition"),
 		cmpopts.IgnoreFields(resolver.Type{}, "Message.Rule", "Enum.Rule", "OneofField"),
 		cmpopts.IgnoreFields(resolver.Oneof{}, "Message"),
-		cmpopts.IgnoreFields(resolver.Field{}, "Message", "Oneof.Message", "Oneof.Fields"),
+		cmpopts.IgnoreFields(resolver.Field{}, "Message", "Oneof.Message", "Oneof.Fields", "Desc"),
 		cmpopts.IgnoreFields(resolver.Value{}, "CEL"),
 		cmpopts.IgnoreFields(resolver.CELValue{}, "CheckedExpr"),
 		cmpopts.IgnoreFields(resolver.MessageExpr{}, "Message.Rule", "Message.Fields"),

diff --git a/lsp/server/completion_test.go b/lsp/server/completion_test.go
@@ -24,8 +24,8 @@ func TestCompletion(t *testing.T) {
 	completer := server.NewCompleter(compiler.New(), slog.New(slog.NewJSONHandler(os.Stdout, nil)))
 	t.Run("method", func(t *testing.T) {
 		// resolver.method value position of Post in service.proto file
-		_, candidates, err := completer.Completion(ctx, nil, path, file, source.Position{
-			Line: 39,
+		_, candidates, err := completer.Completion(ctx, []string{"testdata"}, path, file, source.Position{
+			Line: 40,
 			Col:  19,
 		})
 		if err != nil {
@@ -44,8 +44,8 @@ func TestCompletion(t *testing.T) {
 
 	t.Run("request.field", func(t *testing.T) {
 		// resolver.request.field value position of Post in service.proto file
-		_, candidates, err := completer.Completion(ctx, nil, path, file, source.Position{
-			Line: 40,
+		_, candidates, err := completer.Completion(ctx, []string{"testdata"}, path, file, source.Position{
+			Line: 41,
 			Col:  28,
 		})
 		if err != nil {
@@ -61,8 +61,8 @@ func TestCompletion(t *testing.T) {
 
 	t.Run("request.by", func(t *testing.T) {
 		// resolver.request.by value position os Post in service.proto file
-		_, candidates, err := completer.Completion(ctx, nil, path, file, source.Position{
-			Line: 40,
+		_, candidates, err := completer.Completion(ctx, []string{"testdata"}, path, file, source.Position{
+			Line: 41,
 			Col:  38,
 		})
 		if err != nil {
@@ -78,8 +78,8 @@ func TestCompletion(t *testing.T) {
 
 	t.Run("filter response", func(t *testing.T) {
 		// resolver.response.field value position of Post in service.proto file
-		_, candidates, err := completer.Completion(ctx, nil, path, file, source.Position{
-			Line: 43,
+		_, candidates, err := completer.Completion(ctx, []string{"testdata"}, path, file, source.Position{
+			Line: 44,
 			Col:  28,
 		})
 		if err != nil {
@@ -96,8 +96,8 @@ func TestCompletion(t *testing.T) {
 
 	t.Run("message", func(t *testing.T) {
 		// def[2].message value position of Post in service.proto file
-		_, candidates, err := completer.Completion(ctx, nil, path, file, source.Position{
-			Line: 47,
+		_, candidates, err := completer.Completion(ctx, []string{"testdata"}, path, file, source.Position{
+			Line: 48,
 			Col:  17,
 		})
 		if err != nil {

diff --git a/lsp/server/definition.go b/lsp/server/definition.go
@@ -52,7 +52,7 @@ func (h *Handler) definitionWithLink(ctx context.Context, params *protocol.Defin
 		return nil, nil
 	}
 	h.logger.Info("node", slog.String("text", nodeInfo.RawText()))
-	protoFiles, err := h.compiler.Compile(ctx, file, compiler.ImportPathOption(h.importPaths...))
+	protoFiles, err := h.compiler.Compile(ctx, file, compiler.ImportPathOption(h.importPaths...), compiler.ImportRuleOption())
 	if err != nil {
 		return nil, err
 	}

diff --git a/lsp/server/handler_test.go b/lsp/server/handler_test.go
@@ -110,7 +110,7 @@ func TestHandler_DidChange(t *testing.T) {
 			)
 			client := protocol.ClientDispatcher(conn, zap.NewNop())
 
-			handler := server.NewHandler(client, &bytes.Buffer{}, nil)
+			handler := server.NewHandler(client, &bytes.Buffer{}, []string{"testdata"})
 
 			err := handler.DidChange(context.Background(), tc.params)
 			if err != nil {
@@ -175,7 +175,7 @@ func TestHandler_Definition(t *testing.T) {
 						URI: mustTestdataAbs(t, "testdata/service.proto"),
 					},
 					Position: protocol.Position{
-						Line:      24,
+						Line:      25,
 						Character: 15,
 					},
 				},
@@ -184,8 +184,8 @@ func TestHandler_Definition(t *testing.T) {
 				{
 					URI: mustTestdataAbs(t, "testdata/service.proto"),
 					Range: protocol.Range{
-						Start: protocol.Position{Line: 32, Character: 8},
-						End:   protocol.Position{Line: 32, Character: 12},
+						Start: protocol.Position{Line: 33, Character: 8},
+						End:   protocol.Position{Line: 33, Character: 12},
 					},
 				},
 			},
@@ -198,7 +198,7 @@ func TestHandler_Definition(t *testing.T) {
 						URI: mustTestdataAbs(t, "testdata/service.proto"),
 					},
 					Position: protocol.Position{
-						Line:      38,
+						Line:      39,
 						Character: 19,
 					},
 				},
@@ -221,7 +221,7 @@ func TestHandler_Definition(t *testing.T) {
 						URI: mustTestdataAbs(t, "testdata/service.proto"),
 					},
 					Position: protocol.Position{
-						Line:      19,
+						Line:      20,
 						Character: 0,
 					},
 				},

diff --git a/lsp/server/testdata/service.proto b/lsp/server/testdata/service.proto
@@ -3,10 +3,11 @@ syntax = "proto3";
 package federation;
 
 import "federation.proto";
-import "post.proto";
-import "user.proto";
 
 option go_package = "example/federation;federation";
+option (grpc.federation.file) = {
+  import: ["post.proto", "user.proto"]
+};
 
 service FederationService {
   option (grpc.federation.service) = {};

diff --git a/resolver/context.go b/resolver/context.go
@@ -129,7 +129,6 @@ func (c *context) error() error {
 	return c.errorBuilder.build()
 }
 
-//nolint:unused
 func (c *context) addWarning(w *Warning) {
 	c.allWarnings.warnings = append(c.allWarnings.warnings, w)
 }

diff --git a/resolver/error.go b/resolver/error.go
@@ -61,6 +61,14 @@ func ErrWithLocation(msg string, loc *source.Location) *LocationError {
 	}
 }
 
+// WarnWithLocation creates Warnings instance from message and location.
+func WarnWithLocation(msg string, loc *source.Location) *Warning {
+	return &Warning{
+		Location: loc,
+		Message:  msg,
+	}
+}
+
 type errorBuilder struct {
 	errs []error
 }