Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement simpler import syntax #2207

Merged
merged 1 commit into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .subjects import user
import "subjects.zed"

definition resource {
relation viewer: user
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
from .user import user
import "user.zed"

definition persona {}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
from .subjects import persona
import "subjects.zed"

definition user {}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
from .subjects import user
import "subjects.zed"
Original file line number Diff line number Diff line change
@@ -1 +1 @@
from .subjects import persona
import "subjects.zed"
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from .left import user
from .right import persona
import "left.zed"
import "right.zed"

definition resource {
relation viewer: user
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "../user.zed"
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .transitive.transitive import user
import "transitive/transitive.zed"

definition resource {
relation viewer: user
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
from .user.user import user
import "user/user.zed"
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .user.user import user
import "user/user.zed"

definition resource {
relation viewer: user
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .definitions.user.user import user
import "definitions/user/user.zed"

definition resource {
relation viewer: user
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
from .user import user
import "user.zed"
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .indirection import user
import "indirection.zed"

definition resource {
relation viewer: user
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .user import user
import "user.zed"

definition resource {
relation viewer: user
Expand Down
27 changes: 20 additions & 7 deletions pkg/composableschemadsl/compiler/importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"path"
"path/filepath"
"strings"

"github.com/rs/zerolog/log"

Expand All @@ -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)

Expand Down Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also add a check for .. anywhere in the path; if so, we can also quick-error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like at the parser level?

return fmt.Errorf("import path %s does not stay within its folder", path)
}
return nil
}
20 changes: 20 additions & 0 deletions pkg/composableschemadsl/compiler/importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
17 changes: 4 additions & 13 deletions pkg/composableschemadsl/compiler/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 1 addition & 4 deletions pkg/composableschemadsl/dslshape/dslshape.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
1 change: 0 additions & 1 deletion pkg/composableschemadsl/lexer/lex_def.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ var keywords = map[string]struct{}{
"permission": {},
"nil": {},
"with": {},
"from": {},
"import": {},
"all": {},
"any": {},
Expand Down
1 change: 0 additions & 1 deletion pkg/composableschemadsl/lexer/lex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}},
Expand Down
64 changes: 7 additions & 57 deletions pkg/composableschemadsl/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
}
31 changes: 31 additions & 0 deletions pkg/composableschemadsl/parser/parser_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: newline before large comments here and above


// 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...)
Expand Down
9 changes: 4 additions & 5 deletions pkg/composableschemadsl/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/composableschemadsl/parser/tests/localimport.zed
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .path.to.user import user, persona
import "path/to/user.zed"

definition resource {
relation user: user
Expand Down
Loading
Loading