[engine] Make message optional for formatter findings
Findings from checks will pretty much always specify the same message,
something like "File not formatted, please run `shac fmt`". Autogenerate
this message by default for formatting checks so check writers don't
need to write redundant messages.
This will also incentivize check writers to mark formatting checks as
`formatter=True`, otherwise they'll have to specify a message for every
finding.
Change-Id: I9b822d4177211d537cc539eacbcc109f4d14db6b
Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/887575
Fuchsia-Auto-Submit: Oliver Newman <olivernewman@google.com>
Reviewed-by: Marc-Antoine Ruel <maruel@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
diff --git a/doc/stdlib.md b/doc/stdlib.md
index 46bc06f..62d79b5 100644
--- a/doc/stdlib.md
+++ b/doc/stdlib.md
@@ -122,7 +122,7 @@
### Arguments
* **level**: One of "notice", "warning" or "error".
-* **message**: Message of the finding.
+* **message**: Message of the finding. May be omitted for checks marked with formatter=True, as long as filepath is specified, level is "error", and replacements has a length of 1.
* **filepath**: (optional) Path to the source file to annotate.
* **line**: (optional) Line where the finding should start. 1 based.
* **col**: (optional) Column where the finding should start. 1 based.
diff --git a/doc/stdlib.star b/doc/stdlib.star
index d7d84f3..1dc6ca7 100644
--- a/doc/stdlib.star
+++ b/doc/stdlib.star
@@ -111,7 +111,9 @@
Args:
level: One of "notice", "warning" or "error".
- message: Message of the finding.
+ message: Message of the finding. May be omitted for checks marked with
+ formatter=True, as long as filepath is specified, level is "error", and
+ replacements has a length of 1.
filepath: (optional) Path to the source file to annotate.
line: (optional) Line where the finding should start. 1 based.
col: (optional) Column where the finding should start. 1 based.
diff --git a/internal/engine/run_test.go b/internal/engine/run_test.go
index 2717825..c386c3b 100644
--- a/internal/engine/run_test.go
+++ b/internal/engine/run_test.go
@@ -945,7 +945,7 @@
},
{
"ctx-emit-finding-message.star",
- "ctx.emit.finding: for parameter \"message\": got \"\", want string",
+ "ctx.emit.finding: for parameter \"message\": must not be empty",
" //ctx-emit-finding-message.star:16:19: in cb\n",
},
{
diff --git a/internal/engine/runtime_ctx_emit.go b/internal/engine/runtime_ctx_emit.go
index 9aa44d2..3897490 100644
--- a/internal/engine/runtime_ctx_emit.go
+++ b/internal/engine/runtime_ctx_emit.go
@@ -39,7 +39,7 @@
var argreplacements starlark.Sequence
if err := starlark.UnpackArgs(name, args, kwargs,
"level", &arglevel,
- "message", &argmessage,
+ "message?", &argmessage,
"filepath?", &argfilepath,
"line?", &argline,
"col?", &argcol,
@@ -53,10 +53,7 @@
if !level.isValid() {
return fmt.Errorf("for parameter \"level\": got %s, want one of %q, %q or %q", arglevel, Notice, Warning, Error)
}
- message := string(argmessage)
- if len(message) == 0 {
- return fmt.Errorf("for parameter \"message\": got %s, want string", argmessage)
- }
+
file := string(argfilepath)
span := Span{
Start: Cursor{
@@ -115,7 +112,21 @@
return fmt.Errorf("for parameter \"replacements\": excessive number (%d) of replacements", len(replacements))
}
}
+
c := ctxCheck(ctx)
+ message := string(argmessage)
+ if len(message) == 0 {
+ if c.formatter && file != "" && level == Error && len(replacements) == 1 {
+ // If the check is a formatter, and the finding would be fixed by
+ // `shac fmt`, let users omit `message` as long as `file` is
+ // specified, since `message` will always look something like the
+ // following for formatters.
+ message = "File not formatted. Run `shac fmt` to fix."
+ } else {
+ return fmt.Errorf("for parameter \"message\": must not be empty")
+ }
+ }
+
if c.highestLevel == "" || level == Error || (level == Warning && c.highestLevel != Error) {
c.highestLevel = level
}
