From 1815956d9dc9e32a5163ed7b2693e0dd577ad6e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 27 Mar 2026 01:00:46 -0400 Subject: [PATCH 1/2] feat(core::graders): add import-required and import-allowlist guards Add two new guards for checking student imports: - mk-import-required: verifies a specific module is imported, optionally with a required alias name - mk-import-allowlist: restricts which modules students can import, blocking any unlisted modules Both guards share internal helpers for parsing and extracting import metadata from the AST. Includes unit tests and test fixtures. Co-authored-by: IRONM00N --- pkgs/core/src/graders/imports.arr | 195 ++++++++++++++++++++++++++ pkgs/core/src/graders/main.arr | 2 + pkgs/core/tests/files/has-imports.arr | 4 + pkgs/core/tests/files/no-imports.arr | 1 + pkgs/core/tests/graders/imports.arr | 61 ++++++++ pkgs/core/tests/graders/main.arr | 1 + 6 files changed, 264 insertions(+) create mode 100644 pkgs/core/src/graders/imports.arr create mode 100644 pkgs/core/tests/files/has-imports.arr create mode 100644 pkgs/core/tests/files/no-imports.arr create mode 100644 pkgs/core/tests/graders/imports.arr diff --git a/pkgs/core/src/graders/imports.arr b/pkgs/core/src/graders/imports.arr new file mode 100644 index 0000000..1a1b15d --- /dev/null +++ b/pkgs/core/src/graders/imports.arr @@ -0,0 +1,195 @@ +import file("../core.arr") as C +import file("../grading.arr") as G +import file("../grading-builders.arr") as GB +import file("../common/ast.arr") as CA +import file("../common/markdown.arr") as MD + +import ast as A + +include either +include from C: + type Id +end + +provide: + data ImportRequiredBlock, + data ImportAllowlistBlock, + mk-import-required, + mk-import-allowlist, + check-import-required as _check-import-required, + check-import-allowlist as _check-import-allowlist, + fmt-import-required as _fmt-import-required, + fmt-import-allowlist as _fmt-import-allowlist +end + +# --- Shared helpers --- + +fun get-imports(path :: String): + cases (Either) CA.parse-path(path): + | left(err) => left(err) + | right(ast) => + cases (A.Program) ast: + | s-program(_, _, _, _, _, imports, _) => right(imports) + end + end +end + +fun import-module-name(imp) -> Option: + doc: ``` + Extract the module name string from an import or include AST node. + Returns none for import forms we don't recognize (e.g. file imports). + ``` + fun import-type-name(it): + cases (A.ImportType) it: + | s-const-import(_, mod) => some(mod) + | else => none + end + end + ask: + | A.is-s-import(imp) then: import-type-name(imp.file) + | A.is-s-include(imp) then: import-type-name(imp.mod) + | A.is-s-import-fields(imp) then: import-type-name(imp.file) + | otherwise: none + end +end + +fun import-binding-name(imp) -> Option: + doc: ``` + Extract the binding name from an import AST node. + Returns none for include statements (which don't bind to a name). + ``` + ask: + | A.is-s-import(imp) then: + cases (A.Name) imp.name: + | s-name(_, s) => some(s) + | else => none + end + | otherwise: none + end +end + +# --- Guard 1: Import Required --- + +data ImportRequiredBlock: + | ir-parser-error(err :: CA.ParsePathErr) + | import-not-found(module-name :: String) + | wrong-binding(module-name :: String, expected :: String, actual :: String) +end + +fun check-import-required( + path :: String, + module-name :: String, + binding :: Option +) -> Option: + cases (Either) get-imports(path): + | left(err) => some(ir-parser-error(err)) + | right(imports) => + matching = imports.filter(lam(imp): + import-module-name(imp) == some(module-name) + end) + cases (List) matching: + | empty => some(import-not-found(module-name)) + | link(first, _) => + cases (Option) binding: + | none => none + | some(expected-binding) => + actual = import-binding-name(first) + cases (Option) actual: + | none => none + | some(actual-binding) => + if actual-binding == expected-binding: + none + else: + some(wrong-binding(module-name, expected-binding, actual-binding)) + end + end + end + end + end +end + +fun fmt-import-required(reason :: ImportRequiredBlock) -> GB.ComboAggregate: + student = cases (ImportRequiredBlock) reason: + | ir-parser-error(_) => + "Cannot check your imports because we cannot parse your file." + | import-not-found(mod) => + "Cannot find an import for module " + MD.escape-inline-code(mod) + + ". Make sure you have `import " + MD.escape-inline-code(mod) + + " as ...` at the top of your file." + | wrong-binding(mod, expected, actual) => + "Module " + MD.escape-inline-code(mod) + + " is imported as " + MD.escape-inline-code(actual) + + ", but it should be imported as " + MD.escape-inline-code(expected) + "." + end ^ G.output-markdown + staff = none + {student; staff} +end + +fun mk-import-required( + id :: Id, + deps :: List, + path :: String, + module-name :: String, + binding :: Option +): + name = "Required import " + module-name + checker = lam(): check-import-required(path, module-name, binding) end + GB.mk-guard(id, deps, checker, name, fmt-import-required) +end + +# --- Guard 2: Import Allowlist --- + +data ImportAllowlistBlock: + | ia-parser-error(err :: CA.ParsePathErr) + | forbidden-imports(names :: List, allowed :: List) +end + +fun check-import-allowlist( + path :: String, + allowed :: List +) -> Option: + cases (Either) get-imports(path): + | left(err) => some(ia-parser-error(err)) + | right(imports) => + forbidden = imports + .map(import-module-name) + .filter(is-some) + .map(lam(s): s.value end) + .filter(lam(mod): not(allowed.member(mod)) end) + if forbidden.length() == 0: + none + else: + some(forbidden-imports(forbidden, allowed)) + end + end +end + +fun fmt-import-allowlist(reason :: ImportAllowlistBlock) -> GB.ComboAggregate: + student = cases (ImportAllowlistBlock) reason: + | ia-parser-error(_) => + "Cannot check your imports because we cannot parse your file." + | forbidden-imports(names, allowed) => + forbidden-str = names + .map(MD.escape-inline-code) + .join-str(", ") + allowed-str = allowed + .map(MD.escape-inline-code) + .join-str(", ") + "Your program imports modules that are not allowed for this assignment: " + + forbidden-str + ". " + + "Only the following modules are permitted: " + allowed-str + "." + end ^ G.output-markdown + staff = none + {student; staff} +end + +fun mk-import-allowlist( + id :: Id, + deps :: List, + path :: String, + allowed :: List +): + name = "Allowed imports" + checker = lam(): check-import-allowlist(path, allowed) end + GB.mk-guard(id, deps, checker, name, fmt-import-allowlist) +end diff --git a/pkgs/core/src/graders/main.arr b/pkgs/core/src/graders/main.arr index 678c2b1..600c70c 100644 --- a/pkgs/core/src/graders/main.arr +++ b/pkgs/core/src/graders/main.arr @@ -25,6 +25,7 @@ import file("./const-def.arr") as const-def import file("./test-diversity.arr") as test-diversity import file("./training-wheels.arr") as training-wheels import file("./image-artifact.arr") as image-artifact +import file("./imports.arr") as imports # NOTE: only provides the functions, everything else should be an # implementation detail and can be imported directly from the module @@ -38,3 +39,4 @@ provide from const-def: * end provide from test-diversity: * end provide from training-wheels: * end provide from image-artifact: * end +provide from imports: * end diff --git a/pkgs/core/tests/files/has-imports.arr b/pkgs/core/tests/files/has-imports.arr new file mode 100644 index 0000000..1cffea2 --- /dev/null +++ b/pkgs/core/tests/files/has-imports.arr @@ -0,0 +1,4 @@ +import tables as T +import image as I + +1 + 1 diff --git a/pkgs/core/tests/files/no-imports.arr b/pkgs/core/tests/files/no-imports.arr new file mode 100644 index 0000000..2c45705 --- /dev/null +++ b/pkgs/core/tests/files/no-imports.arr @@ -0,0 +1 @@ +fun double(x): x * 2 end diff --git a/pkgs/core/tests/graders/imports.arr b/pkgs/core/tests/graders/imports.arr new file mode 100644 index 0000000..13da638 --- /dev/null +++ b/pkgs/core/tests/graders/imports.arr @@ -0,0 +1,61 @@ +import file("../meta/path-utils.arr") as P +include file("../../src/graders/imports.arr") +import file("../../src/common/ast.arr") as CA + +check-import-required = _check-import-required +check-import-allowlist = _check-import-allowlist +fmt-import-required = _fmt-import-required +fmt-import-allowlist = _fmt-import-allowlist + +check "import-required: found with correct binding": + check-import-required(P.file("has-imports.arr"), "tables", some("T")) is none + check-import-required(P.file("has-imports.arr"), "image", some("I")) is none +end + +check "import-required: found with any binding": + check-import-required(P.file("has-imports.arr"), "tables", none) is none + check-import-required(P.file("has-imports.arr"), "image", none) is none +end + +check "import-required: not found": + check-import-required(P.file("has-imports.arr"), "math", none) + is some(import-not-found("math")) + check-import-required(P.file("no-imports.arr"), "tables", none) + is some(import-not-found("tables")) +end + +check "import-required: wrong binding": + check-import-required(P.file("has-imports.arr"), "tables", some("Tbl")) + is some(wrong-binding("tables", "Tbl", "T")) +end + +check "import-allowlist: all allowed": + check-import-allowlist(P.file("has-imports.arr"), [list: "tables", "image"]) is none + check-import-allowlist(P.file("has-imports.arr"), [list: "tables", "image", "math"]) is none +end + +check "import-allowlist: no imports": + check-import-allowlist(P.file("no-imports.arr"), [list: "tables"]) is none + check-import-allowlist(P.file("no-imports.arr"), [list:]) is none +end + +check "import-allowlist: forbidden": + result = check-import-allowlist(P.file("has-imports.arr"), [list: "tables"]) + result is some(forbidden-imports([list: "image"], [list: "tables"])) +end + +check "import-allowlist: all forbidden": + result = check-import-allowlist(P.file("has-imports.arr"), [list:]) + result is some(forbidden-imports([list: "tables", "image"], [list:])) +end + +check "fmt-import-required: smoke": + fmt-import-required(ir-parser-error(CA.path-doesnt-exist("/invalid.arr"))) does-not-raise + fmt-import-required(import-not-found("tables")) does-not-raise + fmt-import-required(wrong-binding("tables", "T", "Tbl")) does-not-raise +end + +check "fmt-import-allowlist: smoke": + fmt-import-allowlist(ia-parser-error(CA.path-doesnt-exist("/invalid.arr"))) does-not-raise + fmt-import-allowlist(forbidden-imports([list: "image"], [list: "tables"])) does-not-raise +end diff --git a/pkgs/core/tests/graders/main.arr b/pkgs/core/tests/graders/main.arr index 4e14d4a..5a294db 100644 --- a/pkgs/core/tests/graders/main.arr +++ b/pkgs/core/tests/graders/main.arr @@ -22,3 +22,4 @@ import file("./const-def.arr") as _ import file("./test-diversity.arr") as _ import file("./training-wheels.arr") as _ import file("./image-artifact.arr") as _ +import file("./imports.arr") as _ From 3c9151054ea79d645b5a34162f7bc70afaa7d324 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 27 Mar 2026 01:00:48 -0400 Subject: [PATCH 2/2] test(core): add integration test for import-required guard with wheat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Demonstrates the key use case: a wheat test depends on the student having a particular import (`import image as I`). The DAG is: wf → import-required → fn-def → wheat Includes example student file, wheat implementation, and npm script. Co-authored-by: IRONM00N --- .github/workflows/ci.yml | 4 +++ pkgs/core/package.json | 1 + .../tests/examples/double-grading/wheat.arr | 3 ++ pkgs/core/tests/examples/double.arr | 8 +++++ .../tests/integration/inspect-imports.arr | 34 +++++++++++++++++++ 5 files changed, 50 insertions(+) create mode 100644 pkgs/core/tests/examples/double-grading/wheat.arr create mode 100644 pkgs/core/tests/examples/double.arr create mode 100644 pkgs/core/tests/integration/inspect-imports.arr diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 50899f8..55afc81 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,6 +41,10 @@ jobs: working-directory: pkgs/core run: pnpm run integration:tower + - name: (core) imports integration test + working-directory: pkgs/core + run: pnpm run integration:imports + nix: strategy: fail-fast: false diff --git a/pkgs/core/package.json b/pkgs/core/package.json index 61b7ca4..1371299 100644 --- a/pkgs/core/package.json +++ b/pkgs/core/package.json @@ -15,6 +15,7 @@ "integration:gcd": "pnpm run pyret-trove -- -p tests/integration/inspect-gcd.arr", "integration:fold": "pnpm run pyret-trove -- -p tests/integration/inspect-fold.arr", "integration:tower": "pnpm run pyret-trove -- -p tests/integration/inspect-tower.arr", + "integration:imports": "pnpm run pyret-trove -- -p tests/integration/inspect-imports.arr", "clean": "rm -rf -- ./pyret" }, "dependencies": { diff --git a/pkgs/core/tests/examples/double-grading/wheat.arr b/pkgs/core/tests/examples/double-grading/wheat.arr new file mode 100644 index 0000000..186932c --- /dev/null +++ b/pkgs/core/tests/examples/double-grading/wheat.arr @@ -0,0 +1,3 @@ +fun double(x): + x * 2 +end diff --git a/pkgs/core/tests/examples/double.arr b/pkgs/core/tests/examples/double.arr new file mode 100644 index 0000000..d785902 --- /dev/null +++ b/pkgs/core/tests/examples/double.arr @@ -0,0 +1,8 @@ +import image as I + +fun double(x): + x * 2 +where: + double(3) is 6 + double(0) is 0 +end diff --git a/pkgs/core/tests/integration/inspect-imports.arr b/pkgs/core/tests/integration/inspect-imports.arr new file mode 100644 index 0000000..ced1338 --- /dev/null +++ b/pkgs/core/tests/integration/inspect-imports.arr @@ -0,0 +1,34 @@ +import file("../meta/path-utils.arr") as P +include file("../meta/inspect-grade.arr") +include file("../../src/main.arr") +include file("../../src/tools/main.arr") + +student-path = P.example("double.arr") +wheat-path = P.example("double-grading/wheat.arr") + +graders = + [list: + mk-well-formed("wf", [list:], student-path), + mk-import-required( + "import-image", + [list: "wf"], + student-path, "image", some("I") + ), + mk-fn-def( + "double-defined", + [list: "import-image"], + student-path, "double", 1 + ), + mk-wheat( + "double-wheat", + [list: "double-defined"], + student-path, wheat-path, "double", + 1 + ) + ] + +result = inspect-grade(graders, true, false) + +check "aggregate-to-flat smoke": + grading-helpers.aggregate-to-flat(result.aggregated) does-not-raise +end