Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions buildifier/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,23 @@ When the `--format` flag is provided, buildifier always returns `0` unless there
failures or wrong input parameters, this means the output can be parsed as JSON, and its `success`
field should be used to determine whether the diagnostics result is positive.

## File diagnostics in SARIF

Buildifier can also emit its findings as a [SARIF 2.1.0](https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html)
report, triggered by `--format=sarif` (only works in combination with `--mode=check`). If used in
combination with `-v`, the output will be indented.

* Every lint warning becomes a result whose `ruleId` is the warning category, with a region
(line and column) and level `warning`.
* A file that is not correctly formatted yields a file-level result (no region) with rule `format`.
* A file that cannot be parsed yields a result with level `error` and rule `parse-error`.

Example:

```bash
buildifier --mode=check --lint=warn --format=sarif -r . > buildifier.sarif
```

## Symlink Safety

By default, Buildifier will not write to symlinks pointing outside of the Bazel workspace to prevent unintended path traversal.
Expand Down
4 changes: 2 additions & 2 deletions buildifier/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ type Config struct {
// repo (for REPO.bazel files), vendor (for VENDOR.bazel files),
// or auto (default, based on the filename)
InputType string `json:"type,omitempty"`
// Format sets the diagnostics format: text or json (default text)
// Format sets the diagnostics format: text, json or sarif (default text)
Format string `json:"format,omitempty"`
// Mode determines the formatting mode: check, diff, or fix (default fix)
Mode string `json:"mode,omitempty"`
Expand Down Expand Up @@ -179,7 +179,7 @@ func (c *Config) FlagSet(name string, errorHandling flag.ErrorHandling) *flag.Fl
flags.BoolVar(&c.Recursive, "r", c.Recursive, "find starlark files recursively")
flags.BoolVar(&c.MultiDiff, "multi_diff", c.MultiDiff, "the command specified by the -diff_command flag can diff multiple files in the style of tkdiff (default false)")
flags.StringVar(&c.Mode, "mode", c.Mode, "formatting mode: check, diff, or fix (default fix)")
flags.StringVar(&c.Format, "format", c.Format, "diagnostics format: text or json (default text)")
flags.StringVar(&c.Format, "format", c.Format, "diagnostics format: text, json or sarif (default text)")
flags.StringVar(&c.DiffCommand, "diff_command", c.DiffCommand, "command to run when the formatting mode is diff (default uses the BUILDIFIER_DIFF, BUILDIFIER_MULTIDIFF, and DISPLAY environment variables to create the diff command)")
flags.StringVar(&c.Lint, "lint", c.Lint, "lint mode: off, warn, or fix (default off)")
flags.StringVar(&c.Warnings, "warnings", c.Warnings, "comma-separated warnings used in the lint mode or \"all\"")
Expand Down
6 changes: 4 additions & 2 deletions buildifier/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func ExampleFlagSet() {
// d: alias for -mode=diff ("false")
// diff_command: command to run when the formatting mode is diff (default uses the BUILDIFIER_DIFF, BUILDIFIER_MULTIDIFF, and DISPLAY environment variables to create the diff command) ("")
// disable_symlink_safety: per default Buildifier will not write to symlinks pointing outside of the Bazel workspace. Setting this to true will disable this behavior ("false")
// format: diagnostics format: text or json (default text) ("")
// format: diagnostics format: text, json or sarif (default text) ("")
// help: print usage information ("false")
// lint: lint mode: off, warn, or fix (default off) ("")
// mode: formatting mode: check, diff, or fix (default fix) ("")
Expand Down Expand Up @@ -256,8 +256,10 @@ func TestValidate(t *testing.T) {
"lint fix error": {options: "--lint=fix --mode=check", wantErr: fmt.Errorf("--lint=fix is only compatible with --mode=fix")},
"format mode error": {options: "--mode=fix --format=text", wantErr: fmt.Errorf("cannot specify --format without --mode=check")},
"format text": {options: "--mode=check --format=text"},
"format sarif": {options: "--mode=check --format=sarif"},
"format sarif error": {options: "--mode=fix --format=sarif", wantErr: fmt.Errorf("cannot specify --format without --mode=check")},
"format json": {options: "--mode=check --format=json"},
"format error": {options: "--mode=check --format=foo", wantErr: fmt.Errorf("unrecognized format foo; valid types are text, json")},
"format error": {options: "--mode=check --format=foo", wantErr: fmt.Errorf("unrecognized format foo; valid types are text, json, sarif")},
"type build": {options: "--type=build"},
"type bzl": {options: "--type=bzl"},
"type workspace": {options: "--type=workspace"},
Expand Down
4 changes: 2 additions & 2 deletions buildifier/config/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ func ValidateFormat(format, mode *string) error {
case "":
return nil

case "text", "json":
case "text", "json", "sarif":
if *mode != "check" {
return fmt.Errorf("cannot specify --format without --mode=check")
}

default:
return fmt.Errorf("unrecognized format %s; valid types are text, json", *format)
return fmt.Errorf("unrecognized format %s; valid types are text, json, sarif", *format)
}
return nil
}
Expand Down
267 changes: 266 additions & 1 deletion buildifier/integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ test_lint "all" "--warnings=all" "test_dir/fixed_golden_all.bzl" "$error_docstri
test_lint "cfg" "--warnings=attr-cfg" "test_dir/fixed_golden_cfg.bzl" "$error_cfg" 0
test_lint "custom" "--warnings=-bzl-visibility,-integer-division,+unsorted-dict-items" "test_dir/fixed_golden_dict_cfg.bzl" "$error_docstring"$'\n'"$error_dict"$'\n'"$error_cfg" 1

# Test --format=json
# Test --format=json and --format=sarif

mkdir test_dir/json
cp test_dir/to_fix.bzl test_dir/json
Expand Down Expand Up @@ -658,6 +658,259 @@ cat > golden/json_report_invalid_file_golden <<EOF
}
EOF

cat > golden/sarif_report_golden <<'EOF'
{
"$schema": "https://json.schemastore.org/sarif-2.1.0.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "buildifier",
"informationUri": "https://github.com/bazel-contrib/buildtools/tree/main/buildifier",
"rules": [
{
"id": "bzl-visibility",
"helpUri": "https://github.com/bazel-contrib/buildtools/blob/main/WARNINGS.md#bzl-visibility"
},
{
"id": "integer-division",
"helpUri": "https://github.com/bazel-contrib/buildtools/blob/main/WARNINGS.md#integer-division"
},
{
"id": "attr-cfg",
"helpUri": "https://github.com/bazel-contrib/buildtools/blob/main/WARNINGS.md#attr-cfg"
},
{
"id": "format",
"shortDescription": {
"text": "The file is not formatted according to buildifier's rules."
}
}
]
}
},
"columnKind": "unicodeCodePoints",
"results": [
{
"ruleId": "bzl-visibility",
"ruleIndex": 0,
"level": "warning",
"message": {
"text": "Module \"//foo/bar/internal/baz:module.bzl\" can only be loaded from files located inside \"//foo/bar\", not from \"//json/to_fix.bzl\"."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "to_fix.bzl"
},
"region": {
"startLine": 1,
"startColumn": 6,
"endLine": 1,
"endColumn": 41
}
}
}
]
},
{
"ruleId": "integer-division",
"ruleIndex": 1,
"level": "warning",
"message": {
"text": "The \"/\" operator for integer division is deprecated in favor of \"//\"."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "to_fix.bzl"
},
"region": {
"startLine": 4,
"startColumn": 5,
"endLine": 4,
"endColumn": 10
}
}
}
]
},
{
"ruleId": "attr-cfg",
"ruleIndex": 2,
"level": "warning",
"message": {
"text": "cfg = \"data\" for attr definitions has no effect and should be removed."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "to_fix.bzl"
},
"region": {
"startLine": 6,
"startColumn": 15,
"endLine": 6,
"endColumn": 27
}
}
}
]
},
{
"ruleId": "format",
"ruleIndex": 3,
"level": "warning",
"message": {
"text": "The file needs to be reformatted."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "to_fix_2.bzl"
}
}
}
]
},
{
"ruleId": "format",
"ruleIndex": 3,
"level": "warning",
"message": {
"text": "The file needs to be reformatted."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "to_fix_3.bzl"
}
}
}
]
}
]
}
]
}
EOF

