Skip to content

Commit

Permalink
Add rules_shell fixing
Browse files Browse the repository at this point in the history
Currently these are split into 3 rules because 1 rule cannot add 3 load
statements and this repo doesn't have a defs.bzl that exposes them all
  • Loading branch information
keith committed Dec 6, 2024
1 parent a0444eb commit 5445e37
Show file tree
Hide file tree
Showing 7 changed files with 168 additions and 1 deletion.
33 changes: 33 additions & 0 deletions WARNINGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ Warning categories supported by buildifier's linter:
* [`native-proto-lang-toolchain`](#native-proto-lang-toolchain)
* [`native-proto-lang-toolchain-info`](#native-proto-lang-toolchain-info)
* [`native-py`](#native-py)
* [`native-sh-binary`](#native-sh-binary)
* [`native-sh-library`](#native-sh-library)
* [`native-sh-test`](#native-sh-test)
* [`no-effect`](#no-effect)
* [`out-of-order-load`](#out-of-order-load)
* [`output-group`](#output-group)
Expand Down Expand Up @@ -833,6 +836,36 @@ at the moment it's not required to load Starlark rules.

--------------------------------------------------------------------------------

## <a name="native-sh-binary"></a>sh_binary build rules should be loaded from Starlark

* Category name: `native-sh-binary`
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-sh-binary`

The sh_binary build rules should be loaded from Starlark.

--------------------------------------------------------------------------------

## <a name="native-sh-library"></a>sh_library build rules should be loaded from Starlark

* Category name: `native-sh-library`
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-sh-library`

The sh_library build rules should be loaded from Starlark.

--------------------------------------------------------------------------------

## <a name="native-sh-test"></a>sh_test build rules should be loaded from Starlark

* Category name: `native-sh-test`
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-sh-test`

The sh_test build rules should be loaded from Starlark.

--------------------------------------------------------------------------------

## <a name="no-effect"></a>Expression result is not used

* Category name: `no-effect`
Expand Down
12 changes: 12 additions & 0 deletions buildifier/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ func ExampleExample() {
// "native-proto-lang-toolchain",
// "native-proto-lang-toolchain-info",
// "native-py",
// "native-sh-binary",
// "native-sh-library",
// "native-sh-test",
// "no-effect",
// "output-group",
// "overly-nested-depset",
Expand Down Expand Up @@ -278,6 +281,9 @@ func TestValidate(t *testing.T) {
"native-proto-lang-toolchain",
"native-proto-lang-toolchain-info",
"native-py",
"native-sh-binary",
"native-sh-library",
"native-sh-test",
"no-effect",
"output-group",
"overly-nested-depset",
Expand Down Expand Up @@ -347,6 +353,9 @@ func TestValidate(t *testing.T) {
"native-proto-lang-toolchain",
"native-proto-lang-toolchain-info",
// "native-py",
"native-sh-binary",
"native-sh-library",
"native-sh-test",
"no-effect",
"output-group",
"overly-nested-depset",
Expand Down Expand Up @@ -415,6 +424,9 @@ func TestValidate(t *testing.T) {
"native-proto-lang-toolchain",
"native-proto-lang-toolchain-info",
// "native-py",
"native-sh-binary",
"native-sh-library",
"native-sh-test",
"no-effect",
"output-group",
"overly-nested-depset",
Expand Down
3 changes: 3 additions & 0 deletions buildifier/integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ cat > golden/.buildifier.example.json <<EOF
"native-proto-lang-toolchain",
"native-proto-lang-toolchain-info",
"native-py",
"native-sh-binary",
"native-sh-library",
"native-sh-test",
"no-effect",
"output-group",
"overly-nested-depset",
Expand Down
21 changes: 21 additions & 0 deletions warn/docs/warnings.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,27 @@ warnings: {
autofix: true
}

warnings: {
name: "native-sh-binary"
header: "sh_binary build rules should be loaded from Starlark"
description: "The sh_binary build rules should be loaded from Starlark."
autofix: true
}

warnings: {
name: "native-sh-library"
header: "sh_library build rules should be loaded from Starlark"
description: "The sh_library build rules should be loaded from Starlark."
autofix: true
}

warnings: {
name: "native-sh-test"
header: "sh_test build rules should be loaded from Starlark"
description: "The sh_test build rules should be loaded from Starlark."
autofix: true
}

warnings: {
name: "no-effect"
header: "Expression result is not used"
Expand Down
3 changes: 3 additions & 0 deletions warn/warn.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ var FileWarningMap = map[string]func(f *build.File) []*LinterFinding{
"native-proto-common": nativeProtoSymbolsWarning("proto_common", "proto_common.bzl"),
"native-proto-lang-toolchain-info": nativeProtoSymbolsWarning("ProtoLangToolchainInfo", "proto_lang_toolchain_info.bzl"),
"native-py": nativePyRulesWarning,
"native-sh-binary": nativeShBinaryWarning,
"native-sh-library": nativeShLibraryWarning,
"native-sh-test": nativeShTestWarning,
"no-effect": noEffectWarning,
"output-group": outputGroupWarning,
"overly-nested-depset": overlyNestedDepsetWarning,
Expand Down
22 changes: 21 additions & 1 deletion warn/warn_bazel_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,27 @@ func nativeProtoSymbolsWarning(symbol string, bzlfile string) func(f *build.File
}
}

func nativeShBinaryWarning(f *build.File) []*LinterFinding {
if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
return nil
}
return NotLoadedFunctionUsageCheck(f, []string{"sh_binary"}, "@rules_shell//shell:sh_binary.bzl")
}

func nativeShLibraryWarning(f *build.File) []*LinterFinding {
if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
return nil
}
return NotLoadedFunctionUsageCheck(f, []string{"sh_library"}, "@rules_shell//shell:sh_library.bzl")
}

func nativeShTestWarning(f *build.File) []*LinterFinding {
if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
return nil
}
return NotLoadedFunctionUsageCheck(f, []string{"sh_test"}, "@rules_shell//shell:sh_test.bzl")
}

func contextArgsAPIWarning(f *build.File) []*LinterFinding {
if f.Type != build.TypeBzl {
return nil
Expand Down Expand Up @@ -1110,7 +1131,6 @@ func providerParamsWarning(f *build.File) []*LinterFinding {
return findings
}


func attrNameWarning(f *build.File, names []string) []*LinterFinding {
if f.Type != build.TypeBzl {
return nil
Expand Down
75 changes: 75 additions & 0 deletions warn/warn_bazel_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,81 @@ def macro():
scopeBzl|scopeBuild)
}

func TestNativeShBinaryWarning(t *testing.T) {
checkFindingsAndFix(t, "native-sh-binary", `
"""My file"""
def macro():
native.sh_binary()
sh_binary()
`, `
"""My file"""
load("@rules_shell//shell:sh_binary.bzl", "sh_binary")
def macro():
sh_binary()
sh_binary()
`,
[]string{
fmt.Sprintf(`:4: Function "sh_binary" is not global anymore and needs to be loaded from "@rules_shell//shell:sh_binary.bzl".`),
fmt.Sprintf(`:6: Function "sh_binary" is not global anymore and needs to be loaded from "@rules_shell//shell:sh_binary.bzl".`),
},
scopeBzl|scopeBuild)
}

func TestNativeShLibraryWarning(t *testing.T) {
checkFindingsAndFix(t, "native-sh-library", `
"""My file"""
def macro():
native.sh_library()
sh_library()
`, `
"""My file"""
load("@rules_shell//shell:sh_library.bzl", "sh_library")
def macro():
sh_library()
sh_library()
`,
[]string{
fmt.Sprintf(`:4: Function "sh_library" is not global anymore and needs to be loaded from "@rules_shell//shell:sh_library.bzl".`),
fmt.Sprintf(`:6: Function "sh_library" is not global anymore and needs to be loaded from "@rules_shell//shell:sh_library.bzl".`),
},
scopeBzl|scopeBuild)
}

func TestNativeShTestWarning(t *testing.T) {
checkFindingsAndFix(t, "native-sh-test", `
"""My file"""
def macro():
native.sh_test()
sh_test()
`, `
"""My file"""
load("@rules_shell//shell:sh_test.bzl", "sh_test")
def macro():
sh_test()
sh_test()
`,
[]string{
fmt.Sprintf(`:4: Function "sh_test" is not global anymore and needs to be loaded from "@rules_shell//shell:sh_test.bzl".`),
fmt.Sprintf(`:6: Function "sh_test" is not global anymore and needs to be loaded from "@rules_shell//shell:sh_test.bzl".`),
},
scopeBzl|scopeBuild)
}

func TestKeywordParameters(t *testing.T) {
checkFindingsAndFix(t, "keyword-positional-params", `
foo(key = value)
Expand Down

0 comments on commit 5445e37

Please sign in to comment.