feat: Implement use import by tstirrat15 · Pull Request #2919 · authzed/spicedb

tstirrat15 · 2026-02-24T18:45:53Z

Description

Part of the puzzle of folding composableschemadsl into schemadsl. This implements import logic behind a use flag.

Changes

Port over code from composableschemadsl
Remove test from importer tests that asserted nonlocal use behavior (expiration-usage or whatever it's called)
Add test that asserts that import statements are disallowed in a schema written to WriteSchema
Fix that test

Testing

Review. See that tests pass.

tstirrat15

See comments

tstirrat15 · 2026-02-24T18:46:11Z

internal/services/shared/errors.go

 	var relationNotFoundError sharederrors.UnknownRelationError

 	var compilerError compiler.BaseCompilerError
-	var sourceError *spiceerrors.WithSourceError


I'm pretty sure this was wrong?

Nvm it was right. I'm pretty confused as to why it is the way it is.

tstirrat15 · 2026-02-24T18:47:44Z

pkg/composableschemadsl/compiler/compiler.go

 // Compile compilers the input schema into a set of namespace definition protos.
 func Compile(schema InputSchema, prefix ObjectPrefixOption, opts ...Option) (*CompiledSchema, error) {
 	cfg := &config{
-		allowedFlags: mapz.NewSet[string](expirationFlag, selfFlag),


My LSP marked this as unnecessary

tstirrat15 · 2026-02-24T18:50:26Z

pkg/schemadsl/parser/parser.go

+		opts := strings.Join(slices.Sorted(maps.Keys(lexer.Flags)), ", ")
+		p.emitErrorf("Unknown use flag: `%s`. Options are: %s", useFlag, opts)


Porting this over from composableschemadsl - should give us a stable output order.

tstirrat15 · 2026-02-24T18:51:16Z

pkg/schemadsl/compiler/compiler_test.go

 			},
 		},
-		{
-			"duplicate use of expiration pragmas",


uses should be declared per-file, and should be treated as file-local, so the compiler shouldn't take issue with having multiple of them in the same file.

tstirrat15 · 2026-02-24T18:52:18Z

pkg/schemadsl/compiler/compiler.go

+	present, err := validateImportPresence(cfg.allowedFlags.Has(importFlag), root)
+	if err != nil {
+		// This condition should basically always be satisfied (we trigger errors off of the node),
+		// but we're defensive here in case the implementation changes.
+		var withNodeError withNodeError
+		if errors.As(err, &withNodeError) {
+			return nil, toContextError(withNodeError.Error(), withNodeError.errorSourceCode, withNodeError.node, mapper)
+		}
+	}


This is net-new - this is how we ensure that import compilation isn't attempted if it's disallowed.

tstirrat15 · 2026-02-24T18:52:37Z

pkg/schemadsl/compiler/compiler.go

+		}
+	}
+
+	if present {


And then because we have information about whether imports are present, we can either run this or not - it's a small optimization but why not

codecov · 2026-02-24T22:23:31Z

Codecov Report

❌ Patch coverage is 91.62304% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.49%. Comparing base (80b3228) to head (4a8d83c).

Files with missing lines	Patch %	Lines
pkg/schemadsl/compiler/importer.go	70.00%	4 Missing and 2 partials ⚠️
pkg/schemadsl/compiler/translator.go	89.84%	4 Missing and 2 partials ⚠️
pkg/schemadsl/compiler/node.go	40.00%	2 Missing and 1 partial ⚠️
pkg/schemadsl/compiler/compiler.go	98.12%	1 Missing ⚠️

❌ Your project check has failed because the head coverage (74.49%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2919       +/-   ##
===========================================
+ Coverage   49.06%   74.49%   +25.44%     
===========================================
  Files         420      488       +68     
  Lines       54148    60397     +6249     
===========================================
+ Hits        26560    44989    +18429     
+ Misses      24871    12258    -12613     
- Partials     2717     3150      +433

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

internal/services/v1/schema.go

internal/services/v1/schema_test.go

miparnisari · 2026-02-25T22:09:21Z

pkg/schemadsl/compiler/importer-test/many-imports/caveat.zed

@@ -0,0 +1,2 @@
+// caveat


if i import a file that has one caveat, does it work? or is imports only for definitions 🤔

Imports is for anything - caveats, definitions, partials

oh ok; i don't think i've seen a test where the imports is for caveats

miparnisari · 2026-02-25T22:16:13Z

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

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


do we have a test somewhere for a root with two imports, and both have the same definition sometype with different relations? it should error out

No, but I can add that

pkg/schemadsl/compiler/importer.go

pkg/schemadsl/compiler/importer_test.go

miparnisari · 2026-02-26T15:04:13Z

pkg/composableschemadsl/compiler/importer_test.go

+	}, compiler.AllowUnprefixedObjectType(),
+		compiler.SourceFolder(sourceFolder))
+
+	require.ErrorContains(t, err, "AAAAA")


pkg/schemadsl/compiler/importer-test/conflicting-definitions/left.zed

barakmich

LGTM with a few thoughts, mostly just positive comments