cat > golden/sarif_report_small_golden <<'EOF'
{
"$schema": "https://json.schemastore.org/sarif-2.1.0.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "buildifier",
"informationUri": "https://github.com/bazel-contrib/buildtools/tree/main/buildifier",
"rules": []
}
},
"columnKind": "unicodeCodePoints",
"results": []
}
]
}
EOF

cat > golden/sarif_report_stdin_golden <<'EOF'
{
"$schema": "https://json.schemastore.org/sarif-2.1.0.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "buildifier",
"informationUri": "https://github.com/bazel-contrib/buildtools/tree/main/buildifier",
"rules": [
{
"id": "format",
"shortDescription": {
"text": "The file is not formatted according to buildifier's rules."
}
}
]
}
},
"columnKind": "unicodeCodePoints",
"results": [
{
"ruleId": "format",
"ruleIndex": 0,
"level": "warning",
"message": {
"text": "The file needs to be reformatted."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "%3Cstdin%3E"
}
}
}
]
}
]
}
]
}
EOF

cat > golden/sarif_report_invalid_file_golden <<'EOF'
{
"$schema": "https://json.schemastore.org/sarif-2.1.0.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "buildifier",
"informationUri": "https://github.com/bazel-contrib/buildtools/tree/main/buildifier",
"rules": [
{
"id": "parse-error",
"shortDescription": {
"text": "The file is not a valid Starlark file."
}
}
]
}
},
"columnKind": "unicodeCodePoints",
"results": [
{
"ruleId": "parse-error",
"ruleIndex": 0,
"level": "error",
"message": {
"text": "The file could not be parsed."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "foo.bar"
}
}
}
]
}
]
}
]
}
EOF

