[engine] Fix crash in Doc()
While looking at vendoring support, I realized that a load() statement
without a shac.textproto would cause a crash.
Improve error messages and test coverage by a whopping 0.4%.
Change-Id: Iba84274452a75d1d760d99841d4646b191d6fadd
Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/913034
Reviewed-by: Oliver Newman <olivernewman@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
Fuchsia-Auto-Submit: Marc-Antoine Ruel <maruel@google.com>
diff --git a/internal/engine/docgen.go b/internal/engine/docgen.go
index 22bb510..2cb33a4 100644
--- a/internal/engine/docgen.go
+++ b/internal/engine/docgen.go
@@ -59,6 +59,9 @@
}
b, err := os.ReadFile(src)
if err != nil {
+ if errors.Is(err, fs.ErrNotExist) {
+ return "", fmt.Errorf("file %s not found", src)
+ }
return "", err
}
content = string(b)
@@ -90,6 +93,7 @@
}
}
// Load packages to get the exported symbols.
+ // TODO(maruel): Persist cache.
pkgMgr := PackageManager{Root: tmpdir}
var packages map[string]fs.FS
root := filepath.Dir(src)
@@ -100,12 +104,17 @@
if err = prototext.Unmarshal(b, &doc); err != nil {
return "", err
}
- // TODO(maruel): Only fetch the direct ones!
+ if err = doc.Validate(); err != nil {
+ return "", err
+ }
if packages, err = pkgMgr.RetrievePackages(context.Background(), root, &doc); err != nil {
return "", err
}
} else if !errors.Is(err, fs.ErrNotExist) {
return "", err
+ } else {
+ // Still allow local access even if no shac.textproto is present.
+ packages = map[string]fs.FS{"__main__": os.DirFS(root)}
}
}
d := m.Doc()
@@ -184,7 +193,14 @@
} else if strings.HasPrefix(m, "//") {
relpath = m[2:]
}
- d, err := packages[pkg].Open(relpath)
+ ref := packages[pkg]
+ if ref == nil {
+ return "", fmt.Errorf("package %s not found", pkg)
+ }
+ d, err := ref.Open(relpath)
+ if errors.Is(err, fs.ErrNotExist) {
+ return "", errors.New("file not found")
+ }
if err != nil {
return "", err
}
diff --git a/internal/engine/docgen_test.go b/internal/engine/docgen_test.go
index e412ff0..c349a77 100644
--- a/internal/engine/docgen_test.go
+++ b/internal/engine/docgen_test.go
@@ -17,6 +17,7 @@
import (
"os"
"path/filepath"
+ "runtime"
"strconv"
"strings"
"testing"
@@ -56,6 +57,50 @@
}
}
+func TestDoc_Error(t *testing.T) {
+ t.Parallel()
+ data := []struct {
+ path string
+ err string
+ }{
+ {
+ "file",
+ "invalid source file name, expecting .star suffix",
+ },
+ {
+ "@remote//file.star",
+ "todo: implement @module",
+ },
+ {
+ "/non_existent.star",
+ func() string {
+ if runtime.GOOS == "windows" {
+ p, err := filepath.Abs("/non_existent.star")
+ if err != nil {
+ t.Fatal(err)
+ }
+ return "file " + p + " not found"
+ }
+ return "file /non_existent.star not found"
+ }(),
+ },
+ }
+ for i, line := range data {
+ line := line
+ t.Run(strconv.Itoa(i), func(t *testing.T) {
+ t.Parallel()
+ d, err := Doc(line.path)
+ if err == nil {
+ t.Fatalf("expected error, got: %s", d)
+ }
+ got := err.Error()
+ if diff := cmp.Diff(line.err, got); diff != "" {
+ t.Fatalf("mismatch (-want +got):\n%s", diff)
+ }
+ })
+ }
+}
+
func TestDocTemplate(t *testing.T) {
t.Parallel()
data := []struct {
@@ -88,6 +133,7 @@
line := line
t.Run(strconv.Itoa(i), func(t *testing.T) {
t.Parallel()
+ // This test case cannot call load() because "main.star" is not an absolute path.
got, err := genDoc(t.TempDir(), "main.star", line.in, false)
if err != nil {
t.Fatal(err)
@@ -99,6 +145,66 @@
}
}
+func TestDocTemplate_Testdata_Err(t *testing.T) {
+ t.Parallel()
+ data := []struct {
+ path string
+ err string
+ }{
+ {
+ "testdata/docgen/err/missing_local/test.star",
+ `template: main:115:13: executing "main" at <Symbol "testdata/docgen/err/missing_local/test.star" "fn">: error calling Symbol: in testdata/docgen/err/missing_local/test.star: in //non_existent.star: file not found`,
+ },
+ {
+ "testdata/docgen/err/missing_pkg/test.star",
+ `template: main:115:13: executing "main" at <Symbol "testdata/docgen/err/missing_pkg/test.star" "fn">: error calling Symbol: in testdata/docgen/err/missing_pkg/test.star: in @remote//non_existent.star: package remote not found`,
+ },
+ {
+ "testdata/docgen/err/textproto_is_dir/test.star",
+ func() string {
+ if runtime.GOOS == "windows" {
+ return "read testdata\\docgen\\err\\textproto_is_dir\\shac.textproto: Incorrect function."
+ }
+ return "read testdata/docgen/err/textproto_is_dir/shac.textproto: is a directory"
+ }(),
+ },
+ }
+ for i, line := range data {
+ line := line
+ t.Run(strconv.Itoa(i), func(t *testing.T) {
+ in, err := os.ReadFile(line.path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ d, err := genDoc(filepath.Dir(line.path), line.path, string(in), false)
+ if err == nil {
+ t.Fatalf("expected error, got: %s", d)
+ }
+ got := err.Error()
+ if diff := cmp.Diff(line.err, got); diff != "" {
+ t.Fatalf("mismatch (-want +got):\n%s", diff)
+ }
+ })
+ }
+}
+
+func TestDocTemplateLoad_Load_Local(t *testing.T) {
+ t.Parallel()
+ f := "testdata/docgen/load_local/test.star"
+ in, err := os.ReadFile(f)
+ if err != nil {
+ t.Fatal(err)
+ }
+ got, err := genDoc(filepath.Dir(f), f, string(in), false)
+ if err != nil {
+ t.Fatal(err)
+ }
+ want := "# testdata/docgen/load_local/test.star\n\n## Table of contents\n\n- [fn](#fn)\n- [sym](#sym)\n\n## fn\n\nDo stuff.\n\n\n## sym\n\n\n\nFields:\n\n- fn\n\n## sym.fn\n\nDo stuff.\n\n"
+ if diff := cmp.Diff(want, got); diff != "" {
+ t.Fatalf("mismatch (-want +got):\n%s", diff)
+ }
+}
+
func TestDocgenGenerator(t *testing.T) {
// It's not really a unit test, it's more to document parts of what is
// available in the template engine.
diff --git a/internal/engine/testdata/docgen/err/missing_local/test.star b/internal/engine/testdata/docgen/err/missing_local/test.star
new file mode 100644
index 0000000..7e1b7d2
--- /dev/null
+++ b/internal/engine/testdata/docgen/err/missing_local/test.star
@@ -0,0 +1,15 @@
+# Copyright 2023 The Shac Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("//non_existent.star", fn = "fn")
diff --git a/internal/engine/testdata/docgen/err/missing_pkg/test.star b/internal/engine/testdata/docgen/err/missing_pkg/test.star
new file mode 100644
index 0000000..9af78ec
--- /dev/null
+++ b/internal/engine/testdata/docgen/err/missing_pkg/test.star
@@ -0,0 +1,15 @@
+# Copyright 2023 The Shac Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("@remote//non_existent.star", fn = "fn")
diff --git a/internal/engine/testdata/docgen/err/textproto_is_dir/shac.textproto/.gitignore b/internal/engine/testdata/docgen/err/textproto_is_dir/shac.textproto/.gitignore
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/internal/engine/testdata/docgen/err/textproto_is_dir/shac.textproto/.gitignore
diff --git a/internal/engine/testdata/docgen/err/textproto_is_dir/test.star b/internal/engine/testdata/docgen/err/textproto_is_dir/test.star
new file mode 100644
index 0000000..d5a9020
--- /dev/null
+++ b/internal/engine/testdata/docgen/err/textproto_is_dir/test.star
@@ -0,0 +1,15 @@
+# Copyright 2023 The Shac Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+pass
diff --git a/internal/engine/testdata/docgen/load_local/common.star b/internal/engine/testdata/docgen/load_local/common.star
new file mode 100644
index 0000000..1c34e4c
--- /dev/null
+++ b/internal/engine/testdata/docgen/load_local/common.star
@@ -0,0 +1,17 @@
+# Copyright 2023 The Shac Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+def fn():
+ """Do stuff."""
+ pass
diff --git a/internal/engine/testdata/docgen/load_local/test.star b/internal/engine/testdata/docgen/load_local/test.star
new file mode 100644
index 0000000..cb3cea9
--- /dev/null
+++ b/internal/engine/testdata/docgen/load_local/test.star
@@ -0,0 +1,19 @@
+# Copyright 2023 The Shac Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("//common.star", fn = "fn")
+
+sym = struct(
+ fn = fn,
+)
