Address Pascal's comments from fxr/365760
https://fuchsia-review.googlesource.com/c/fuchsia/+/365760
diff --git a/backend/library.go b/backend/library.go
index 5b2f7d8..a2ea51a 100644
--- a/backend/library.go
+++ b/backend/library.go
@@ -12,7 +12,6 @@
"os"
"path/filepath"
"regexp"
- "sort"
"strings"
"sync"
)
@@ -244,15 +243,13 @@
return args, nil
}
-const jsonDepIndentSize = 4
-
-// jsonDependencyGraph returns a JSON object encoded as a string representing
-// the dependency graph of m. Each key is a library name, and each value is
-// another object whose keys are its direct dependencies.
-func (m libraryMap) jsonDependencyGraph() (string, error) {
+// dependencyGraph returns a map representing the the dependency graph of m.
+// Each key is a library name, and each value is another map whose keys are its
+// direct dependencies.
+func (m libraryMap) dependencyGraph() (map[string]interface{}, error) {
// Make sure there are no cycles, otherwise this will never terminate.
if _, err := m.topologicalSort(); err != nil {
- return "", err
+ return nil, err
}
sources := map[library]struct{}{}
@@ -264,44 +261,20 @@
delete(sources, dep)
}
}
- var start []string
+ var start []library
for lib := range sources {
- start = append(start, string(lib))
+ start = append(start, lib)
}
- sort.Strings(start)
- var b strings.Builder
- indent := 0
- var gen func(libs []string)
- gen = func(libs []string) {
- b.WriteString("{\n")
- indent++
- for i, lib := range libs {
- b.WriteString(strings.Repeat(" ", indent*jsonDepIndentSize))
- b.WriteString(fmt.Sprintf(`"%s": `, lib))
- deps := m[library(lib)].deps
- if len(deps) == 0 {
- b.WriteString("{}")
- } else {
- var next []string
- for _, dep := range deps {
- next = append(next, string(dep))
- }
- sort.Strings(next)
- gen(next)
- }
- if i == len(libs)-1 {
- b.WriteString("\n")
- } else {
- b.WriteString(",\n")
- }
+ var gen func(libs []library) map[string]interface{}
+ gen = func(libs []library) map[string]interface{} {
+ g := map[string]interface{}{}
+ for _, lib := range libs {
+ g[string(lib)] = gen(m[library(lib)].deps)
}
- indent--
- b.WriteString(strings.Repeat(" ", indent*jsonDepIndentSize))
- b.WriteString("}")
+ return g
}
- gen(start)
- return b.String(), nil
+ return gen(start), nil
}
// topologicalSort runs topological sort on m, whose deps fields define a
diff --git a/backend/server.go b/backend/server.go
index a9b3934..2208cc0 100644
--- a/backend/server.go
+++ b/backend/server.go
@@ -7,6 +7,7 @@
import (
"bytes"
"context"
+ "encoding/json"
"fmt"
"io/ioutil"
"log"
@@ -79,8 +80,6 @@
OutputMode Mode `json:"outputMode"`
// Options for configuring the conversion.
Options Options `json:"options"`
- // Request context (not serialized).
- ctx context.Context
}
// A Response contains the result of converting to an output mode.
@@ -161,34 +160,28 @@
Bytes: {},
}
-func outputModes(input Mode) modeSet {
- switch input {
- case FIDL:
- return modeSet{
- FIDL: {},
- JSON: {},
- C: {},
- LLCPP: {},
- HLCPP: {},
- Rust: {},
- Go: {},
- Dart: {},
- }
- case FIDLText:
- return modeSet{
- FIDLText: {},
- Bytes: {},
- BytesPlus: {},
- }
- case Bytes:
- return modeSet{
- Bytes: {},
- Diff: {},
- FIDLText: {},
- BytesPlus: {},
- }
- }
- return modeSet{}
+var outputModes = map[Mode]modeSet{
+ FIDL: {
+ FIDL: {},
+ JSON: {},
+ C: {},
+ LLCPP: {},
+ HLCPP: {},
+ Rust: {},
+ Go: {},
+ Dart: {},
+ },
+ FIDLText: {
+ FIDLText: {},
+ Bytes: {},
+ BytesPlus: {},
+ },
+ Bytes: {
+ Bytes: {},
+ Diff: {},
+ FIDLText: {},
+ BytesPlus: {},
+ },
}
// Validate validates a request. Once validated, any errors in processing the
@@ -197,7 +190,7 @@
if err := r.InputMode.validate(inputModes); err != nil {
return err
}
- if err := r.OutputMode.validate(outputModes(r.InputMode)); err != nil {
+ if err := r.OutputMode.validate(outputModes[r.InputMode]); err != nil {
return err
}
@@ -254,25 +247,24 @@
// Serve processes a request and serves a response. It returns an error if the
// request could not be processed for some reason (e.g., an IO failure).
func (s *Server) Serve(ctx context.Context, r *Request) (Response, error) {
- r.ctx = ctx
switch r.InputMode {
case FIDL:
- return s.handleFIDL(r)
+ return s.handleFIDL(ctx, r)
case FIDLText:
- return s.handleFIDLText(r)
+ return s.handleFIDLText(ctx, r)
case Bytes:
- return s.handleBytes(r)
+ return s.handleBytes(ctx, r)
}
return internalErrorf("invalid input mode: %q", r.InputMode)
}
-func (s *Server) handleFIDL(r *Request) (Response, error) {
+func (s *Server) handleFIDL(ctx context.Context, r *Request) (Response, error) {
temp, err := newTempDir()
if err != nil {
return internalError(err)
}
defer os.RemoveAll(temp.path)
- res, err := s.handleFIDLWithTemp(r, temp)
+ res, err := s.handleFIDLWithTemp(ctx, r, temp)
if err != nil {
return res, err
}
@@ -287,7 +279,9 @@
return res, nil
}
-func (s *Server) handleFIDLWithTemp(r *Request, temp tempDir) (Response, error) {
+const jsonDepIndentSize = 4
+
+func (s *Server) handleFIDLWithTemp(ctx context.Context, r *Request, temp tempDir) (Response, error) {
fidl, err := temp.createFile("fidlbolt.fidl", r.Content)
if err != nil {
return internalError(err)
@@ -302,7 +296,7 @@
msg := fmt.Sprintf("%s:1:1: error: %s", fidl, err)
return Response{Ok: false, Content: msg, Annotations: anns}
}
- res := s.fidlc.run(r.ctx, append(args, fileArgs...)...)
+ res := s.fidlc.run(ctx, append(args, fileArgs...)...)
res.Annotations = anns
return res
}
@@ -317,30 +311,45 @@
switch r.OutputMode {
case FIDL:
if r.Options.Lint {
- if res := s.fidlLint.run(r.ctx, fidl); !res.Ok {
+ if res := s.fidlLint.run(ctx, fidl); !res.Ok {
return res, nil
}
+ return Response{Ok: true, Content: "// Lint passed!"}, nil
}
- return s.fidlFormat.run(r.ctx, fidl), nil
+ return s.fidlFormat.run(ctx, fidl), nil
case JSON:
switch r.Options.File {
case "ir":
return fidlc("--json", "/dev/stdout"), nil
case "schema":
- return s.fidlc.run(r.ctx, "--json-schema"), nil
+ return s.fidlc.run(ctx, "--json-schema"), nil
case "deps":
libs, anns := s.libraryAnalyzer.analyze(fidl, r.Content)
- graph, err := libs.jsonDependencyGraph()
+ graph, err := libs.dependencyGraph()
if err != nil {
msg := fmt.Sprintf("%s:1:1: error: %s", fidl, err)
return Response{Ok: false, Content: msg, Annotations: anns}, nil
}
- return Response{Ok: true, Content: graph, Annotations: anns}, nil
+ b, err := json.MarshalIndent(graph, "", strings.Repeat(" ", jsonDepIndentSize))
+ if err != nil {
+ msg := fmt.Sprintf("%s:1:1: error: %s", fidl, err)
+ return Response{Ok: false, Content: msg, Annotations: anns}, nil
+ }
+ return Response{Ok: true, Content: string(b), Annotations: anns}, nil
default:
return internalErrorf("invalid file: %q", r.Options.File)
}
case C:
- return fidlc("--c-"+r.Options.File, "/dev/stdout"), nil
+ switch r.Options.File {
+ case "header":
+ return fidlc("--c-header", "/dev/stdout"), nil
+ case "client":
+ return fidlc("--c-client", "/dev/stdout"), nil
+ case "server":
+ return fidlc("--c-server", "/dev/stdout"), nil
+ default:
+ return internalErrorf("invalid file: %q", r.Options.File)
+ }
}
jsonIR := fidl + ".json"
@@ -348,7 +357,7 @@
if !fidlcRes.Ok {
return fidlcRes, nil
}
- res, err := s.handleFIDLWithIR(r, temp, jsonIR)
+ res, err := s.handleFIDLWithIR(ctx, r, temp, jsonIR)
if err != nil {
return internalError(err)
}
@@ -356,11 +365,10 @@
return res, nil
}
-func (s *Server) handleFIDLWithIR(r *Request, temp tempDir, jsonIR string) (Response, error) {
+func (s *Server) handleFIDLWithIR(ctx context.Context, r *Request, temp tempDir, jsonIR string) (Response, error) {
switch r.OutputMode {
case HLCPP:
- if res := s.fidlgenHlcpp.run(r.ctx,
- "-generators", "cpp",
+ if res := s.fidlgenHlcpp.run(ctx,
"-json", jsonIR,
"-include-base", temp.path,
"-output-base", temp.join("impl"),
@@ -376,7 +384,7 @@
return internalErrorf("invalid file: %q", r.Options.File)
}
case LLCPP:
- if res := s.fidlgenLlcpp.run(r.ctx,
+ if res := s.fidlgenLlcpp.run(ctx,
"-json", jsonIR,
"-header", temp.join("impl.h"),
"-source", temp.join("impl.cc"),
@@ -392,7 +400,7 @@
return internalErrorf("invalid file: %q", r.Options.File)
}
case Rust:
- if res := s.fidlgenRust.run(r.ctx,
+ if res := s.fidlgenRust.run(ctx,
"-json", jsonIR,
"-output-filename", temp.join("impl.rs"),
); !res.Ok {
@@ -400,7 +408,7 @@
}
return temp.readFile("impl.rs")
case Go:
- if res := s.fidlgenGo.run(r.ctx,
+ if res := s.fidlgenGo.run(ctx,
"-json", jsonIR,
"-output-impl", temp.join("impl.go"),
); !res.Ok {
@@ -417,7 +425,7 @@
default:
return internalErrorf("invalid file: %q", r.Options.File)
}
- if res := s.fidlgenDart.run(r.ctx,
+ if res := s.fidlgenDart.run(ctx,
"-json", jsonIR,
"-output-"+name, temp.join("impl.dart"),
); !res.Ok {
@@ -428,11 +436,11 @@
return internalErrorf("invalid output mode: %q", r.OutputMode)
}
-func (s *Server) handleFIDLText(r *Request) (Response, error) {
+func (s *Server) handleFIDLText(ctx context.Context, r *Request) (Response, error) {
return Response{Ok: false, Content: "Not implemented"}, nil
}
-func (s *Server) handleBytes(r *Request) (Response, error) {
+func (s *Server) handleBytes(ctx context.Context, r *Request) (Response, error) {
switch r.OutputMode {
case Bytes:
b, pos, err := parseBytes(r.Content)
@@ -486,6 +494,8 @@
// run runs a program with arguments and produces a response. It stops the
// program and returns if ctx is cancelled.
+// TODO: Consider returning a string and error here, and letting the top-level
+// handlers produce responses instead.
func (p program) run(ctx context.Context, arg ...string) Response {
// TODO: use exec.CommandContext!
cmd := exec.Command(p.path, arg...)
@@ -540,10 +550,10 @@
if err != nil {
return "", err
}
+ defer f.Close()
if _, err = f.WriteString(content); err != nil {
return "", err
}
- f.Close()
return f.Name(), nil
}
@@ -556,8 +566,15 @@
return Response{Ok: true, Content: string(b)}, nil
}
-var fidlAnnotationRegexp = regexp.MustCompile(
- `(?m)^fidlbolt\.fidl:(\d+):(\d+).*?(info|error|warning):\s*(.*)$`)
+var fidlAnnotationRegexp = regexp.MustCompile(`` +
+ // Multi-line mode: ^ and $ match begin/end of line OR text.
+ `(?m)` +
+ `^fidlbolt\.fidl:(\d+):(\d+).*?` +
+ `(` +
+ string(Info) + `|` +
+ string(Warning) + `|` +
+ string(Error) + `)` +
+ `:\s*(.*)$`)
// parseFidlAnnotations parses a list of annotations from error output. It
// assumes that the output refers to a file called "fidlbolt.fidl".
