Merge pull request authzed#2207 from authzed/implement-simpler-import-syntax

Implement simpler import syntax

Author: tstirrat15

pkg/composableschemadsl/compiler/importer-test/circular-import/root.zed

@@ -1,4 +1,4 @@
-from .subjects import user
+import "subjects.zed"
 
 definition resource {
   relation viewer: user

pkg/composableschemadsl/compiler/importer-test/circular-import/subjects.zed

@@ -1,3 +1,3 @@
-from .user import user
+import "user.zed"
 
 definition persona {}

pkg/composableschemadsl/compiler/importer-test/circular-import/user.zed

@@ -1,3 +1,3 @@
-from .subjects import persona
+import "subjects.zed"
 
 definition user {}

pkg/composableschemadsl/compiler/importer-test/diamond-shaped/left.zed

@@ -1 +1 @@
-from .subjects import user
+import "subjects.zed"

pkg/composableschemadsl/compiler/importer-test/diamond-shaped/right.zed

@@ -1 +1 @@
-from .subjects import persona
+import "subjects.zed"

pkg/composableschemadsl/compiler/importer-test/diamond-shaped/root.zed

@@ -1,5 +1,5 @@
-from .left import user
-from .right import persona
+import "left.zed"
+import "right.zed"
 
 definition resource {
   relation viewer: user

pkg/composableschemadsl/compiler/importer-test/escape-attempt/root.zed

@@ -0,0 +1 @@
+import "../user.zed"

pkg/composableschemadsl/compiler/importer-test/nested-local-with-hop/root.zed

@@ -1,4 +1,4 @@
-from .transitive.transitive import user
+import "transitive/transitive.zed"
 
 definition resource {
   relation viewer: user

pkg/composableschemadsl/compiler/importer-test/nested-local-with-hop/transitive/transitive.zed

@@ -1 +1 @@
-from .user.user import user
+import "user/user.zed"

pkg/composableschemadsl/compiler/importer-test/nested-local/root.zed

@@ -1,4 +1,4 @@
-from .user.user import user
+import "user/user.zed"
 
 definition resource {
   relation viewer: user

pkg/composableschemadsl/compiler/importer-test/nested-two-layer-local/root.zed

@@ -1,4 +1,4 @@
-from .definitions.user.user import user
+import "definitions/user/user.zed"
 
 definition resource {
   relation viewer: user

pkg/composableschemadsl/compiler/importer-test/simple-local-with-hop/indirection.zed

@@ -1 +1 @@
-from .user import user
+import "user.zed"

pkg/composableschemadsl/compiler/importer-test/simple-local-with-hop/root.zed

@@ -1,4 +1,4 @@
-from .indirection import user
+import "indirection.zed"
 
 definition resource {
   relation viewer: user

pkg/composableschemadsl/compiler/importer-test/simple-local/root.zed

@@ -1,4 +1,4 @@
-from .user import user
+import "user.zed"
 
 definition resource {
   relation viewer: user

pkg/composableschemadsl/compiler/importer.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"strings"
 
 	"github.com/rs/zerolog/log"
 
@@ -13,23 +14,23 @@ import (
 )
 
 type importContext struct {
-	pathSegments         []string
+	path                 string
 	sourceFolder         string
 	names                *mapz.Set[string]
 	locallyVisitedFiles  *mapz.Set[string]
 	globallyVisitedFiles *mapz.Set[string]
 }
 
-const SchemaFileSuffix = ".zed"
-
 type CircularImportError struct {
 	error
 	filePath string
 }
 
 func importFile(importContext importContext) (*CompiledSchema, error) {
-	relativeFilepath := constructFilePath(importContext.pathSegments)
-	filePath := path.Join(importContext.sourceFolder, relativeFilepath)
+	if err := validateFilepath(importContext.path); err != nil {
+		return nil, err
+	}
+	filePath := path.Join(importContext.sourceFolder, importContext.path)
 
 	newSourceFolder := filepath.Dir(filePath)
 
@@ -84,6 +85,18 @@ func importFile(importContext importContext) (*CompiledSchema, error) {
 	)
 }
 
-func constructFilePath(segments []string) string {
-	return path.Join(segments...) + SchemaFileSuffix
+// Take a filepath and ensure that it's local to the current context.
+func validateFilepath(path string) error {
+	if strings.Contains(path, "..") {
+		return fmt.Errorf("path %s contains '..'; paths must stay within their directory and this is likely an error", path)
+	}
+	// NOTE: This is slightly overly restrictive; it should theoretically be possible
+	// to take a given filepath and figure out whether it's local to the context where
+	// the compiler is being invoked, rather than whether it's local to the source
+	// folder of the current context. The assumption is that that won't matter
+	// right now, and we can fix it if we need to.
+	if !filepath.IsLocal(path) {
+		return fmt.Errorf("import path %s does not stay within its folder", path)
+	}
+	return nil
 }

pkg/composableschemadsl/compiler/importer_test.go

@@ -110,3 +110,23 @@ func TestImportCycleCausesError(t *testing.T) {
 
 	require.ErrorContains(t, err, "circular import")
 }
+
+func TestEscapeAttemptCausesError(t *testing.T) {
+	t.Parallel()
+
+	workingDir, err := os.Getwd()
+	require.NoError(t, err)
+	test := importerTest{"", "escape-attempt"}
+
+	sourceFolder := path.Join(workingDir, test.relativePath())
+
+	inputSchema := test.input()
+
+	_, err = compiler.Compile(compiler.InputSchema{
+		Source:       input.Source("schema"),
+		SchemaString: inputSchema,
+	}, compiler.AllowUnprefixedObjectType(),
+		compiler.SourceFolder(sourceFolder))
+
+	require.ErrorContains(t, err, "must stay within")
+}

pkg/composableschemadsl/compiler/translator.go

@@ -697,23 +697,14 @@ func addWithCaveats(tctx translationContext, typeRefNode *dslNode, ref *core.All
 }
 
 func translateImport(tctx translationContext, importNode *dslNode, names *mapz.Set[string]) (*CompiledSchema, error) {
-	// NOTE: this function currently just grabs everything that's in the target file.
-	// TODO: only grab the requested definitions
-	pathNodes := importNode.List(dslshape.NodeImportPredicatePathSegment)
-	pathSegments := make([]string, 0, len(pathNodes))
-
-	// Get the filepath segments out of the AST nodes
-	for _, pathSegmentNode := range pathNodes {
-		segment, err := pathSegmentNode.GetString(dslshape.NodeIdentiferPredicateValue)
-		if err != nil {
-			return nil, err
-		}
-		pathSegments = append(pathSegments, segment)
+	path, err := importNode.GetString(dslshape.NodeImportPredicatePath)
+	if err != nil {
+		return nil, err
 	}
 
 	compiledSchema, err := importFile(importContext{
 		names:                names,
-		pathSegments:         pathSegments,
+		path:                 path,
 		sourceFolder:         tctx.sourceFolder,
 		globallyVisitedFiles: tctx.globallyVisitedFiles,
 		locallyVisitedFiles:  tctx.locallyVisitedFiles,

pkg/composableschemadsl/dslshape/dslshape.go

@@ -194,8 +194,5 @@ const (
 	//
 	// NodeTypeImport
 	//
-	// TODO: still need to figure out what form this should take - full path? relative path?
-	NodeImportPredicateSource         = "import-source"
-	NodeImportPredicatePathSegment    = "path-segment"
-	NodeImportPredicateDefinitionName = "imported-definition"
+	NodeImportPredicatePath = "import-path"
 )

pkg/composableschemadsl/lexer/lex_def.go

@@ -78,7 +78,6 @@ var keywords = map[string]struct{}{
 	"permission": {},
 	"nil":        {},
 	"with":       {},
-	"from":       {},
 	"import":     {},
 	"all":        {},
 	"any":        {},

pkg/composableschemadsl/lexer/lex_test.go

@@ -62,7 +62,6 @@ var lexerTests = []lexerTest{
 	{"keyword", "permission", []Lexeme{{TokenTypeKeyword, 0, "permission", ""}, tEOF}},
 	{"keyword", "nil", []Lexeme{{TokenTypeKeyword, 0, "nil", ""}, tEOF}},
 	{"keyword", "with", []Lexeme{{TokenTypeKeyword, 0, "with", ""}, tEOF}},
-	{"keyword", "from", []Lexeme{{TokenTypeKeyword, 0, "from", ""}, tEOF}},
 	{"keyword", "import", []Lexeme{{TokenTypeKeyword, 0, "import", ""}, tEOF}},
 	{"keyword", "all", []Lexeme{{TokenTypeKeyword, 0, "all", ""}, tEOF}},
 	{"keyword", "nil", []Lexeme{{TokenTypeKeyword, 0, "nil", ""}, tEOF}},

pkg/composableschemadsl/parser/parser.go

@@ -62,7 +62,7 @@ Loop:
 		case p.isKeyword("caveat"):
 			rootNode.Connect(dslshape.NodePredicateChild, p.consumeCaveat())
 
-		case p.isKeyword("from"):
+		case p.isKeyword("import"):
 			rootNode.Connect(dslshape.NodePredicateChild, p.consumeImport())
 
 		default:
@@ -586,20 +586,6 @@ func (p *sourceParser) tryConsumeIdentifierLiteral() (AstNode, bool) {
 	return identNode, true
 }
 
-// consumeIdentifierLiteral is similar to the above, but attempts and errors
-// rather than checking the token type beforehand
-func (p *sourceParser) consumeIdentifierLiteral() (AstNode, bool) {
-	identNode := p.startNode(dslshape.NodeTypeIdentifier)
-	defer p.mustFinishNode()
-
-	identifier, ok := p.consumeIdentifier()
-	if !ok {
-		return identNode, false
-	}
-	identNode.MustDecorate(dslshape.NodeIdentiferPredicateValue, identifier)
-	return identNode, true
-}
-
 func (p *sourceParser) tryConsumeNilExpression() (AstNode, bool) {
 	if !p.isKeyword("nil") {
 		return nil, false
@@ -615,53 +601,17 @@ func (p *sourceParser) consumeImport() AstNode {
 	importNode := p.startNode(dslshape.NodeTypeImport)
 	defer p.mustFinishNode()
 
-	// from ...
+	// import ...
 	// NOTE: error handling isn't necessary here because this function is only
-	// invoked if the `from` keyword is found in the function above.
-	p.consumeKeyword("from")
-
-	// Consume alternating periods and identifiers
-	for {
-		if _, ok := p.consume(lexer.TokenTypePeriod); !ok {
-			return importNode
-		}
-
-		segmentNode, ok := p.consumeIdentifierLiteral()
-		// We connect the node so that the error information is retained, then break the loop
-		// so that we aren't continuing to attempt to consume.
-		importNode.Connect(dslshape.NodeImportPredicatePathSegment, segmentNode)
-		if !ok {
-			break
-		}
-
-		if !p.isToken(lexer.TokenTypePeriod) {
-			// If we don't have a period as our next token, we move
-			// to the next step of parsing.
-			break
-		}
-	}
+	// invoked if the `import` keyword is found in the function above.
+	p.consumeKeyword("import")
 
-	if ok := p.consumeKeyword("import"); !ok {
+	importPath, ok := p.consumeStringLiteral()
+	if !ok {
 		return importNode
 	}
 
-	// Consume alternating identifiers and commas until we reach the end of the import statement
-	for {
-		definitionNode, ok := p.consumeIdentifierLiteral()
-		// We connect the node so that the error information is retained, then break the loop
-		// so that we aren't continuing to attempt to consume.
-		importNode.Connect(dslshape.NodeImportPredicateDefinitionName, definitionNode)
-		if !ok {
-			break
-		}
-
-		if _, ok := p.tryConsumeStatementTerminator(); ok {
-			break
-		}
-		if _, ok := p.consume(lexer.TokenTypeComma); !ok {
-			return importNode
-		}
-	}
+	importNode.MustDecorate(dslshape.NodeImportPredicatePath, importPath)
 
 	return importNode
 }

pkg/composableschemadsl/parser/parser_impl.go

@@ -198,6 +198,37 @@ func (p *sourceParser) tryConsumeKeyword(keyword string) bool {
 	return true
 }
 
+// consumeString consumes a string token and returns the unwrapped string or adds an error node.
+func (p *sourceParser) consumeStringLiteral() (string, bool) {
+	consumedString, ok := p.tryConsumeStringLiteral()
+	if !ok {
+		p.emitErrorf("Expected quote-delimited string, found token %v", p.currentToken.Kind)
+		return "", false
+	}
+	return consumedString, true
+}
+
+// tryConsumeString attempts to consume an expected string token and return the unwrapped string.
+func (p *sourceParser) tryConsumeStringLiteral() (string, bool) {
+	wrappedStringToken, ok := p.tryConsume(lexer.TokenTypeString)
+	if !ok {
+		return "", false
+	}
+	wrappedString := wrappedStringToken.Value
+
+	// NOTE: We can't just trim here, because a user may use a combination of
+	// single and double quotes to escape.
+	// If we have a string wrapped in singlequotes (singular or plural),
+	// strip those specifically.
+	if strings.Index(wrappedString, `'`) == 0 {
+		return strings.Trim(wrappedString, `'`), true
+	}
+
+	// Else strip doublequotes, because the set of string delimiters is limited
+	// by the lexer.
+	return strings.Trim(wrappedString, `"`), true
+}
+
 // consumeKeywords consumes any of a set of keywords or adds an error node
 func (p *sourceParser) consumeKeywords(keywords ...string) (string, bool) {
 	keyword, ok := p.tryConsumeKeywords(keywords...)

pkg/composableschemadsl/parser/parser_test.go

@@ -123,12 +123,11 @@ func TestParser(t *testing.T) {
 		{"arrow illegal function test", "arrowillegalfunc"},
 		{"caveat with keyword parameter test", "caveatwithkeywordparam"},
 		{"local imports test", "localimport"},
+		{"local imports with singlequotes on import test", "localimport_with_singlequotes"},
+		{"local imports with quotes within quotes on import test", "localimport_with_quotes_in_quotes"},
+		{"local imports with unterminated string on import test", "localimport_with_unterminated_string"},
+		{"local imports with mismatched quotes on import test", "localimport_with_mismatched_quotes"},
 		{"local imports with keyword in import path test", "localimport_import_path_with_keyword"},
-		{"local imports with keyword in identifiers test", "localimport_keyword_in_identifiers"},
-		{"local imports with malformed identifiers set test", "localimport_malformed_identifier_set"},
-		{"local imports with malformed import path test", "localimport_malformed_import_path"},
-		{"local imports with path missing leading period test", "localimport_path_missing_leading_period"},
-		{"local imports with typo in import separator test", "localimport_typo_in_import_separator"},
 	}
 
 	for _, test := range parserTests {

pkg/composableschemadsl/parser/tests/localimport.zed

@@ -1,4 +1,4 @@
-from .path.to.user import user, persona
+import "path/to/user.zed"
 
 definition resource {
     relation user: user