[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>

internal/engine/docgen.go

@@ -59,6 +59,9 @@ func Doc(src string) (string, error) {
 		}
 		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 @@ func genDoc(tmpdir, src, content string, isStdlib bool) (string, error) {
 		}
 	}
 	// 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 @@ func genDoc(tmpdir, src, content string, isStdlib bool) (string, error) {
 			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 @@ func getStarlark(packages map[string]fs.FS, m string) (string, error) {
 	} 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
 	}

internal/engine/docgen_test.go

@@ -17,6 +17,7 @@ package engine
 import (
 	"os"
 	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 	"testing"
@@ -56,6 +57,50 @@ func TestDoc(t *testing.T) {
 	}
 }
 
+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 @@ func TestDocTemplate(t *testing.T) {
 		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(t *testing.T) {
 	}
 }
 
+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.

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")

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")

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

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

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,
+)