barakmich · 2026-02-24T19:15:47Z

internal/services/v1/schema_test.go

+func TestSchemaWriteImportsDisallowed(t *testing.T) {
+	conn, cleanup, _, _ := testserver.NewTestServer(require.New(t), 0, memdb.DisableGC, true, tf.EmptyDatastore)
+	t.Cleanup(cleanup)
+	client := v1.NewSchemaServiceClient(conn)
+
+	_, err := client.WriteSchema(t.Context(), &v1.WriteSchemaRequest{
+		Schema: `
+		use import


This is correct, in the context of the WriteSchema API -- however, I think that part of the overarching problem is that we don't have a WriteCompiledSchema API or similar

barakmich · 2026-02-24T19:17:25Z

pkg/schemadsl/compiler/importer-test/README.md

+
+Every folder should have a `root.zed`. This is the target for compilation.
+
+Every folder will have an `expected.zed`, which is the output of the compilation process.


The corollary to a WriteCompiledSchema API is that every folder would have a generated expected.proto. Again, farther in the future...

barakmich · 2026-02-24T19:18:13Z

pkg/schemadsl/compiler/compiler.go

+
+	// In an import context, this is the FS containing
+	// the importing schema (as opposed to imported schemas)
+	sourceFS fs.FS


nice use of fs.FS

I think I ended up here accidentally 😅

barakmich · 2026-02-24T19:21:22Z

pkg/schemadsl/compiler/importer_test.go

+	importerTests := []importerTest{
+		{"simple local import", "simple-local"},
+		{"simple local import with transitive hop", "simple-local-with-hop"},
+		{"nested local import", "nested-local"},
+		{"nested local import with transitive hop", "nested-local-with-hop"},
+		{"nested local two layers deep import", "nested-two-layer-local"},
+		{"diamond-shaped imports are fine", "diamond-shaped"},
+		{"multiple use directives are fine", "multiple-use-directives"},
+		{"many imports are correctly resolved", "many-imports"},
+	}


If you're feeling saucy, this could be collected by walking the directory (and omitting the name field), but I don't feel strongly to do so either way.

It does match pkg/schemadsl/parser/parser_test.go though so there's that too

Yeah, and the other part is that I'd then need a convention for negative tests (of which there are two or three). I found that it was easier to be explicit.

Co-authored-by: Maria Ines Parnisari <maria.ines.parnisari@authzed.com>

tstirrat15 requested a review from a team as a code owner February 24, 2026 18:45

github-actions bot added area/schema Affects the Schema Language area/api v1 Affects the v1 API area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Feb 24, 2026

tstirrat15 force-pushed the implement-use-import branch 3 times, most recently from 938d746 to b6a32f2 Compare February 24, 2026 18:51

tstirrat15 commented Feb 24, 2026

View reviewed changes

tstirrat15 force-pushed the implement-use-import branch 2 times, most recently from 5619cfd to 47395a4 Compare February 24, 2026 20:14

tstirrat15 force-pushed the implement-use-import branch from 47395a4 to 1bb8a7d Compare February 25, 2026 18:03

miparnisari reviewed Feb 26, 2026

View reviewed changes

tstirrat15 force-pushed the implement-use-import branch 4 times, most recently from cdfea10 to 6bc1e5e Compare February 26, 2026 14:59

miparnisari reviewed Feb 26, 2026

View reviewed changes

pkg/schemadsl/compiler/importer-test/conflicting-definitions/left.zed Show resolved Hide resolved

tstirrat15 force-pushed the implement-use-import branch 3 times, most recently from 86fa9b4 to 4a8d83c Compare February 26, 2026 16:05

tstirrat15 enabled auto-merge (squash) February 26, 2026 18:12

barakmich approved these changes Feb 26, 2026

View reviewed changes

tstirrat15 and others added 4 commits February 26, 2026 12:47

add import behavior to schemadsl

8b3f093

chore: remove test that asserts non-local lexer state

d6d3a22

chore: disable import behavior in writeschema

8362c23

Update pkg/schemadsl/compiler/importer.go

e433ebd

Co-authored-by: Maria Ines Parnisari <maria.ines.parnisari@authzed.com>

tstirrat15 force-pushed the implement-use-import branch from 4a8d83c to e433ebd Compare February 26, 2026 19:47

tstirrat15 merged commit 68bc889 into main Feb 26, 2026
67 of 76 checks passed

tstirrat15 deleted the implement-use-import branch February 26, 2026 20:16

github-actions bot locked and limited conversation to collaborators Feb 26, 2026

		opts := strings.Join(slices.Sorted(maps.Keys(lexer.Flags)), ", ")
		p.emitErrorf("Unknown use flag: `%s`. Options are: %s", useFlag, opts)


		Every folder should have a `root.zed`. This is the target for compilation.

		Every folder will have an `expected.zed`, which is the output of the compilation process.

Conversation

tstirrat15 commented Feb 24, 2026

Description

Changes

Testing

Uh oh!

tstirrat15 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

barakmich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Feb 24, 2026 •

edited

Loading