[engine] Make filepath a requirement for replacements or line

It doesn't make sense to specify a line range in a finding without
specifying a file. Likewise, it doesn't make sense to suggest
replacements without specifying a file in which to apply the
replacements.

Change-Id: Iabfe0e34e1af3797b573b49e079207d5f270340a
Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/887574
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>

internal/engine/run_test.go

@@ -933,6 +933,11 @@ func TestTestDataFailOrThrow(t *testing.T) {
 			"ctx.emit.finding: for parameter \"level\": got \"invalid\", want one of \"notice\", \"warning\" or \"error\"",
 			"  //ctx-emit-finding-level.star:16:19: in cb\n",
 		},
+		{
+			"ctx-emit-finding-line-no_file.star",
+			"ctx.emit.finding: for parameter \"line\": \"filepath\" must be specified",
+			"  //ctx-emit-finding-line-no_file.star:16:19: in cb\n",
+		},
 		{
 			"ctx-emit-finding-line.star",
 			"ctx.emit.finding: for parameter \"line\": got -1, line are 1 based",
@@ -953,6 +958,11 @@ func TestTestDataFailOrThrow(t *testing.T) {
 			"ctx.emit.finding: for parameter \"replacements\": got list, want sequence of str",
 			"  //ctx-emit-finding-replacements-list.star:16:19: in cb\n",
 		},
+		{
+			"ctx-emit-finding-replacements-no_file.star",
+			"ctx.emit.finding: for parameter \"replacements\": \"filepath\" must be specified",
+			"  //ctx-emit-finding-replacements-no_file.star:16:19: in cb\n",
+		},
 		{
 			"ctx-emit-finding-replacements-str.star",
 			"ctx.emit.finding: for parameter \"replacements\": got string, want starlark.Sequence",
@@ -1450,11 +1460,15 @@ func TestTestDataEmit(t *testing.T) {
 					Check:   "cb",
 					Level:   Notice,
 					Message: "great code",
+					Root:    root,
+					File:    "file.txt",
 					Span:    Span{Start: Cursor{Line: 100, Col: 2}},
 				}, {
 					Check:   "cb",
 					Level:   "notice",
 					Message: "nice",
+					Root:    root,
+					File:    "file.txt",
 					Span:    Span{Start: Cursor{Line: 100, Col: 2}, End: Cursor{Line: 100, Col: 3}},
 				},
 				{
@@ -1470,13 +1484,17 @@ func TestTestDataEmit(t *testing.T) {
 					Check:        "cb",
 					Level:        "warning",
 					Message:      "weird",
+					Root:         root,
+					File:         "file.txt",
 					Span:         Span{Start: Cursor{Line: 1}, End: Cursor{Line: 10}},
 					Replacements: []string{"a", "dict"},
 				},
 				{
 					Check:        "cb",
 					Level:        "warning",
 					Message:      "no span, full file",
+					Root:         root,
+					File:         "file.txt",
 					Replacements: []string{"this text is a replacement\nfor the entire file\n"},
 				},
 			},

internal/engine/runtime_ctx_emit.go

@@ -57,8 +57,6 @@ func ctxEmitFinding(ctx context.Context, s *shacState, name string, args starlar
 	if len(message) == 0 {
 		return fmt.Errorf("for parameter \"message\": got %s, want string", argmessage)
 	}
-	// TODO(olivernewman): Require filepath to be set if span and/or
-	// replacements are specified.
 	file := string(argfilepath)
 	span := Span{
 		Start: Cursor{
@@ -83,6 +81,9 @@ func ctxEmitFinding(ctx context.Context, s *shacState, name string, args starlar
 		return errors.New("for parameter \"end_col\": \"col\" must be specified")
 	}
 	if span.Start.Line > 0 {
+		if file == "" {
+			return errors.New("for parameter \"line\": \"filepath\" must be specified")
+		}
 		if span.End.Line > 0 {
 			if span.End.Line < span.Start.Line {
 				return errors.New("for parameter \"end_line\": must be greater than or equal to \"line\"")
@@ -104,6 +105,9 @@ func ctxEmitFinding(ctx context.Context, s *shacState, name string, args starlar
 	}
 	var replacements []string
 	if argreplacements != nil {
+		if file == "" {
+			return errors.New("for parameter \"replacements\": \"filepath\" must be specified")
+		}
 		if replacements = sequenceToStrings(argreplacements); replacements == nil {
 			return fmt.Errorf("for parameter \"replacements\": got %s, want sequence of str", argreplacements.Type())
 		}

internal/engine/testdata/emit/ctx-emit-finding.star

@@ -22,8 +22,19 @@ def cb(ctx):
       end_line=10,
       end_col=1,
       replacements=("a", "tuple"))
-  ctx.emit.finding(level="notice", line=100, col=2, message="great code")
-  ctx.emit.finding(level="notice", line=100, col=2, end_col=3, message="nice")
+  ctx.emit.finding(
+      level="notice",
+      filepath="file.txt",
+      line=100,
+      col=2,
+      message="great code")
+  ctx.emit.finding(
+      level="notice",
+      filepath="file.txt",
+      line=100,
+      col=2,
+      end_col=3,
+      message="nice")
   ctx.emit.finding(
       level="warning",
       message="please fix",
@@ -36,12 +47,14 @@ def cb(ctx):
   ctx.emit.finding(
       level="warning",
       message="weird",
+      filepath="file.txt",
       line=1,
       end_line=10,
       replacements={"a": True, "dict": 42})
   ctx.emit.finding(
       level="warning",
       message="no span, full file",
+      filepath="file.txt",
       replacements=["this text is a replacement\nfor the entire file\n"])
 
 shac.register_check(cb)

internal/engine/testdata/fail_or_throw/ctx-emit-finding-end_col-col-equal.star

@@ -16,6 +16,7 @@ def cb(ctx):
   ctx.emit.finding(
       level="notice",
       message="fix it",
+      filepath="foo.txt",
       line=1,
       col=2,
       end_line=1,

internal/engine/testdata/fail_or_throw/ctx-emit-finding-end_line-line-reverse.star

@@ -13,6 +13,11 @@
 # limitations under the License.
 
 def cb(ctx):
-  ctx.emit.finding(level="notice", message="fix it", line=2, end_line=1)
+  ctx.emit.finding(
+    level="notice",
+    message="fix it",
+    filepath="foo.txt",
+    line=2,
+    end_line=1)
 
 shac.register_check(cb)

internal/engine/testdata/fail_or_throw/ctx-emit-finding-line-no_file.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):
+  ctx.emit.finding(level="notice", message="fix it", line=1)
+
+shac.register_check(cb)

internal/engine/testdata/fail_or_throw/ctx-emit-finding-replacements-limit.star

@@ -17,6 +17,7 @@ def cb(ctx):
   ctx.emit.finding(
       level="notice",
       message="fix it",
+      filepath="foo.txt",
       replacements=too_many)
 
 shac.register_check(cb)

internal/engine/testdata/fail_or_throw/ctx-emit-finding-replacements-list.star

@@ -16,6 +16,7 @@ def cb(ctx):
   ctx.emit.finding(
       level="notice",
       message="fix it",
+      filepath="foo.txt",
       replacements=["nothing", 42])
 
 shac.register_check(cb)

internal/engine/testdata/fail_or_throw/ctx-emit-finding-replacements-no_file.star

@@ -0,0 +1,21 @@
+# 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):
+  ctx.emit.finding(
+      level="notice",
+      message="fix it",
+      replacements=["foo"])
+
+shac.register_check(cb)

internal/engine/testdata/fail_or_throw/ctx-emit-finding-replacements-tuple.star

@@ -16,6 +16,7 @@ def cb(ctx):
   ctx.emit.finding(
       level="notice",
       message="fix it",
+      filepath="foo.txt",
       replacements=("nothing", 42))
 
 shac.register_check(cb)