[engine] Add check.with_args(...) method
This allows downstream repositories to override arguments to checks in
shared checks repositories, such as the path to an executable to run.
I also added a `with_name()` method, mainly to make it easier to test
`with_args()` (since it's not allowed to register multiple checks with
the same name) but it may also be useful for downstream users.
Change-Id: I43e50768bbf9c90d3c9aaa28ef447ef330c31d5c
Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/919559
Reviewed-by: Marc-Antoine Ruel <maruel@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
Fuchsia-Auto-Submit: Oliver Newman <olivernewman@google.com>
diff --git a/checks/check_doc.star b/checks/check_doc.star
index 20d4118..56272be 100644
--- a/checks/check_doc.star
+++ b/checks/check_doc.star
@@ -12,15 +12,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-load("//doc/stdlib.star", doc_ctx = "ctx", doc_shac = "shac")
+load("//doc/stdlib.star", doc_check = "check", doc_ctx = "ctx", doc_shac = "shac")
+
+a_check = shac.check(lambda ctx: None, name = "a_check")
def check_docs(ctx):
- """Validates that the `ctx` and `shac` structs in //doc/stdlib.star are
- up-to-date.
+ """Validates that structs in //doc/stdlib.star are up-to-date.
- Specifically, it should (recursively) have all the same fields with all the
- same types as the `shac` global and the `ctx` object that gets passed to
- checks.
+ Specifically, documented structs should (recursively) have all the same
+ fields with all the same types as the real objects.
Function signatures are not validated.
@@ -28,25 +28,24 @@
Starlark interpreter to get function signatures.
Args:
- ctx: A ctx instance.
+ ctx: A ctx instance.
"""
- want = _struct_signature(ctx)
- got = _struct_signature(doc_ctx)
- if want != got:
- ctx.emit.finding(
- level = "error",
- message = "stdlib.star needs to be updated. Want:\n%s\nGot:\n%s" % (want, got),
- )
- want = _struct_signature(shac)
- got = _struct_signature(doc_shac)
- if want != got:
- ctx.emit.finding(
- level = "error",
- message = "stdlib.star needs to be updated. Want:\n%s\nGot:\n%s" % (want, got),
- )
+ pairs = [
+ (ctx, doc_ctx),
+ (shac, doc_shac),
+ (a_check, doc_check),
+ ]
+ for actual, documented in pairs:
+ want = _struct_signature(actual)
+ got = _struct_signature(documented)
+ if want != got:
+ ctx.emit.finding(
+ level = "error",
+ message = "stdlib.star needs to be updated. Want:\n%s\nGot:\n%s" % (want, got),
+ )
def _struct_signature(s):
- if type(s) != type(struct()):
+ if type(s) != type(struct()) and not type(s).startswith("shac."):
# stdlib.star uses dummy functions instead of actual builtin functions, they
# should be considered equivalent.
if type(s) == "builtin_function_or_method":
diff --git a/doc/stdlib.md b/doc/stdlib.md
index 55643c5..b63737c 100644
--- a/doc/stdlib.md
+++ b/doc/stdlib.md
@@ -20,6 +20,7 @@
## Table of contents
- [shac](#shac)
+- [check](#check)
- [ctx](#ctx)
- [dir](#dir)
- [fail](#fail)
@@ -101,6 +102,66 @@
* **check**: `shac.check()` object or Starlark function that is called back to implement the check.
+## check
+
+check is the object returned by shac.check().
+
+Fields:
+
+- with_args
+- with_name
+
+## check.with_args
+
+Create a copy of the check with keyword arguments overridden.
+
+### Example
+
+```python
+def cb(ctx, level = "warning"):
+ ctx.emit.finding(
+ level = level,
+ message = "Found an issue",
+ )
+
+warning_check = shac.check(cb)
+error_check = warning_check.with_args(level = "error")
+```
+
+### Arguments
+
+* **\*\*kwargs**: Overridden keyword arguments.
+
+## check.with_name
+
+Create a copy of the check with name overridden.
+
+Useful for changing the name of checks provided by shared Starlark
+libraries.
+
+### Example
+
+Shared library code:
+
+```python
+def _check_with_bad_name(ctx):
+ pass
+
+check_with_bad_name = shac.check(_check_with_bad_name)
+```
+
+Downstream code:
+
+```
+load("@library", "check_with_bad_name")
+
+shac.register_check(check_with_bad_name.with_name("better_name"))
+```
+
+### Arguments
+
+* **name**: The new name of the check.
+
## ctx
ctx is the object passed to [shac.register_check(...)](#shac.register-check) callback.
diff --git a/doc/stdlib.star b/doc/stdlib.star
index 91336f3..84473dc 100644
--- a/doc/stdlib.star
+++ b/doc/stdlib.star
@@ -104,6 +104,65 @@
version = (0, 0, 1),
)
+## Methods on the shac.check object.
+
+def _check_with_args(**kwargs):
+ """Create a copy of the check with keyword arguments overridden.
+
+ Example:
+
+ ```python
+ def cb(ctx, level = "warning"):
+ ctx.emit.finding(
+ level = level,
+ message = "Found an issue",
+ )
+
+ warning_check = shac.check(cb)
+ error_check = warning_check.with_args(level = "error")
+ ```
+
+ Args:
+ **kwargs: Overridden keyword arguments.
+ """
+ pass
+
+def _check_with_name(name):
+ """Create a copy of the check with name overridden.
+
+ Useful for changing the name of checks provided by shared Starlark
+ libraries.
+
+ Example:
+
+ Shared library code:
+
+ ```python
+ def _check_with_bad_name(ctx):
+ pass
+
+ check_with_bad_name = shac.check(_check_with_bad_name)
+ ```
+
+ Downstream code:
+
+ ```
+ load("@library", "check_with_bad_name")
+
+ shac.register_check(check_with_bad_name.with_name("better_name"))
+ ```
+
+ Args:
+ name: The new name of the check.
+ """
+ pass
+
+# check is the object returned by shac.check().
+check = struct(
+ with_args = _check_with_args,
+ with_name = _check_with_name,
+)
+
## Methods inside ctx object.
def _ctx_emit_finding(level, message, filepath = None, line = None, col = None, end_line = None, end_col = None, replacements = None):
diff --git a/internal/engine/run.go b/internal/engine/run.go
index 244269f..0851e2d 100644
--- a/internal/engine/run.go
+++ b/internal/engine/run.go
@@ -668,7 +668,7 @@
func (c *registeredCheck) call(ctx context.Context, env *starlarkEnv, args starlark.Tuple, pi printImpl) error {
ctx = context.WithValue(ctx, &checkCtxKey, c)
th := env.thread(ctx, c.name, pi)
- if r, err := starlark.Call(th, c.impl, args, nil); err != nil {
+ if r, err := starlark.Call(th, c.impl, args, c.kwargs); err != nil {
if c.failErr != nil {
// fail() was called, return this error since this is an abnormal failure.
return c.failErr
diff --git a/internal/engine/run_test.go b/internal/engine/run_test.go
index 84f5424..32b1ca1 100644
--- a/internal/engine/run_test.go
+++ b/internal/engine/run_test.go
@@ -179,7 +179,7 @@
i := i
t.Run(data[i].name, func(t *testing.T) {
t.Parallel()
- testStarlarkPrint(t, data[i].dir, "", false, data[i].want)
+ testStarlarkPrint(t, data[i].dir, "", false, false, data[i].want)
})
}
}
@@ -266,7 +266,7 @@
t.Fatal(err)
}
}()
- testStarlarkPrint(t, root, data[i].starlarkFile, false, data[i].want, data[i].files...)
+ testStarlarkPrint(t, root, data[i].starlarkFile, false, false, data[i].want, data[i].files...)
})
}
@@ -439,7 +439,7 @@
i := i
t.Run(data[i].name, func(t *testing.T) {
t.Parallel()
- testStarlarkPrint(t, root, data[i].name, false, data[i].want)
+ testStarlarkPrint(t, root, data[i].name, false, false, data[i].want)
})
}
@@ -536,13 +536,13 @@
"a.txt: \n" +
scmStarlarkFiles("") +
"\n"
- testStarlarkPrint(t, root, "ctx-scm-affected_files.star", false, want)
+ testStarlarkPrint(t, root, "ctx-scm-affected_files.star", false, false, want)
})
t.Run("affected-new_lines", func(t *testing.T) {
t.Parallel()
want := "[//ctx-scm-affected_files-new_lines.star:33] a.txt\n" +
"1: First file\n"
- testStarlarkPrint(t, root, "ctx-scm-affected_files-new_lines.star", false, want)
+ testStarlarkPrint(t, root, "ctx-scm-affected_files-new_lines.star", false, false, want)
})
t.Run("all", func(t *testing.T) {
t.Parallel()
@@ -550,7 +550,7 @@
"a.txt: \n" +
scmStarlarkFiles("") +
"\n"
- testStarlarkPrint(t, root, "ctx-scm-all_files.star", false, want)
+ testStarlarkPrint(t, root, "ctx-scm-all_files.star", false, false, want)
})
}
@@ -611,7 +611,7 @@
}
t.Run(name, func(t *testing.T) {
t.Parallel()
- testStarlarkPrint(t, root, data[i].name, data[i].all, data[i].want)
+ testStarlarkPrint(t, root, data[i].name, data[i].all, false, data[i].want)
})
}
}
@@ -665,7 +665,7 @@
}
t.Run(name, func(t *testing.T) {
t.Parallel()
- testStarlarkPrint(t, root, data[i].name, data[i].all, data[i].want)
+ testStarlarkPrint(t, root, data[i].name, data[i].all, false, data[i].want)
})
}
}
@@ -705,7 +705,7 @@
i := i
t.Run(data[i].name, func(t *testing.T) {
t.Parallel()
- testStarlarkPrint(t, root, data[i].name, false, data[i].want)
+ testStarlarkPrint(t, root, data[i].name, false, false, data[i].want)
})
}
}
@@ -823,7 +823,7 @@
i := i
t.Run(data[i].name, func(t *testing.T) {
t.Parallel()
- testStarlarkPrint(t, root, data[i].name, false, data[i].want)
+ testStarlarkPrint(t, root, data[i].name, false, false, data[i].want)
})
}
}
@@ -887,7 +887,7 @@
i := i
t.Run(data[i].name, func(t *testing.T) {
t.Parallel()
- testStarlarkPrint(t, root, data[i].name, false, data[i].want)
+ testStarlarkPrint(t, root, data[i].name, false, false, data[i].want)
})
}
}
@@ -938,7 +938,7 @@
}
t.Run(name, func(t *testing.T) {
t.Parallel()
- testStarlarkPrint(t, root, data[i].name, data[i].all, data[i].want)
+ testStarlarkPrint(t, root, data[i].name, data[i].all, false, data[i].want)
})
}
}
@@ -1549,6 +1549,21 @@
" //load-recurse.star:15:1: in <toplevel>\n",
},
{
+ "shac-check-with_args-ctx.star",
+ "with_args: \"ctx\" argument cannot be overridden",
+ " //shac-check-with_args-ctx.star:18:45: in <toplevel>\n",
+ },
+ {
+ "shac-check-with_args-nonexistent_kwarg.star",
+ "with_args: invalid argument \"nonexistent\", must be one of: (foo)",
+ " //shac-check-with_args-nonexistent_kwarg.star:18:45: in <toplevel>\n",
+ },
+ {
+ "shac-check-with_args-positional_args.star",
+ "with_args: only keyword arguments are allowed",
+ " //shac-check-with_args-positional_args.star:18:45: in <toplevel>\n",
+ },
+ {
"shac-immutable.star",
"can't assign to .key field of struct",
" //shac-immutable.star:16:5: in <toplevel>\n",
@@ -1718,7 +1733,7 @@
if err := os.Chmod(filepath.Join(root, "http_get.sh"), 0o700); err != nil {
t.Fatal(err)
}
- testStarlarkPrint(t, root, "shac.star", false, data[i].want)
+ testStarlarkPrint(t, root, "shac.star", false, false, data[i].want)
})
}
}
@@ -1872,20 +1887,21 @@
p, got := enumDir(t, "print")
v := fmt.Sprintf("(%d, %d, %d)", Version[0], Version[1], Version[2])
data := []struct {
- name string
- want string
+ name string
+ want string
+ ignoreOrder bool
}{
{
- "ctx-io-read_file-size.star",
- "[//ctx-io-read_file-size.star:16] {\n \"key\":\n",
+ name: "ctx-io-read_file-size.star",
+ want: "[//ctx-io-read_file-size.star:16] {\n \"key\":\n",
},
{
- "ctx-io-read_file.star",
- "[//ctx-io-read_file.star:17] {\"key\": \"value\"}\n",
+ name: "ctx-io-read_file.star",
+ want: "[//ctx-io-read_file.star:17] {\"key\": \"value\"}\n",
},
{
- "ctx-io-tempdir.star",
- func() string {
+ name: "ctx-io-tempdir.star",
+ want: func() string {
if runtime.GOOS == "windows" {
return "[//ctx-io-tempdir.star:16] \\0\\0\n" +
"[//ctx-io-tempdir.star:17] \\0\\1\n" +
@@ -1897,17 +1913,17 @@
}(),
},
{
- "ctx-io-tempfile.star",
- "[//ctx-io-tempfile.star:18] first\nfile\ncontents\n\n" +
+ name: "ctx-io-tempfile.star",
+ want: "[//ctx-io-tempfile.star:18] first\nfile\ncontents\n\n" +
"[//ctx-io-tempfile.star:19] contents\nof\nsecond\nfile\n\n",
},
{
- "ctx-os-exec-10Mib.star",
- "[//ctx-os-exec-10Mib.star:17] 0\n",
+ name: "ctx-os-exec-10Mib.star",
+ want: "[//ctx-os-exec-10Mib.star:17] 0\n",
},
{
- "ctx-os-exec-abspath.star",
- func() string {
+ name: "ctx-os-exec-abspath.star",
+ want: func() string {
// TODO(maruel): Decide if we want to do CRLF translation automatically.
if runtime.GOOS == "windows" {
return "[//ctx-os-exec-abspath.star:17] Hello, world\r\n\n"
@@ -1916,8 +1932,8 @@
}(),
},
{
- "ctx-os-exec-env.star",
- func() string {
+ name: "ctx-os-exec-env.star",
+ want: func() string {
// TODO(maruel): Decide if we want to do CRLF translation automatically.
if runtime.GOOS == "windows" {
return "[//ctx-os-exec-env.star:24] FOO=foo-value\r\nBAR=bar-value\n"
@@ -1926,12 +1942,12 @@
}(),
},
{
- "ctx-os-exec-parallel.star",
- strings.Repeat("[//ctx-os-exec-parallel.star:27] Hello, world\n", 10),
+ name: "ctx-os-exec-parallel.star",
+ want: strings.Repeat("[//ctx-os-exec-parallel.star:27] Hello, world\n", 10),
},
{
- "ctx-os-exec-relpath.star",
- func() string {
+ name: "ctx-os-exec-relpath.star",
+ want: func() string {
// TODO(maruel): Decide if we want to do CRLF translation automatically.
if runtime.GOOS == "windows" {
return "[//ctx-os-exec-relpath.star:17] Hello from a nested file\r\n\n"
@@ -1940,8 +1956,8 @@
}(),
},
{
- "ctx-os-exec-stdin.star",
- "[//ctx-os-exec-stdin.star:30] stdout given NoneType for stdin:\n" +
+ name: "ctx-os-exec-stdin.star",
+ want: "[//ctx-os-exec-stdin.star:30] stdout given NoneType for stdin:\n" +
"\n" +
"[//ctx-os-exec-stdin.star:30] stdout given string for stdin:\n" +
"hello\nfrom\nstdin\nstring\n" +
@@ -1949,75 +1965,88 @@
"hello\nfrom\nstdin\nbytes\n",
},
{
- "ctx-os-exec-success.star",
- "[//ctx-os-exec-success.star:21] retcode: 0\n" +
+ name: "ctx-os-exec-success.star",
+ want: "[//ctx-os-exec-success.star:21] retcode: 0\n" +
"[//ctx-os-exec-success.star:22] stdout: hello from stdout\n" +
"[//ctx-os-exec-success.star:23] stderr: hello from stderr\n",
},
{
- "ctx-platform.star",
- "[//ctx-platform.star:16] OS: " + runtime.GOOS + "\n" +
+ name: "ctx-platform.star",
+ want: "[//ctx-platform.star:16] OS: " + runtime.GOOS + "\n" +
"[//ctx-platform.star:17] Arch: " + runtime.GOARCH + "\n",
},
{
- "ctx-re-allmatches.star",
- "[//ctx-re-allmatches.star:17] ()\n" +
+ name: "ctx-re-allmatches.star",
+ want: "[//ctx-re-allmatches.star:17] ()\n" +
"[//ctx-re-allmatches.star:20] (match(groups = (\"TODO(foo)\",), offset = 4), match(groups = (\"TODO(bar)\",), offset = 14))\n" +
"[//ctx-re-allmatches.star:23] (match(groups = (\"anc\", \"n\", \"c\"), offset = 0),)\n",
},
{
- "ctx-re-match.star",
- "[//ctx-re-match.star:17] None\n" +
+ name: "ctx-re-match.star",
+ want: "[//ctx-re-match.star:17] None\n" +
"[//ctx-re-match.star:20] match(groups = (\"TODO(foo)\",), offset = 4)\n" +
"[//ctx-re-match.star:23] match(groups = (\"anc\", \"n\", \"c\"), offset = 0)\n" +
"[//ctx-re-match.star:26] match(groups = (\"a\", None), offset = 0)\n",
},
{
- "dir-ctx.star",
- "[//dir-ctx.star:16] [\"emit\", \"io\", \"os\", \"platform\", \"re\", \"scm\", \"vars\"]\n",
+ name: "dir-ctx.star",
+ want: "[//dir-ctx.star:16] [\"emit\", \"io\", \"os\", \"platform\", \"re\", \"scm\", \"vars\"]\n",
},
{
- "dir-shac.star",
- "[//dir-shac.star:15] [\"check\", \"commit_hash\", \"register_check\", \"version\"]\n",
+ name: "dir-shac.star",
+ want: "[//dir-shac.star:15] [\"check\", \"commit_hash\", \"register_check\", \"version\"]\n",
},
{
- "load-diamond_dependency.star",
- "[//load-diamond_dependency.star:18] i am a constant\n" +
+ name: "load-diamond_dependency.star",
+ want: "[//load-diamond_dependency.star:18] i am a constant\n" +
"[//load-diamond_dependency.star:19] i am a constant #2\n",
},
{
- "print-shac-version.star",
- "[//print-shac-version.star:15] " + v + "\n",
+ name: "print-shac-version.star",
+ want: "[//print-shac-version.star:15] " + v + "\n",
},
{
- "shac-check.star",
- "[//shac-check.star:19] str(check): <check hello_world>\n" +
+ name: "shac-check-with_args.star",
+ want: "[//shac-check-with_args.star:33] print_hello_check: <check print_hello>\n" +
+ "[//shac-check-with_args.star:34] print_goodbye_check: <check print_goodbye>\n" +
+ "[//shac-check-with_args.star:35] print_hello_again_check: <check print_hello_again>\n" +
+ "[//shac-check-with_args.star:16] hello again\n" +
+ "[//shac-check-with_args.star:16] goodbye\n" +
+ "[//shac-check-with_args.star:16] hello\n",
+ // Ordering of output lines depends on the order in which checks are
+ // run, which is nondeterministic and doesn't matter for the
+ // purposes of this test.
+ ignoreOrder: true,
+ },
+ {
+ name: "shac-check.star",
+ want: "[//shac-check.star:19] str(check): <check hello_world>\n" +
"[//shac-check.star:20] type(check): shac.check\n" +
"[//shac-check.star:21] bool(check): True\n" +
"[//shac-check.star:26] hashed: set([<check hello_world>])\n",
},
{
- "shac-register_check-object.star",
- "[//shac-register_check-object.star:16] running from a check object\n",
+ name: "shac-register_check-object.star",
+ want: "[//shac-register_check-object.star:16] running from a check object\n",
},
{
- "shac-register_check-optional_param.star",
- "[//shac-register_check-optional_param.star:16] optional_param=\"optional-param-value\"\n",
+ name: "shac-register_check-optional_param.star",
+ want: "[//shac-register_check-optional_param.star:16] optional_param=\"optional-param-value\"\n",
},
{
- "shac-register_check.star",
- "[//shac-register_check.star:16] running\n",
+ name: "shac-register_check.star",
+ want: "[//shac-register_check.star:16] running\n",
},
{
- "subprocess.star",
- "[//subprocess.star:17] str(proc): <subprocess \"echo hello\">\n" +
+ name: "subprocess.star",
+ want: "[//subprocess.star:17] str(proc): <subprocess \"echo hello\">\n" +
"[//subprocess.star:18] type(proc): subprocess\n" +
"[//subprocess.star:19] bool(proc): True\n" +
"[//subprocess.star:20] dir(proc): [\"wait\"]\n",
},
{
- "true.star",
- "[//true.star:15] True\n",
+ name: "true.star",
+ want: "[//true.star:15] True\n",
},
}
want := make([]string, len(data))
@@ -2031,7 +2060,7 @@
i := i
t.Run(data[i].name, func(t *testing.T) {
t.Parallel()
- testStarlarkPrint(t, p, data[i].name, false, data[i].want)
+ testStarlarkPrint(t, p, data[i].name, false, data[i].ignoreOrder, data[i].want)
})
}
}
@@ -2069,32 +2098,44 @@
"\n" +
"[//ctx-os-exec-filesystem_sandbox.star:21] sandbox_write.sh retcode: 1\n" +
"[//ctx-os-exec-filesystem_sandbox.star:22] touch: cannot touch 'file.txt': Read-only file system\n\n"
- testStarlarkPrint(t, root, "ctx-os-exec-filesystem_sandbox.star", false, want)
+ testStarlarkPrint(t, root, "ctx-os-exec-filesystem_sandbox.star", false, false, want)
}
func TestRun_Vendored(t *testing.T) {
t.Parallel()
dir := t.TempDir()
copyTree(t, dir, "testdata/vendored", nil)
- testStarlarkPrint(t, dir, "shac.star", false, "[//shac.star:17] True\n")
+ testStarlarkPrint(t, dir, "shac.star", false, false, "[//shac.star:17] True\n")
}
// Utilities
// testStarlarkPrint test a starlark file that calls print().
-func testStarlarkPrint(t testing.TB, root, name string, all bool, want string, files ...string) {
+func testStarlarkPrint(t testing.TB, root, name string, all bool, ignoreOrder bool, want string, files ...string) {
r := reportPrint{reportNoPrint: reportNoPrint{t: t}}
o := Options{Report: &r, Dir: root, AllFiles: all, EntryPoint: name, Files: files}
if err := Run(context.Background(), &o); err != nil {
t.Helper()
t.Fatal(err)
}
- if diff := cmp.Diff(want, r.b.String()); diff != "" {
+ got := r.b.String()
+ if ignoreOrder {
+ want = sortLines(want)
+ got = sortLines(got)
+ }
+ if diff := cmp.Diff(want, got); diff != "" {
t.Helper()
t.Fatalf("mismatch (-want +got):\n%s", diff)
}
}
+func sortLines(str string) string {
+ str = strings.TrimSuffix(str, "\n")
+ lines := strings.Split(str, "\n")
+ slices.Sort(lines)
+ return strings.Join(lines, "\n") + "\n"
+}
+
func enumDir(t *testing.T, name string) (string, []string) {
p, err := filepath.Abs(filepath.Join("testdata", name))
if err != nil {
diff --git a/internal/engine/runtime.go b/internal/engine/runtime.go
index 041ba62..2073526 100644
--- a/internal/engine/runtime.go
+++ b/internal/engine/runtime.go
@@ -126,19 +126,20 @@
ctx := getContext(th)
s := ctxShacState(ctx)
val, err := f(ctx, s)
- // starlark.UnpackArgs already adds the function name prefix to errors
- // it returns, so make sure not to duplicate the prefix if it's already
- // there.
- if err != nil && !strings.HasPrefix(err.Error(), name+": ") {
- err = fmt.Errorf("%s: %w", name, err)
+ if err != nil {
+ // starlark.UnpackArgs already adds the function name prefix to errors
+ // it returns, so make sure not to duplicate the prefix if it's already
+ // there.
+ if !strings.HasPrefix(err.Error(), name+": ") {
+ err = fmt.Errorf("%s: %w", name, err)
+ }
+ return nil, err
}
- if val != nil {
- // All values returned by builtins are immutable. This is not a hard
- // requirement, and can be relaxed if there's a use case for mutable
- // return values, but it's still a sensible default.
- val.Freeze()
- }
- return val, err
+ // All values returned by builtins are immutable. This is not a hard
+ // requirement, and can be relaxed if there's a use case for mutable
+ // return values, but it's still a sensible default.
+ val.Freeze()
+ return val, nil
}
func newBuiltinNone(name string, f func(ctx context.Context, s *shacState, name string, args starlark.Tuple, kwargs []starlark.Tuple) error) *starlark.Builtin {
diff --git a/internal/engine/runtime_shac_check.go b/internal/engine/runtime_shac_check.go
index 934598a..e6ec24f 100644
--- a/internal/engine/runtime_shac_check.go
+++ b/internal/engine/runtime_shac_check.go
@@ -18,6 +18,8 @@
"context"
"errors"
"fmt"
+ "log"
+ "slices"
"strings"
"go.starlark.net/starlark"
@@ -77,9 +79,10 @@
name string
// Whether the check is an auto-formatter or not.
formatter bool
+ kwargs []starlark.Tuple
}
-var _ starlark.Value = (*check)(nil)
+var _ starlark.HasAttrs = (*check)(nil)
func (c *check) String() string {
return fmt.Sprintf("<check %s>", c.name)
@@ -102,3 +105,90 @@
// hashing just the name of the check seems reasonable.
return starlark.String(c.name).Hash()
}
+
+func (c *check) Attr(name string) (starlark.Value, error) {
+ switch name {
+ case "with_args":
+ return checkWithArgsBuiltin.BindReceiver(c), nil
+ case "with_name":
+ return checkWithNameBuiltin.BindReceiver(c), nil
+ default:
+ return nil, nil
+ }
+}
+
+func (c *check) AttrNames() []string {
+ return []string{"with_args", "with_name"}
+}
+
+func (c *check) withName(name string) (starlark.Value, error) {
+ // Make a copy to modify.
+ res := *c
+ res.name = name
+ return &res, nil
+}
+
+func (c *check) withArgs(kwargs []starlark.Tuple) (starlark.Value, error) {
+ // Make a copy to modify.
+ res := *c
+
+ validParams := make([]string, 0, c.impl.NumParams()-1)
+ for i := 1; i < c.impl.NumParams(); i++ {
+ name, _ := c.impl.Param(i)
+ validParams = append(validParams, name)
+ }
+
+ newKwargs := kwargsMap(res.kwargs)
+ for k, v := range kwargsMap(kwargs) {
+ if k == "ctx" {
+ return nil, errors.New("\"ctx\" argument cannot be overridden")
+ }
+ if !slices.Contains(validParams, k) {
+ return nil, fmt.Errorf("invalid argument %q, must be one of: (%s)", k, strings.Join(validParams, ", "))
+ }
+ newKwargs[k] = v
+ }
+
+ res.kwargs = make([]starlark.Tuple, 0, len(newKwargs))
+ for k, v := range newKwargs {
+ res.kwargs = append(res.kwargs, starlark.Tuple{starlark.String(k), v})
+ }
+ return &res, nil
+}
+
+// checkWithArgsBuiltin implements the native function shac.check().with_args().
+//
+// Make sure to update //doc/stdlib.star whenever this function is modified.
+var checkWithArgsBuiltin = newBoundBuiltin("with_args", func(ctx context.Context, s *shacState, name string, self starlark.Value, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
+ if len(args) != 0 {
+ return nil, fmt.Errorf("only keyword arguments are allowed")
+ }
+ return self.(*check).withArgs(kwargs)
+})
+
+// checkWithNameBuiltin implements the native function shac.check().with_name().
+//
+// Make sure to update //doc/stdlib.star whenever this function is modified.
+var checkWithNameBuiltin = newBoundBuiltin("with_name", func(ctx context.Context, s *shacState, name string, self starlark.Value, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
+ var argname starlark.String
+ if err := starlark.UnpackArgs(name, args, kwargs,
+ "name?", &argname); err != nil {
+ return nil, err
+ }
+ return self.(*check).withName(string(argname))
+})
+
+func kwargsMap(kwargs []starlark.Tuple) map[string]starlark.Value {
+ res := make(map[string]starlark.Value, len(kwargs))
+ for _, item := range kwargs {
+ if len(item) != 2 {
+ log.Panicf("kwargs item does not have length 2: %+v", kwargs)
+ }
+ s, ok := item[0].(starlark.String)
+ if !ok {
+ log.Panicf("kwargs item does not have a string key: %+v", kwargs)
+ }
+ res[string(s)] = item[1]
+ }
+ return res
+}
diff --git a/internal/engine/testdata/fail_or_throw/shac-check-with_args-ctx.star b/internal/engine/testdata/fail_or_throw/shac-check-with_args-ctx.star
new file mode 100644
index 0000000..2ed9f44
--- /dev/null
+++ b/internal/engine/testdata/fail_or_throw/shac-check-with_args-ctx.star
@@ -0,0 +1,18 @@
+# Copyright 2023 The Shac Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+def cb(ctx):
+ pass
+
+shac.register_check(shac.check(cb).with_args(ctx = "blah"))
diff --git a/internal/engine/testdata/fail_or_throw/shac-check-with_args-nonexistent_kwarg.star b/internal/engine/testdata/fail_or_throw/shac-check-with_args-nonexistent_kwarg.star
new file mode 100644
index 0000000..a09ab5e
--- /dev/null
+++ b/internal/engine/testdata/fail_or_throw/shac-check-with_args-nonexistent_kwarg.star
@@ -0,0 +1,18 @@
+# Copyright 2023 The Shac Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+def cb(ctx, foo = "bar"):
+ pass
+
+shac.register_check(shac.check(cb).with_args(nonexistent = "blah"))
diff --git a/internal/engine/testdata/fail_or_throw/shac-check-with_args-positional_args.star b/internal/engine/testdata/fail_or_throw/shac-check-with_args-positional_args.star
new file mode 100644
index 0000000..fb6374d
--- /dev/null
+++ b/internal/engine/testdata/fail_or_throw/shac-check-with_args-positional_args.star
@@ -0,0 +1,18 @@
+# Copyright 2023 The Shac Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+def cb(ctx, an_arg = "bar"):
+ pass
+
+shac.register_check(shac.check(cb).with_args("foo"))
diff --git a/internal/engine/testdata/print/shac-check-with_args.star b/internal/engine/testdata/print/shac-check-with_args.star
new file mode 100644
index 0000000..56ea87d
--- /dev/null
+++ b/internal/engine/testdata/print/shac-check-with_args.star
@@ -0,0 +1,39 @@
+# Copyright 2023 The Shac Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+def _print_something(ctx, to_print = "something"):
+ print(to_print)
+
+print_hello_check = (
+ shac.check(_print_something, name = "print_hello")
+ .with_args(to_print = "hello")
+)
+print_goodbye_check = (
+ print_hello_check
+ .with_args(to_print = "goodbye")
+ .with_name("print_goodbye")
+)
+print_hello_again_check = (
+ print_goodbye_check
+ .with_args(to_print = "hello again")
+ .with_name("print_hello_again")
+)
+
+print("print_hello_check:", print_hello_check)
+print("print_goodbye_check:", print_goodbye_check)
+print("print_hello_again_check:", print_hello_again_check)
+
+shac.register_check(print_hello_check)
+shac.register_check(print_goodbye_check)
+shac.register_check(print_hello_again_check)
diff --git a/internal/engine/version.go b/internal/engine/version.go
index b4779ed..24ed15a 100644
--- a/internal/engine/version.go
+++ b/internal/engine/version.go
@@ -18,5 +18,5 @@
// Version is the current tool version.
//
// TODO(maruel): Add proper version, preferably from git tag.
- Version = [...]int{0, 1, 5}
+ Version = [...]int{0, 1, 6}
)
diff --git a/shac.textproto b/shac.textproto
index 8d73af9..bc4b74c 100644
--- a/shac.textproto
+++ b/shac.textproto
@@ -15,7 +15,7 @@
# TODO(olivernewman): Build github.com/protocolbuffers/txtpbfmt into shac and
# enforce formatting of shac.textproto files in all repos that use shac.
-min_shac_version: "0.1.5"
+min_shac_version: "0.1.6"
allow_network: False
ignore: "/vendor/"
# Vendored code for test data only.
