Skip to content

Commit

Permalink
feat: added --fixes flag to Aspect CLI lint command (#7015)
Browse files Browse the repository at this point in the history
Feature request from Figma where they have a custom eslint.bzl and
requesting the patches results in two actions on CI.

Also fixes flag registration so they are "no-able" to match expected
behaviour of other flags: `--nofixes`, `--noreport`, etc...

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

New `--fixes` flag added to lint command in the the Aspect CLI which
defaults to true. Users can now set `--nofixes` to turn off requesting
the lint patch output group (`rules_lint_patch`). Requesting the report
outputs groups (`rules_lint_human` and `rules_lint_machine`) is still
controlled by the `--report` and `--machine` flags.

### Test plan

- Covered by existing test cases
- New test cases added

GitOrigin-RevId: 8351b619b16979d089dd600db9065d8cd48f699d
  • Loading branch information
gregmagolan authored and jbedard committed Oct 23, 2024
1 parent ef386d5 commit 39f5538
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 10 deletions.
56 changes: 56 additions & 0 deletions integration_tests/aspect/lint_test.bats
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,64 @@ EOF
run aspect lint //:all
assert_failure
assert_output --partial "SC2068"
assert_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out"
assert_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out.exit_code"
assert_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.patch"

run aspect lint //:all --@aspect_rules_lint//lint:fail_on_violation
assert_failure
assert_output --partial "SC2068"
# if bazel exits non-zero then outputs are not reported
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out.exit_code"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.patch"
}

@test 'aspect lint should not output patch files if --nofixes flag is set' {
run aspect lint //:all --nofixes
assert_failure
assert_output --partial "SC2068"
assert_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out"
assert_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out.exit_code"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.patch"

run aspect lint //:all --nofixes --@aspect_rules_lint//lint:fail_on_violation
assert_failure
assert_output --partial "SC2068"
# if bazel exits non-zero then outputs are not reported
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out.exit_code"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.patch"
}

@test 'aspect lint should not output report files if --noreport flag is set' {
run aspect lint //:all --noreport
assert_success
refute_output --partial "SC2068"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out.exit_code"
assert_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.patch"

run aspect lint //:all --noreport --@aspect_rules_lint//lint:fail_on_violation
assert_success
refute_output --partial "SC2068"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out.exit_code"
assert_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.patch"
}

@test 'aspect lint should always pass if --nofixes and --noreport flags are both set' {
run aspect lint //:all --nofixes --noreport
assert_success
refute_output --partial "SC2068"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out.exit_code"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.patch"

run aspect lint //:all --nofixes --noreport --@aspect_rules_lint//lint:fail_on_violation
assert_success
refute_output --partial "SC2068"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.out.exit_code"
refute_output --partial "bazel-bin/shell.AspectRulesLintShellCheck.patch"
}
22 changes: 12 additions & 10 deletions pkg/aspect/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,12 @@ func New(
}
}

func AddFlags(flags *pflag.FlagSet) {
flags.Bool("fix", false, "Apply all patch fixes for lint violations")
flags.Bool("diff", false, "Show unified diff instead of diff stats for fixes")
flags.Bool("report", true, "Output lint reports")
flags.Bool("machine", false, "Request the machine readable output from linters")
func AddFlags(flagSet *pflag.FlagSet) {
flags.RegisterNoableBoolP(flagSet, "fix", "", false, "Auto-apply all fixes")
flags.RegisterNoableBoolP(flagSet, "diff", "", false, "Show unified diff instead of diff stats for fixes")
flags.RegisterNoableBoolP(flagSet, "fixes", "", true, "Request fixes from linters (where supported)")
flags.RegisterNoableBoolP(flagSet, "report", "", true, "Request lint reports from linters")
flags.RegisterNoableBoolP(flagSet, "machine", "", false, "Request machine readable lint reports from linters (where supported)")
}

// TODO: hoist this to a flags package so it can be used by other commands that require this functionality
Expand Down Expand Up @@ -159,8 +160,9 @@ lint:
// Get values of lint command specific flags
applyAll, _ := cmd.Flags().GetBool("fix")
showDiff, _ := cmd.Flags().GetBool("diff")
printReport, _ := cmd.Flags().GetBool("report")
machine, _ := cmd.Flags().GetBool("machine")
requestFixes, _ := cmd.Flags().GetBool("fixes")
requestReports, _ := cmd.Flags().GetBool("report")
machineReports, _ := cmd.Flags().GetBool("machine")

// Separate out the lint command specific flags from the list of args to
// pass to `bazel build`
Expand All @@ -181,12 +183,12 @@ lint:

// Don't request report and patch files in a mode where we don't need them
outputGroups := []string{}
if printReport || applyAll || isInteractiveMode {
if requestFixes || applyAll {
bazelCmd = append(bazelCmd, "--@aspect_rules_lint//lint:fix")
outputGroups = append(outputGroups, LINT_PATCH_GROUP)
}
if printReport {
if machine {
if requestReports {
if machineReports {
outputGroups = append(outputGroups, LINT_REPORT_GROUP_MACHINE)
} else {
outputGroups = append(outputGroups, LINT_REPORT_GROUP_HUMAN)
Expand Down

0 comments on commit 39f5538

Please sign in to comment.