internal/filetypes: better checking of filetype-dependent tags

Currently the filetypes logic allows arbitrary key-value pairs
to be specified with no checking. It also assumes that all
tags are equally valid to specify at the top level.

Enhance the logic to allow any given tag to specify additional
tags that can be specified when the tag is active, including key-value pairs.
As well as key-value pairs, it's useful to have boolean flags,
so add a field to `build.File` that allows those to be specified too.
A slightly different approach might be to continue to use
File.Tags but to have some mechanism in internal/filetypes
to indicate that a given tag is boolean, but that just seems to
add more complexity: the string values would then need to
be parsed in some way to be used.

This change means that key-value pairs are now checked properly
(for example, it's no longer possible to specify `lang=go` unless
the `code` tag is in play).

Also add some different strictness modes for jsonschema inside
`internal/filetypes` to check that the new mechanism works, but
we don't yet hook this up to the actual filetypes logic.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: Ica91c400d51db84c42ef4a48bd9bd0199a3bb33a
Dispatch-Trailer: {"type":"trybot","CL":1200901,"patchset":9,"ref":"refs/changes/01/1200901/9","targetBranch":"master"}

cmd/cue/cmd/testdata/script/unknown_filetype_attrval.txtar

@@ -1,9 +1,8 @@
 # Check that we cannot specify an arbitary unknown key-value pair
 # in a filetype.
-# TODO fix this test.
 
-exec cue export cue+foo=bar: x.cue
-# cmp stderr expect-stderr
+! exec cue export cue+foo=bar: x.cue
+cmp stderr expect-stderr
 -- x.cue --
 true
 -- expect-stderr --

cue/build/file.go

@@ -16,14 +16,24 @@ package build
 
 import "cuelang.org/go/cue/errors"
 
