Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rules_shell fixing #1303

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
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