Skip to content

Commit

Permalink
feat(cli): add lint --fix|diff|report (#4161)
Browse files Browse the repository at this point in the history
Adds `lint --fix|diff|report` bool args to apply fixes or output a diff,
while keeping the existing report output as the default.

### Type of change

- New feature or functionality (change which adds functionality)

### Test plan

- Manual testing

GitOrigin-RevId: ab7514c87f5fba12d0b7e8899cc8e51f47777657
  • Loading branch information
jbedard authored and alexeagle committed Jan 4, 2024
1 parent 8fa431c commit 06cea8b
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 23 deletions.
8 changes: 7 additions & 1 deletion cmd/aspect/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func NewDefaultCmd(pluginSystem system.PluginSystem) *cobra.Command {
}

func NewCmd(streams ioutils.Streams, pluginSystem system.PluginSystem, bzl bazel.Bazel) *cobra.Command {
return &cobra.Command{
cmd := &cobra.Command{
Use: "lint <target patterns>",
Args: cobra.MinimumNArgs(1),
Short: "Run configured linters over the dependency graph.",
Expand All @@ -46,4 +46,10 @@ func NewCmd(streams ioutils.Streams, pluginSystem system.PluginSystem, bzl bazel
lint.New(streams, bzl).Run,
),
}

cmd.Flags().Bool("fix", false, "Apply patch fixes for lint errors")
cmd.Flags().Bool("diff", false, "Output patch fixes for lint errors")
cmd.Flags().Bool("report", true, "Output lint reports")

return cmd
}
1 change: 1 addition & 0 deletions pkg/aspect/lint/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ go_library(
"//pkg/bazel",
"//pkg/ioutils",
"//pkg/plugin/system/bep",
"@com_github_bluekeyes_go_gitdiff//gitdiff",
"@com_github_fatih_color//:color",
"@com_github_spf13_cobra//:cobra",
"@com_github_spf13_viper//:viper",
Expand Down
124 changes: 102 additions & 22 deletions pkg/aspect/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package lint

import (
"bytes"
"context"
"fmt"
"os"
Expand All @@ -27,17 +28,40 @@ import (
"aspect.build/cli/pkg/bazel"
"aspect.build/cli/pkg/ioutils"
"aspect.build/cli/pkg/plugin/system/bep"
"github.com/bluekeyes/go-gitdiff/gitdiff"
"github.com/fatih/color"
"github.com/spf13/cobra"
"github.com/spf13/viper"
)

type Linter struct {
ioutils.Streams
bzl bazel.Bazel
bazelBin string
executionRoot string
reports map[string]*buildeventstream.NamedSetOfFiles
bzl bazel.Bazel
}

type LintBEPHandler struct {
ioutils.Streams
report bool
fix bool
diff bool
reports map[string]*buildeventstream.NamedSetOfFiles
}

// Align with rules_lint
const (
LINT_REPORT_GROUP = "rules_lint_report"
LINT_PATCH_GROUP = "rules_lint_patch"
LINT_RESULT_REGEX = ".*aspect_rules_lint.*"
)

func newLintBEPHandler(streams ioutils.Streams, printReport, applyFix, printDiff bool) *LintBEPHandler {
return &LintBEPHandler{
Streams: streams,
report: printReport,
fix: applyFix,
diff: printDiff,
reports: make(map[string]*buildeventstream.NamedSetOfFiles),
}
}

func New(
Expand All @@ -47,11 +71,10 @@ func New(
return &Linter{
Streams: streams,
bzl: bzl,
reports: make(map[string]*buildeventstream.NamedSetOfFiles),
}
}

func (runner *Linter) Run(ctx context.Context, _ *cobra.Command, args []string) error {
func (runner *Linter) Run(ctx context.Context, cmd *cobra.Command, args []string) error {
linters := viper.GetStringSlice("lint.aspects")

if len(linters) == 0 {
Expand All @@ -66,9 +89,23 @@ lint:
return nil
}

applyFix, _ := cmd.Flags().GetBool("fix")
printDiff, _ := cmd.Flags().GetBool("diff")
printReport, _ := cmd.Flags().GetBool("report")

bazelCmd := []string{"build"}
bazelCmd = append(bazelCmd, fmt.Sprintf("--aspects=%s", strings.Join(linters, ",")), "--output_groups=rules_lint_report")
bazelCmd = append(bazelCmd, args...)
bazelCmd = append(bazelCmd, fmt.Sprintf("--aspects=%s", strings.Join(linters, ",")))
bazelCmd = append(bazelCmd, cmd.Flags().Args()...)

outputGroups := []string{}
if applyFix || printDiff {
outputGroups = append(outputGroups, LINT_PATCH_GROUP)
}
if printReport {
outputGroups = append(outputGroups, LINT_REPORT_GROUP)
}
bazelCmd = append(bazelCmd, fmt.Sprintf("--output_groups=%s", strings.Join(outputGroups, ",")))
bazelCmd = append(bazelCmd, fmt.Sprintf("--experimental_remote_download_regex='%s'", LINT_RESULT_REGEX))

// Currently Bazel only supports a single --bes_backend so adding ours after
// any user supplied value will result in our bes_backend taking precedence.
Expand All @@ -78,9 +115,11 @@ lint:
// that using the Aspect CLI resolves it.
if bep.HasBESBackend(ctx) {
besBackend := bep.BESBackendFromContext(ctx)
besBackend.RegisterSubscriber(runner.BEPEventCallback)
besBackendFlag := fmt.Sprintf("--bes_backend=%s", besBackend.Addr())
bazelCmd = flags.AddFlagToCommand(bazelCmd, besBackendFlag)

lintBEPHandler := newLintBEPHandler(runner.Streams, printReport, applyFix, printDiff)
besBackend.RegisterSubscriber(lintBEPHandler.BEPEventCallback)
}

err := runner.bzl.RunCommand(runner.Streams, nil, bazelCmd...)
Expand All @@ -99,17 +138,9 @@ lint:
return err
}

func (runner *Linter) BEPEventCallback(event *buildeventstream.BuildEvent) error {
func (runner *LintBEPHandler) BEPEventCallback(event *buildeventstream.BuildEvent) error {
switch event.Payload.(type) {

case *buildeventstream.BuildEvent_WorkspaceInfo:
// Record workspace information
runner.executionRoot = event.GetWorkspaceInfo().GetLocalExecRoot()

case *buildeventstream.BuildEvent_Configuration:
// Record configuration information
runner.bazelBin = event.GetConfiguration().GetMakeVariable()["BINDIR"]

case *buildeventstream.BuildEvent_NamedSetOfFiles:
// Assert no collisions
namedSetId := event.Id.GetNamedSet().GetId()
Expand All @@ -128,7 +159,14 @@ func (runner *Linter) BEPEventCallback(event *buildeventstream.BuildEvent) error
for _, fileSetId := range outputGroup.FileSets {
if fileSet := runner.reports[fileSetId.Id]; fileSet != nil {
for _, f := range fileSet.GetFiles() {
err := runner.outputLintResult(label, f)

var err error
if outputGroup.Name == LINT_PATCH_GROUP {
err = runner.patchLintResult(label, f, runner.fix, runner.diff)
} else if outputGroup.Name == LINT_REPORT_GROUP && runner.report {
err = runner.outputLintResult(label, f)
}

if err != nil {
return err
}
Expand All @@ -143,8 +181,50 @@ func (runner *Linter) BEPEventCallback(event *buildeventstream.BuildEvent) error
return nil
}

func (runner *Linter) outputLintResult(label string, f *buildeventstream.File) error {
lineResult, err := runner.readLintResultFile(f)
func (runner *LintBEPHandler) patchLintResult(label string, f *buildeventstream.File, applyDiff, printDiff bool) error {
lintPatch, err := runner.readBEPFile(f)
if err != nil {
return err
}

if printDiff {
color.New(color.FgYellow).Fprintf(runner.Streams.Stdout, "From %s:\n", label)
fmt.Fprintf(runner.Streams.Stdout, "%s\n", lintPatch)
}

if applyDiff {
files, _, err := gitdiff.Parse(strings.NewReader(lintPatch))
if err != nil {
return err
}

for _, file := range files {
// TODO: file.IsNew|IsDeleted|IsRename|IsCopy

oldSrc, openErr := os.OpenFile(file.OldName[2:], os.O_RDONLY, 0)
if openErr != nil {
return openErr
}
defer oldSrc.Close()

var output bytes.Buffer
applyErr := gitdiff.Apply(&output, oldSrc, file)
if applyErr != nil {
return applyErr
}

writeErr := os.WriteFile(file.NewName[2:], output.Bytes(), file.NewMode.Perm())
if writeErr != nil {
return writeErr
}
}
}

return nil
}

func (runner *LintBEPHandler) outputLintResult(label string, f *buildeventstream.File) error {
lineResult, err := runner.readBEPFile(f)
if err != nil {
return err
}
Expand All @@ -158,7 +238,7 @@ func (runner *Linter) outputLintResult(label string, f *buildeventstream.File) e
return nil
}

func (runner *Linter) readLintResultFile(f *buildeventstream.File) (string, error) {
func (runner *LintBEPHandler) readBEPFile(f *buildeventstream.File) (string, error) {
// TODO: use f.GetContents()?

if strings.HasPrefix(f.GetUri(), "file://") {
Expand Down

0 comments on commit 06cea8b

Please sign in to comment.