+// Note: the json tags in File correspond directly to names
+// used in the encoding/filetypes package, which unmarshals
+// results from CUE into a build.File.
+
 // A File represents a file that is part of the build process.
 type File struct {
 	Filename string `json:"filename"`
 
-	Encoding       Encoding          `json:"encoding,omitempty"`
-	Interpretation Interpretation    `json:"interpretation,omitempty"`
-	Form           Form              `json:"form,omitempty"`
-	Tags           map[string]string `json:"tags,omitempty"` // code=go
+	Encoding       Encoding       `json:"encoding,omitempty"`
+	Interpretation Interpretation `json:"interpretation,omitempty"`
+	Form           Form           `json:"form,omitempty"`
+	// Tags holds key-value pairs relating to the encoding
+	// conventions to use for the file.
+	Tags map[string]string `json:"tags,omitempty"` // e.g. code+lang=go
+
+	// BoolTags holds boolean-valued tags relating to the
+	// encoding conventions to use for the file.
+	BoolTags map[string]bool `json:"boolTags,omitempty"`
 
 	ExcludeReason errors.Error `json:"-"`
 	Source        interface{}  `json:"-"` // TODO: swap out with concrete type.

internal/filetypes/filetypes.go

@@ -15,7 +15,9 @@
 package filetypes
 
 import (
+	"fmt"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"cuelang.org/go/cue"
@@ -313,21 +315,57 @@ func parseType(scope string, mode Mode) (modeVal, fileVal cue.Value, _ error) {
 	modeVal = typesValue.LookupPath(cue.MakePath(cue.Str("modes"), cue.Str(mode.String())))
 	fileVal = modeVal.LookupPath(cue.MakePath(cue.Str("FileInfo")))
 
-	if scope != "" {
-		for _, tag := range strings.Split(scope, "+") {
-			tagName, tagVal, ok := strings.Cut(tag, "=")
-			if ok {
-				fileVal = fileVal.FillPath(cue.MakePath(cue.Str("tags"), cue.Str(tagName)), tagVal)
-			} else {
-				info := typesValue.LookupPath(cue.MakePath(cue.Str("tagInfo"), cue.Str(tag)))
-				if !info.Exists() {
-					return cue.Value{}, cue.Value{}, errors.Newf(token.NoPos, "unknown filetype %s", tag)
-				}
+	if scope == "" {
+		return modeVal, fileVal, nil
+	}
+	var otherTags []string
+	for _, tag := range strings.Split(scope, "+") {
+		tagName, _, ok := strings.Cut(tag, "=")
+		if ok {
+			otherTags = append(otherTags, tag)
+		} else {
+			info := typesValue.LookupPath(cue.MakePath(cue.Str("tagInfo"), cue.Str(tagName)))
+			if info.Exists() {
 				fileVal = fileVal.Unify(info)
+			} else {
+				// The tag might only be available when all the
+				// other tags have been evaluated.
+				otherTags = append(otherTags, tag)
 			}
 		}
 	}
-
+	if len(otherTags) == 0 {
+		return modeVal, fileVal, nil
+	}
+	// There are tags that aren't mentioned in tagInfo.
+	// They might still be valid, but just only valid within the file types that
+	// have been specified above, so look at the schema that we've got
+	// and see if it specifies any tags.
+	allowedTags := fileVal.LookupPath(cue.MakePath(cue.Str("tags")))
+	allowedBoolTags := fileVal.LookupPath(cue.MakePath(cue.Str("boolTags")))
+	for _, tag := range otherTags {
+		tagName, tagVal, hasValue := strings.Cut(tag, "=")
+		tagNamePath := cue.MakePath(cue.Str(tagName)).Optional()
+		tagSchema := allowedTags.LookupPath(tagNamePath)
+		if tagSchema.Exists() {
+			fileVal = fileVal.FillPath(cue.MakePath(cue.Str("tags"), cue.Str(tagName)), tagVal)
+			continue
+		}
+		if !allowedBoolTags.LookupPath(tagNamePath).Exists() {
+			return cue.Value{}, cue.Value{}, errors.Newf(token.NoPos, "unknown filetype %s", tagName)
+		}
+		tagValBool := true
+		if hasValue {
+			// It's a boolean tag and an explicit value has been specified.
+			// Allow the usual boolean string values.
+			t, err := strconv.ParseBool(tagVal)
+			if err != nil {
+				return cue.Value{}, cue.Value{}, fmt.Errorf("invalid boolean value for tag %q", tagName)
+			}
+			tagValBool = t
+		}
+		fileVal = fileVal.FillPath(cue.MakePath(cue.Str("boolTags"), cue.Str(tagName)), tagValBool)
+	}
 	return modeVal, fileVal, nil
 }
 

internal/filetypes/filetypes_test.go

@@ -179,6 +179,11 @@ func TestFromFile(t *testing.T) {
 				Encoding:       build.JSON,
 				Interpretation: "jsonschema",
 				Form:           build.Schema,
+				BoolTags: map[string]bool{
+					"strict":         false,
+					"strictFeatures": false,
+					"strictKeywords": false,
+				},
 			},
 			Definitions:  true,
 			Data:         true,
@@ -324,8 +329,8 @@ func TestParseFile(t *testing.T) {
 		out: &build.File{
 			Filename:       "-",
 			Encoding:       build.JSON,
-			Form:           build.Schema,
 			Interpretation: build.OpenAPI,
+			Form:           build.Schema,
 		},
 	}, {
 		in: "cue:file.json",
@@ -347,6 +352,9 @@ func TestParseFile(t *testing.T) {
 			Encoding: build.Code,
 			Tags:     map[string]string{"lang": "js"},
 		},
+	}, {
+		in:  "json+lang=js:foo.x",
+		out: `unknown filetype lang`,
 	}, {
 		in:  "foo:file.bar",
 		out: `unknown filetype foo`,
@@ -409,6 +417,26 @@ func TestParseArgs(t *testing.T) {
 				Encoding:       build.JSON,
 				Form:           build.Schema,
 				Interpretation: "jsonschema",
+				BoolTags: map[string]bool{
+					"strict":         false,
+					"strictFeatures": false,
+					"strictKeywords": false,
+				},
+			},
+		},
+	}, {
+		in: "jsonschema+strict: bar.schema",
+		out: []*build.File{
+			{
+				Filename:       "bar.schema",
+				Encoding:       "json",
+				Interpretation: "jsonschema",
+				Form:           build.Schema,
+				BoolTags: map[string]bool{
+					"strict":         true,
+					"strictFeatures": true,
+					"strictKeywords": true,
+				},
 			},
 		},
 	}, {

internal/filetypes/types.cue

@@ -28,6 +28,7 @@ package build
 	form?:           #Form
 	// Note: tags includes values for non-boolean tags only.
 	tags?: [string]: string
+	boolTags?: [string]: bool
 }
 
 // Default is the file used for stdin and stdout. The settings depend
@@ -293,6 +294,11 @@ interpretations: auto: forms.schema
 interpretations: jsonschema: {
 	forms.schema
 	encoding: *"json" | _
+	boolTags: {
+		strict:         *false | bool
+		strictKeywords: *strict | bool
+		strictFeatures: *strict | bool
+	}
 }
 
 interpretations: openapi: {