Repository: openllb/hlb
Pull request #230 (2021-01-26 01:19:36)

Unparse BuiltinDecl correctly
This fixes two problems:

1) `(*SpanError).Pretty` would panic if there were no spans for the
file. This seemed to be an oversight, because the length of `spans` was
checked just below.

2) `Module`'s implementation of `Unparse` was used for `BuiltinDecl`.
This meant that unrecognized builtin errors would end up printing the
whole builtin module instead of just the name of the builtin.


Patch:
diff --git a/checker/builtin.go b/checker/builtin.go
index 0001c41..1e41100 100644
--- a/checker/builtin.go
+++ b/checker/builtin.go
@@ -24,6 +24,7 @@ func NewBuiltinScope(builtins builtin.BuiltinLookup) *parser.Scope {
 					Ident: fun.Name,
 					Node: &parser.BuiltinDecl{
 						Module:         builtin.Module,
+						Name:           fun.Name.String(),
 						FuncDeclByKind: make(map[parser.Kind]*parser.FuncDecl),
 						CallableByKind: make(map[parser.Kind]parser.Callable),
 					},
diff --git a/diagnostic/span.go b/diagnostic/span.go
index 570f160..0df75ed 100644
--- a/diagnostic/span.go
+++ b/diagnostic/span.go
@@ -95,6 +95,11 @@ func (se *SpanError) Pretty(ctx context.Context, opts ...PrettyOption) string {
 
 		// Sort spans in the same module by line number.
 		spans := spansByFilename[filename]
+
+		if len(spans) == 0 {
+			continue
+		}
+
 		sort.SliceStable(spans, func(i, j int) bool {
 			return spans[i].Start.Line < spans[j].Start.Line
 		})
@@ -112,14 +117,12 @@ func (se *SpanError) Pretty(ctx context.Context, opts ...PrettyOption) string {
 			sections []string
 			prevLn   int
 		)
-		if len(spans) > 0 {
-			// Initialize the previous line number, this will be updated after every
-			// span to determine how the next span render should join with the previous.
-			// (i.e. if there's a gap or there's overlap).
-			prevLn = spans[0].Start.Line - info.NumContext - 1
-			if prevLn < 0 {
-				prevLn = 0
-			}
+		// Initialize the previous line number, this will be updated after every
+		// span to determine how the next span render should join with the previous.
+		// (i.e. if there's a gap or there's overlap).
+		prevLn = spans[0].Start.Line - info.NumContext - 1
+		if prevLn < 0 {
+			prevLn = 0
 		}
 
 		for i, span := range spans {
diff --git a/parser/cst.go b/parser/cst.go
index 8e5eb36..ba1bbbe 100644
--- a/parser/cst.go
+++ b/parser/cst.go
@@ -214,6 +214,7 @@ type Export struct {
 // Special type checking rules apply to builtins.
 type BuiltinDecl struct {
 	*Module
+	Name           string
 	FuncDeclByKind map[Kind]*FuncDecl
 	CallableByKind map[Kind]Callable
 }
diff --git a/parser/unparse.go b/parser/unparse.go
index bd66f08..f96af31 100644
--- a/parser/unparse.go
+++ b/parser/unparse.go
@@ -34,6 +34,12 @@ func WithNoStmtEnd() UnparseOption {
 	}
 }
 
+func (bd *BuiltinDecl) String() string { return bd.Unparse() }
+
+func (bd *BuiltinDecl) Unparse(opts ...UnparseOption) string {
+	return bd.Name
+}
+
 func (m *Module) String() string { return m.Unparse() }
 
 func (m *Module) Unparse(opts ...UnparseOption) string {


Test patch:
diff --git a/codegen/codegen_test.go b/codegen/codegen_test.go
index 80ceac5..c402fd5 100644
--- a/codegen/codegen_test.go
+++ b/codegen/codegen_test.go
@@ -905,3 +905,57 @@ func TestCodeGen(t *testing.T) {
 		})
 	}
 }
+
+func TestCodegenError(t *testing.T) {
+	t.Parallel()
+
+	type errorTestCase struct {
+		name          string
+		targets       []string
+		input         string
+		expectedError string
+	}
+
+	for _, tc := range []errorTestCase{
+		{
+			"invalid builtin",
+			[]string{"default"},
+			`
+			fs default() {
+				bug includePatterns("*.go")
+			}
+			fs bug(option::local pattern) {
+				local "." with pattern
+			}
+		`,
+			"<stdin>:3:9: unrecognized builtin `includePatterns`",
+		},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := diagnostic.WithSources(context.Background(), builtin.Sources())
+			mod, err := parser.Parse(ctx, strings.NewReader(tc.input))
+			require.NoError(t, err, "unexpected parse error")
+
+			err = checker.SemanticPass(mod)
+			require.NoError(t, err, tc.name)
+
+			_ = linter.Lint(ctx, mod)
+
+			err = checker.Check(mod)
+			require.NoError(t, err, tc.name)
+
+			var targets []codegen.Target
+			for _, target := range tc.targets {
+				targets = append(targets, codegen.Target{Name: target})
+			}
+
+			cg, err := codegen.New(nil)
+			require.NoError(t, err, tc.name)
+
+			ctx = codegen.WithSessionID(ctx, identity.NewID())
+			_, err = cg.Generate(ctx, mod, targets)
+			require.EqualError(t, err, tc.expectedError)
+		})
+	}
+}