[engine] Support analyzing only specified files
Users can now run `shac {check,fix,fmt} file1 file2 file3 ...` to run
checks only on the specified list of files, regardless of which files
are affected according to the SCM.
Checks that explicitly call `all_files()` will still see all files.
Change-Id: Ife998392327438a1fae443e477e0b20b5f8490a8
Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/904638
Commit-Queue: Marc-Antoine Ruel <maruel@google.com>
Reviewed-by: Marc-Antoine Ruel <maruel@google.com>
Fuchsia-Auto-Submit: Oliver Newman <olivernewman@google.com>
diff --git a/internal/cli/base.go b/internal/cli/base.go
index 3ad981c..929e908 100644
--- a/internal/cli/base.go
+++ b/internal/cli/base.go
@@ -15,6 +15,8 @@
package cli
import (
+ "errors"
+
flag "github.com/spf13/pflag"
"go.fuchsia.dev/shac-project/shac/internal/engine"
)
@@ -31,10 +33,14 @@
f.BoolVar(&c.noRecurse, "no-recurse", false, "do not look for shac.star files recursively")
}
-func (c *commandBase) options() engine.Options {
+func (c *commandBase) options(files []string) (engine.Options, error) {
+ if c.allFiles && len(files) > 0 {
+ return engine.Options{}, errors.New("--all cannot be set together with positional file arguments")
+ }
return engine.Options{
Root: c.root,
AllFiles: c.allFiles,
+ Files: files,
Recurse: !c.noRecurse,
- }
+ }, nil
}
diff --git a/internal/cli/check.go b/internal/cli/check.go
index d82f1f9..b88e5c7 100644
--- a/internal/cli/check.go
+++ b/internal/cli/check.go
@@ -17,7 +17,6 @@
import (
"bytes"
"context"
- "errors"
"os"
flag "github.com/spf13/pflag"
@@ -43,11 +42,7 @@
f.StringVar(&c.jsonOutput, "json-output", "", "path to write SARIF output to")
}
-func (c *checkCmd) Execute(ctx context.Context, args []string) error {
- if len(args) != 0 {
- return errors.New("unsupported arguments")
- }
-
+func (c *checkCmd) Execute(ctx context.Context, files []string) error {
var buf bytes.Buffer
r, err := reporting.Get(ctx)
@@ -56,7 +51,10 @@
}
r.Reporters = append(r.Reporters, &reporting.SarifReport{Out: &buf})
- o := c.options()
+ o, err := c.options(files)
+ if err != nil {
+ return err
+ }
o.Report = r
err = engine.Run(ctx, &o)
diff --git a/internal/cli/fix.go b/internal/cli/fix.go
index 064b48a..5ceb9e9 100644
--- a/internal/cli/fix.go
+++ b/internal/cli/fix.go
@@ -16,7 +16,6 @@
import (
"context"
- "errors"
flag "github.com/spf13/pflag"
"go.fuchsia.dev/shac-project/shac/internal/engine"
@@ -38,11 +37,11 @@
c.commandBase.SetFlags(f)
}
-func (c *fixCmd) Execute(ctx context.Context, args []string) error {
- if len(args) != 0 {
- return errors.New("unsupported arguments")
+func (c *fixCmd) Execute(ctx context.Context, files []string) error {
+ o, err := c.options(files)
+ if err != nil {
+ return err
}
- o := c.options()
o.Filter = engine.OnlyNonFormatters
return engine.Fix(ctx, &o)
}
diff --git a/internal/cli/fmt.go b/internal/cli/fmt.go
index 019249b..37fefaa 100644
--- a/internal/cli/fmt.go
+++ b/internal/cli/fmt.go
@@ -16,7 +16,6 @@
import (
"context"
- "errors"
flag "github.com/spf13/pflag"
"go.fuchsia.dev/shac-project/shac/internal/engine"
@@ -38,11 +37,11 @@
c.commandBase.SetFlags(f)
}
-func (c *fmtCmd) Execute(ctx context.Context, args []string) error {
- if len(args) != 0 {
- return errors.New("unsupported arguments")
+func (c *fmtCmd) Execute(ctx context.Context, files []string) error {
+ o, err := c.options(files)
+ if err != nil {
+ return err
}
- o := c.options()
o.Filter = engine.OnlyFormatters
return engine.Fix(ctx, &o)
}
diff --git a/internal/engine/run.go b/internal/engine/run.go
index 91f4de2..66a71ff 100644
--- a/internal/engine/run.go
+++ b/internal/engine/run.go
@@ -143,6 +143,8 @@
Report Report
// Root directory. Defaults to the current working directory.
Root string
+ // Files lists specific files to analyze.
+ Files []string
// AllFiles tells to consider all files as affected.
AllFiles bool
// Recurse tells the engine to run all Main files found in subdirectories.
@@ -204,6 +206,7 @@
}
scm = &cachingSCM{
scm: &filteredSCM{
+ files: o.Files,
matcher: gitignore.NewMatcher(patterns),
scm: scm,
},
diff --git a/internal/engine/run_test.go b/internal/engine/run_test.go
index 3c187b6..7c37ef0 100644
--- a/internal/engine/run_test.go
+++ b/internal/engine/run_test.go
@@ -116,6 +116,73 @@
}
}
+func TestRun_SpecificFiles(t *testing.T) {
+ t.Parallel()
+ root := t.TempDir()
+ writeFile(t, root, "shac.textproto", prototext.Format(&Document{
+ Ignore: []string{
+ "*.py",
+ },
+ }))
+
+ writeFile(t, root, "python.py", "a python file")
+ writeFile(t, root, "rust.rs", "a rust file")
+
+ copySCM(t, root)
+
+ data := []struct {
+ name string
+ want string
+ files []string
+ }{
+ {
+ "ctx-scm-affected_files.star",
+ "[//ctx-scm-affected_files.star:19] \n" +
+ scmStarlarkFiles("") +
+ "rust.rs: \n" +
+ "shac.textproto: \n" +
+ "\n",
+ nil,
+ },
+ {
+ "ctx-scm-all_files.star",
+ "[//ctx-scm-all_files.star:19] \n" +
+ scmStarlarkFiles("") +
+ "rust.rs: \n" +
+ "shac.textproto: \n" +
+ "\n",
+ nil,
+ },
+ }
+ for i := range data {
+ i := i
+ t.Run(data[i].name, func(t *testing.T) {
+ t.Parallel()
+ testStarlarkPrint(t, root, data[i].name, false, data[i].want)
+ })
+ }
+
+ t.Run("empty ignore field", func(t *testing.T) {
+ t.Parallel()
+ root := t.TempDir()
+ writeFile(t, root, "shac.textproto", prototext.Format(&Document{
+ Ignore: []string{
+ "*.foo",
+ "",
+ },
+ }))
+
+ r := reportPrint{reportNoPrint: reportNoPrint{t: t}}
+ o := Options{Report: &r, Root: root, AllFiles: false, main: "shac.star"}
+ err := Run(context.Background(), &o)
+ if err == nil {
+ t.Fatal("Expected empty ignore field to be rejected")
+ } else if !errors.Is(err, errEmptyIgnore) {
+ t.Fatalf("Expected error %q, got %q", errEmptyIgnore, err)
+ }
+ })
+}
+
func TestRun_Ignore(t *testing.T) {
t.Parallel()
root := t.TempDir()
@@ -207,11 +274,7 @@
t.Parallel()
want := "[//ctx-scm-affected_files.star:19] \n" +
"a.txt: \n" +
- "ctx-scm-affected_files-include_deleted.star: \n" +
- "ctx-scm-affected_files-new_lines.star: \n" +
- "ctx-scm-affected_files.star: \n" +
- "ctx-scm-all_files-include_deleted.star: \n" +
- "ctx-scm-all_files.star: \n" +
+ scmStarlarkFiles("") +
"\n"
testStarlarkPrint(t, root, "ctx-scm-affected_files.star", false, want)
})
@@ -225,22 +288,21 @@
t.Parallel()
want := "[//ctx-scm-all_files.star:19] \n" +
"a.txt: \n" +
- "ctx-scm-affected_files-include_deleted.star: \n" +
- "ctx-scm-affected_files-new_lines.star: \n" +
- "ctx-scm-affected_files.star: \n" +
- "ctx-scm-all_files-include_deleted.star: \n" +
- "ctx-scm-all_files.star: \n" +
+ scmStarlarkFiles("") +
"\n"
testStarlarkPrint(t, root, "ctx-scm-all_files.star", false, want)
})
}
-const scmStarlarkAddedFiles = "" +
- "ctx-scm-affected_files-include_deleted.star: A\n" +
- "ctx-scm-affected_files-new_lines.star: A\n" +
- "ctx-scm-affected_files.star: A\n" +
- "ctx-scm-all_files-include_deleted.star: A\n" +
- "ctx-scm-all_files.star: A\n"
+func scmStarlarkFiles(action string) string {
+ return fmt.Sprintf("" +
+ "ctx-scm-affected_files-include_deleted.star: " + action + "\n" +
+ "ctx-scm-affected_files-new_lines.star: " + action + "\n" +
+ "ctx-scm-affected_files.star: " + action + "\n" +
+ "ctx-scm-all_files-include_deleted.star: " + action + "\n" +
+ "ctx-scm-all_files.star: " + action + "\n",
+ )
+}
func TestRun_SCM_Git_NoUpstream_Pristine(t *testing.T) {
// No upstream branch set, pristine checkout.
@@ -259,7 +321,7 @@
"ctx-scm-affected_files.star",
false,
"[//ctx-scm-affected_files.star:19] \n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"\n",
},
{
@@ -267,7 +329,7 @@
true,
"[//ctx-scm-affected_files.star:19] \n" +
"a.txt: A\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
@@ -276,7 +338,7 @@
false,
"[//ctx-scm-all_files.star:19] \n" +
"a.txt: A\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
@@ -310,7 +372,7 @@
"ctx-scm-affected_files.star",
false,
"[//ctx-scm-affected_files.star:19] \n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"\n",
},
{
@@ -330,7 +392,7 @@
false,
"[//ctx-scm-all_files.star:19] \n" +
"a.txt: A\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
@@ -366,7 +428,7 @@
"ctx-scm-affected_files.star",
"[//ctx-scm-affected_files.star:19] \n" +
"a.txt: R\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
@@ -374,7 +436,7 @@
"ctx-scm-all_files.star",
"[//ctx-scm-all_files.star:19] \n" +
"a.txt: A\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
@@ -484,7 +546,7 @@
"ctx-scm-affected_files.star",
"[//ctx-scm-affected_files.star:19] \n" +
".gitmodules: A\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"\n",
},
{
@@ -492,7 +554,7 @@
"[//ctx-scm-all_files.star:19] \n" +
".gitmodules: A\n" +
"a.txt: A\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
@@ -542,7 +604,7 @@
"ctx-scm-all_files.star",
"[//ctx-scm-all_files.star:19] \n" +
"a.txt: A\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
@@ -551,13 +613,13 @@
"[//ctx-scm-all_files-include_deleted.star:26] \n" +
"With deleted:\n" +
"a.txt: A\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"file-to-delete.txt: D\n" +
"z.txt: A\n" +
"\n" +
"Without deleted:\n" +
"a.txt: A\n" +
- scmStarlarkAddedFiles +
+ scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n"},
}
diff --git a/internal/engine/runtime_ctx_scm.go b/internal/engine/runtime_ctx_scm.go
index 7f637e0..58edb9a 100644
--- a/internal/engine/runtime_ctx_scm.go
+++ b/internal/engine/runtime_ctx_scm.go
@@ -129,11 +129,20 @@
}
type filteredSCM struct {
+ // files lists specific files that should be force-included.
+ files []string
matcher gitignore.Matcher
scm scmCheckout
}
func (f *filteredSCM) affectedFiles(ctx context.Context, includeDeleted bool) ([]file, error) {
+ if len(f.files) > 0 {
+ var res []file
+ for _, path := range f.files {
+ res = append(res, &fileImpl{path: filepath.ToSlash(path)})
+ }
+ return res, nil
+ }
files, err := f.scm.affectedFiles(ctx, includeDeleted)
return f.filter(files), err
}
