Skip to content

Commit

Permalink
[engine] Support analyzing only specified files
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Reviewed-by: Marc-Antoine Ruel <[email protected]>
Fuchsia-Auto-Submit: Oliver Newman <[email protected]>
  • Loading branch information
@orn688
orn688 authored and CQ Bot committed Aug 28, 2023
1 parent 9a26b49 commit 54b9a12
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 47 deletions.
10 changes: 8 additions & 2 deletions internal/cli/base.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package cli

import (
"errors"

flag "github.com/spf13/pflag"
"go.fuchsia.dev/shac-project/shac/internal/engine"
)
Expand All @@ -31,10 +33,14 @@ func (c *commandBase) SetFlags(f *flag.FlagSet) {
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
}
12 changes: 5 additions & 7 deletions internal/cli/check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package cli
import (
"bytes"
"context"
"errors"
"os"

flag "github.com/spf13/pflag"
Expand All @@ -43,11 +42,7 @@ func (c *checkCmd) SetFlags(f *flag.FlagSet) {
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)
Expand All @@ -56,7 +51,10 @@ func (c *checkCmd) Execute(ctx context.Context, args []string) error {
}
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)
Expand Down
9 changes: 4 additions & 5 deletions internal/cli/fix.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package cli

import (
"context"
"errors"

flag "github.com/spf13/pflag"
"go.fuchsia.dev/shac-project/shac/internal/engine"
Expand All @@ -38,11 +37,11 @@ func (c *fixCmd) SetFlags(f *flag.FlagSet) {
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)
}
9 changes: 4 additions & 5 deletions internal/cli/fmt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package cli

import (
"context"
"errors"

flag "github.com/spf13/pflag"
"go.fuchsia.dev/shac-project/shac/internal/engine"
Expand All @@ -38,11 +37,11 @@ func (c *fmtCmd) SetFlags(f *flag.FlagSet) {
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)
}
3 changes: 3 additions & 0 deletions internal/engine/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ type Options struct {
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.
Expand Down Expand Up @@ -204,6 +206,7 @@ func Run(ctx context.Context, o *Options) error {
}
scm = &cachingSCM{
scm: &filteredSCM{
files: o.Files,
matcher: gitignore.NewMatcher(patterns),
scm: scm,
},
Expand Down
118 changes: 90 additions & 28 deletions internal/engine/run_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,73 @@ func TestRun_Fail(t *testing.T) {
}
}

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()
Expand Down Expand Up @@ -207,11 +274,7 @@ func TestRun_SCM_Raw(t *testing.T) {
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)
})
Expand All @@ -225,22 +288,21 @@ func TestRun_SCM_Raw(t *testing.T) {
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.
Expand All @@ -259,15 +321,15 @@ func TestRun_SCM_Git_NoUpstream_Pristine(t *testing.T) {
"ctx-scm-affected_files.star",
false,
"[//ctx-scm-affected_files.star:19] \n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"\n",
},
{
"ctx-scm-affected_files.star",
true,
"[//ctx-scm-affected_files.star:19] \n" +
"a.txt: A\n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
Expand All @@ -276,7 +338,7 @@ func TestRun_SCM_Git_NoUpstream_Pristine(t *testing.T) {
false,
"[//ctx-scm-all_files.star:19] \n" +
"a.txt: A\n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
Expand Down Expand Up @@ -310,7 +372,7 @@ func TestRun_SCM_Git_NoUpstream_Staged(t *testing.T) {
"ctx-scm-affected_files.star",
false,
"[//ctx-scm-affected_files.star:19] \n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"\n",
},
{
Expand All @@ -330,7 +392,7 @@ func TestRun_SCM_Git_NoUpstream_Staged(t *testing.T) {
false,
"[//ctx-scm-all_files.star:19] \n" +
"a.txt: A\n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
Expand Down Expand Up @@ -366,15 +428,15 @@ func TestRun_SCM_Git_Upstream_Staged(t *testing.T) {
"ctx-scm-affected_files.star",
"[//ctx-scm-affected_files.star:19] \n" +
"a.txt: R\n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
{
"ctx-scm-all_files.star",
"[//ctx-scm-all_files.star:19] \n" +
"a.txt: A\n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
Expand Down Expand Up @@ -484,15 +546,15 @@ func TestRun_SCM_Git_Submodule(t *testing.T) {
"ctx-scm-affected_files.star",
"[//ctx-scm-affected_files.star:19] \n" +
".gitmodules: A\n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"\n",
},
{
"ctx-scm-all_files.star",
"[//ctx-scm-all_files.star:19] \n" +
".gitmodules: A\n" +
"a.txt: A\n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
Expand Down Expand Up @@ -542,7 +604,7 @@ func TestRun_SCM_DeletedFile(t *testing.T) {
"ctx-scm-all_files.star",
"[//ctx-scm-all_files.star:19] \n" +
"a.txt: A\n" +
scmStarlarkAddedFiles +
scmStarlarkFiles("A") +
"z.txt: A\n" +
"\n",
},
Expand All @@ -551,13 +613,13 @@ func TestRun_SCM_DeletedFile(t *testing.T) {
"[//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"},
}
Expand Down
9 changes: 9 additions & 0 deletions internal/engine/runtime_ctx_scm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,20 @@ type scmCheckout interface {
}

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
}
Expand Down

0 comments on commit 54b9a12

Please sign in to comment.