cd test_dir/json

$buildifier --mode=check --format=json --lint=warn --warnings=-module-docstring -v to_fix.bzl to_fix_2.bzl to_fix_3.bzl to_fix_4.bzl > json_report
Expand All @@ -672,6 +925,18 @@ diff -u json_report ../../golden/json_report_stdin_golden || die "$1: wrong cons
$buildifier --mode=check --format=json --lint=warn --warnings=-module-docstring -v to_fix_4.bzl foo.bar > json_report
diff -u json_report ../../golden/json_report_invalid_file_golden || die "$1: wrong console output for --mode=check --format=json --lint=warn with an invalid file"

$buildifier --mode=check --format=sarif --lint=warn --warnings=-module-docstring -v to_fix.bzl to_fix_2.bzl to_fix_3.bzl to_fix_4.bzl > sarif_report
diff -u sarif_report ../../golden/sarif_report_golden || die "$1: wrong console output for --mode=check --format=sarif --lint=warn with many files"

$buildifier --mode=check --format=sarif --lint=warn --warnings=-module-docstring -v to_fix_4.bzl > sarif_report
diff -u sarif_report ../../golden/sarif_report_small_golden || die "$1: wrong console output for --mode=check --format=sarif --lint=warn with a single file"

$buildifier --mode=check --format=sarif --lint=warn --warnings=-module-docstring -v < to_fix_2.bzl > sarif_report
diff -u sarif_report ../../golden/sarif_report_stdin_golden || die "$1: wrong console output for --mode=check --format=sarif --lint=warn with stdin"

$buildifier --mode=check --format=sarif --lint=warn --warnings=-module-docstring -v to_fix_4.bzl foo.bar > sarif_report
diff -u sarif_report ../../golden/sarif_report_invalid_file_golden || die "$1: wrong console output for --mode=check --format=sarif --lint=warn with an invalid file"

cd ../..

# Test the multifile functionality
Expand Down
6 changes: 5 additions & 1 deletion buildifier/utils/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go_library(
name = "utils",
srcs = [
"diagnostics.go",
"sarif.go",
"tempfile.go",
"utils.go",
],
Expand All @@ -17,7 +18,10 @@ go_library(

go_test(
name = "utils_test",
srcs = ["utils_test.go"],
srcs = [
"sarif_test.go",
"utils_test.go",
],
embed = [":utils"],
)

Expand Down
Loading
Loading