Skip to content

Commit

Permalink
Enable warnings for the native cc rules & symbols (#1326)
Browse files Browse the repository at this point in the history
  • Loading branch information
hvadehra authored Jan 28, 2025
1 parent a84ab46 commit 1d76340
Show file tree
Hide file tree
Showing 8 changed files with 251 additions and 86 deletions.
51 changes: 41 additions & 10 deletions WARNINGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,25 @@ Warning categories supported by buildifier's linter:
* [`name-conventions`](#name-conventions)
* [`native-android`](#native-android)
* [`native-build`](#native-build)
* [`native-cc`](#native-cc)
* [`native-cc-binary`](#native-cc-binary)
* [`native-cc-common`](#native-cc-common)
* [`native-cc-debug-package-info`](#native-cc-debug-package-info)
* [`native-cc-fdo-prefetch-hints`](#native-cc-fdo-prefetch-hints)
* [`native-cc-fdo-profile`](#native-cc-fdo-profile)
* [`native-cc-import`](#native-cc-import)
* [`native-cc-info`](#native-cc-info)
* [`native-cc-library`](#native-cc-library)
* [`native-cc-memprof-profile`](#native-cc-memprof-profile)
* [`native-cc-objc-import`](#native-cc-objc-import)
* [`native-cc-objc-library`](#native-cc-objc-library)
* [`native-cc-propeller-optimize`](#native-cc-propeller-optimize)
* [`native-cc-proto`](#native-cc-proto)
* [`native-cc-shared-library`](#native-cc-shared-library)
* [`native-cc-shared-library-hint-info`](#native-cc-shared-library-hint-info)
* [`native-cc-shared-library-info`](#native-cc-shared-library-info)
* [`native-cc-test`](#native-cc-test)
* [`native-cc-toolchain`](#native-cc-toolchain)
* [`native-cc-toolchain-suite`](#native-cc-toolchain-suite)
* [`native-java-binary`](#native-java-binary)
* [`native-java-common`](#native-java-common)
* [`native-java-import`](#native-java-import)
Expand Down Expand Up @@ -698,18 +715,32 @@ as global symbols there.

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

## <a name="native-cc"></a>All C++ build rules should be loaded from Starlark
## <a name="native-cc-binary"></a><a name="native-cc-common"></a><a name="native-cc-debug-package-info"></a><a name="native-cc-fdo-prefetch-hints"></a><a name="native-cc-fdo-profile"></a><a name="native-cc-import"></a><a name="native-cc-info"></a><a name="native-cc-library"></a><a name="native-cc-memprof-profile"></a><a name="native-cc-objc-import"></a><a name="native-cc-objc-library"></a><a name="native-cc-propeller-optimize"></a><a name="native-cc-shared-library"></a><a name="native-cc-shared-library-hint-info"></a><a name="native-cc-shared-library-info"></a><a name="native-cc-test"></a><a name="native-cc-toolchain"></a><a name="native-cc-toolchain-suite"></a>All C++ build rules should be loaded from Starlark

* Category name: `native-cc`
* Flag in Bazel: [`--incompatible_load_cc_rules_from_bzl`](https://github.com/bazelbuild/bazel/issues/8743)
* Category names:
* `native-cc-binary`
* `native-cc-common`
* `native-cc-debug-package-info`
* `native-cc-fdo-prefetch-hints`
* `native-cc-fdo-profile`
* `native-cc-import`
* `native-cc-info`
* `native-cc-library`
* `native-cc-memprof-profile`
* `native-cc-objc-import`
* `native-cc-objc-library`
* `native-cc-propeller-optimize`
* `native-cc-shared-library`
* `native-cc-shared-library-hint-info`
* `native-cc-shared-library-info`
* `native-cc-test`
* `native-cc-toolchain`
* `native-cc-toolchain-suite`
* Flag in Bazel: [`--incompatible_autoload_externally`](https://github.com/bazelbuild/bazel/issues/23043)
* Automatic fix: yes
* [Suppress the warning](#suppress): `# buildifier: disable=native-cc`

The CC build rules should be loaded from Starlark.
* [Suppress the warning](#suppress): `# buildifier: disable=native-cc-binary`, `# buildifier: disable=native-cc-common`, `# buildifier: disable=native-cc-debug-package-info`, `# buildifier: disable=native-cc-fdo-prefetch-hints`, `# buildifier: disable=native-cc-fdo-profile`, `# buildifier: disable=native-cc-import`, `# buildifier: disable=native-cc-info`, `# buildifier: disable=native-cc-library`, `# buildifier: disable=native-cc-memprof-profile`, `# buildifier: disable=native-cc-objc-import`, `# buildifier: disable=native-cc-objc-library`, `# buildifier: disable=native-cc-propeller-optimize`, `# buildifier: disable=native-cc-shared-library`, `# buildifier: disable=native-cc-shared-library-hint-info`, `# buildifier: disable=native-cc-shared-library-info`, `# buildifier: disable=native-cc-test`, `# buildifier: disable=native-cc-toolchain`, `# buildifier: disable=native-cc-toolchain-suite`

Update: the plans for disabling native rules
[have been postponed](https://groups.google.com/g/bazel-discuss/c/XNvpWcge4AE/m/aJ-aQzszAwAJ),
at the moment it's not required to load Starlark rules.
The C++ build rules should be loaded from @rules_cc.

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

Expand Down
78 changes: 73 additions & 5 deletions buildifier/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,25 @@ func ExampleExample() {
// "name-conventions",
// "native-android",
// "native-build",
// "native-cc",
// "native-cc-binary",
// "native-cc-common",
// "native-cc-debug-package-info",
// "native-cc-fdo-prefetch-hints",
// "native-cc-fdo-profile",
// "native-cc-import",
// "native-cc-info",
// "native-cc-library",
// "native-cc-memprof-profile",
// "native-cc-objc-import",
// "native-cc-objc-library",
// "native-cc-propeller-optimize",
// "native-cc-proto",
// "native-cc-shared-library",
// "native-cc-shared-library-hint-info",
// "native-cc-shared-library-info",
// "native-cc-test",
// "native-cc-toolchain",
// "native-cc-toolchain-suite",
// "native-java-binary",
// "native-java-common",
// "native-java-import",
Expand Down Expand Up @@ -279,8 +296,25 @@ func TestValidate(t *testing.T) {
"name-conventions",
"native-android",
"native-build",
"native-cc",
"native-cc-binary",
"native-cc-common",
"native-cc-debug-package-info",
"native-cc-fdo-prefetch-hints",
"native-cc-fdo-profile",
"native-cc-import",
"native-cc-info",
"native-cc-library",
"native-cc-memprof-profile",
"native-cc-objc-import",
"native-cc-objc-library",
"native-cc-propeller-optimize",
"native-cc-proto",
"native-cc-shared-library",
"native-cc-shared-library-hint-info",
"native-cc-shared-library-info",
"native-cc-test",
"native-cc-toolchain",
"native-cc-toolchain-suite",
"native-java-binary",
"native-java-common",
"native-java-import",
Expand Down Expand Up @@ -361,8 +395,25 @@ func TestValidate(t *testing.T) {
"name-conventions",
"native-android",
"native-build",
"native-cc",
"native-cc-binary",
"native-cc-common",
"native-cc-debug-package-info",
"native-cc-fdo-prefetch-hints",
"native-cc-fdo-profile",
"native-cc-import",
"native-cc-info",
"native-cc-library",
"native-cc-memprof-profile",
"native-cc-objc-import",
"native-cc-objc-library",
"native-cc-propeller-optimize",
"native-cc-proto",
"native-cc-shared-library",
"native-cc-shared-library-hint-info",
"native-cc-shared-library-info",
"native-cc-test",
"native-cc-toolchain",
"native-cc-toolchain-suite",
"native-java-binary",
"native-java-common",
"native-java-import",
Expand Down Expand Up @@ -443,8 +494,25 @@ func TestValidate(t *testing.T) {
"name-conventions",
"native-android",
"native-build",
"native-cc",
"native-cc-binary",
"native-cc-common",
"native-cc-debug-package-info",
"native-cc-fdo-prefetch-hints",
"native-cc-fdo-profile",
"native-cc-import",
"native-cc-info",
"native-cc-library",
"native-cc-memprof-profile",
"native-cc-objc-import",
"native-cc-objc-library",
"native-cc-propeller-optimize",
"native-cc-proto",
"native-cc-shared-library",
"native-cc-shared-library-hint-info",
"native-cc-shared-library-info",
"native-cc-test",
"native-cc-toolchain",
"native-cc-toolchain-suite",
"native-java-binary",
"native-java-common",
"native-java-import",
Expand Down Expand Up @@ -490,7 +558,7 @@ func TestValidate(t *testing.T) {
"unused-variable",
"unsorted-dict-items",
}},
"warnings error": {options: "--warnings=native-cc,-print,-deprecated-function", wantErr: fmt.Errorf(`warning categories with modifiers ("+" or "-") can't be mixed with raw warning categories`)},
"warnings error": {options: "--warnings=native-py,-print,-deprecated-function", wantErr: fmt.Errorf(`warning categories with modifiers ("+" or "-") can't be mixed with raw warning categories`)},
} {
t.Run(name, func(t *testing.T) {
c := New()
Expand Down
19 changes: 18 additions & 1 deletion buildifier/integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,25 @@ cat > golden/.buildifier.example.json <<EOF
"name-conventions",
"native-android",
"native-build",
"native-cc",
"native-cc-binary",
"native-cc-common",
"native-cc-debug-package-info",
"native-cc-fdo-prefetch-hints",
"native-cc-fdo-profile",
"native-cc-import",
"native-cc-info",
"native-cc-library",
"native-cc-memprof-profile",
"native-cc-objc-import",
"native-cc-objc-library",
"native-cc-propeller-optimize",
"native-cc-proto",
"native-cc-shared-library",
"native-cc-shared-library-hint-info",
"native-cc-shared-library-info",
"native-cc-test",
"native-cc-toolchain",
"native-cc-toolchain-suite",
"native-java-binary",
"native-java-common",
"native-java-import",
Expand Down
18 changes: 2 additions & 16 deletions tables/tables.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,22 +228,8 @@ var AndroidNativeRules = []string{
// AndroidLoadPath is the load path for the Starlark Android Rules.
var AndroidLoadPath = "@rules_android//android:rules.bzl"

// CcNativeRules lists all C++ rules that are being migrated from Native to Starlark.
var CcNativeRules = []string{
"cc_binary",
"cc_test",
"cc_library",
"cc_import",
"fdo_prefetch_hints",
"fdo_profile",
"cc_toolchain",
"cc_toolchain_suite",
"objc_library",
"objc_import",
}

// CcLoadPath is the load path for the Starlark C++ Rules.
var CcLoadPath = "@rules_cc//cc:defs.bzl"
// CcLoadPathPrefix is the load path for the Starlark C++ Rules.
var CcLoadPathPrefix = "@rules_cc//cc"

// JavaLoadPathPrefix is the load package for the Starlark Java Rules.
var JavaLoadPathPrefix = "@rules_java//java"
Expand Down
29 changes: 21 additions & 8 deletions warn/docs/warnings.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -480,15 +480,28 @@ warnings: {
}

warnings: {
name: "native-cc"
name: "native-cc-binary"
name: "native-cc-common"
name: "native-cc-debug-package-info"
name: "native-cc-fdo-prefetch-hints"
name: "native-cc-fdo-profile"
name: "native-cc-import"
name: "native-cc-info"
name: "native-cc-library"
name: "native-cc-memprof-profile"
name: "native-cc-objc-import"
name: "native-cc-objc-library"
name: "native-cc-propeller-optimize"
name: "native-cc-shared-library"
name: "native-cc-shared-library-hint-info"
name: "native-cc-shared-library-info"
name: "native-cc-test"
name: "native-cc-toolchain"
name: "native-cc-toolchain-suite"
header: "All C++ build rules should be loaded from Starlark"
description:
"The CC build rules should be loaded from Starlark.\n\n"
"Update: the plans for disabling native rules\n"
"[have been postponed](https://groups.google.com/g/bazel-discuss/c/XNvpWcge4AE/m/aJ-aQzszAwAJ),\n"
"at the moment it's not required to load Starlark rules."
bazel_flag: "--incompatible_load_cc_rules_from_bzl"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/8743"
description: "The C++ build rules should be loaded from @rules_cc."
bazel_flag: "--incompatible_autoload_externally"
bazel_flag_link: "https://github.com/bazelbuild/bazel/issues/23043"
autofix: true
}

Expand Down
75 changes: 46 additions & 29 deletions warn/warn.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,35 +172,52 @@ var FileWarningMap = map[string]func(f *build.File) []*LinterFinding{

// MultiFileWarningMap lists the warnings that run on the whole file, but may use other files.
var MultiFileWarningMap = map[string]func(f *build.File, fileReader *FileReader) []*LinterFinding{
"deprecated-function": deprecatedFunctionWarning,
"git-repository": nativeGitRepositoryWarning,
"http-archive": nativeHTTPArchiveWarning,
"native-android": nativeAndroidRulesWarning,
"native-cc": nativeCcRulesWarning,
"native-java-binary": NativeJavaRulesWarning("java_binary"),
"native-java-import": NativeJavaRulesWarning("java_import"),
"native-java-library": NativeJavaRulesWarning("java_library"),
"native-java-plugin": NativeJavaRulesWarning("java_plugin"),
"native-java-test": NativeJavaRulesWarning("java_test"),
"native-java-package-config": NativeJavaToolchainRulesWarning("java_package_configuration"),
"native-java-runtime": NativeJavaToolchainRulesWarning("java_runtime"),
"native-java-toolchain": NativeJavaToolchainRulesWarning("java_toolchain"),
"native-java-common": NativeJavaSymbolsWarning("java_common", "java_common"),
"native-java-info": NativeJavaSymbolsWarning("JavaInfo", "java_info"),
"native-java-plugin-info": NativeJavaSymbolsWarning("JavaPluginInfo", "java_plugin_info"),
"native-proto": NativeProtoRulesWarning("proto_library"),
"native-java-proto": NativeProtoRulesWarning("java_proto_library"),
"native-java-lite-proto": NativeProtoRulesWarning("java_lite_proto_library"),
"native-cc-proto": NativeProtoRulesWarning("cc_proto_library"),
"native-proto-lang-toolchain": nativeProtoLangToolchainWarning,
"native-proto-info": nativeProtoSymbolsWarning("ProtoInfo", "proto_info.bzl"),
"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": NativeShellRulesWarning("sh_binary"),
"native-sh-library": NativeShellRulesWarning("sh_library"),
"native-sh-test": NativeShellRulesWarning("sh_test"),
"unnamed-macro": unnamedMacroWarning,
"deprecated-function": deprecatedFunctionWarning,
"git-repository": nativeGitRepositoryWarning,
"http-archive": nativeHTTPArchiveWarning,
"native-android": nativeAndroidRulesWarning,
"native-cc-binary": NativeCcRulesWarning("cc_binary"),
"native-cc-import": NativeCcRulesWarning("cc_import"),
"native-cc-library": NativeCcRulesWarning("cc_library"),
"native-cc-objc-import": NativeCcRulesWarning("objc_import"),
"native-cc-objc-library": NativeCcRulesWarning("objc_library"),
"native-cc-shared-library": NativeCcRulesWarning("cc_shared_library"),
"native-cc-test": NativeCcRulesWarning("cc_test"),
"native-cc-toolchain": NativeCcToolchainRulesWarning("cc_toolchain"),
"native-cc-toolchain-suite": NativeCcToolchainRulesWarning("cc_toolchain_suite"),
"native-cc-fdo-prefetch-hints": NativeCcToolchainRulesWarning("fdo_prefetch_hints"),
"native-cc-fdo-profile": NativeCcToolchainRulesWarning("fdo_profile"),
"native-cc-memprof-profile": NativeCcToolchainRulesWarning("memprof_profile"),
"native-cc-propeller-optimize": NativeCcToolchainRulesWarning("propeller_optimize"),
"native-cc-common": NativeCcSymbolsWarning("cc_common", "cc_common"),
"native-cc-debug-package-info": NativeCcSymbolsWarning("DebugPackageInfo", "debug_package_info"),
"native-cc-info": NativeCcSymbolsWarning("CcInfo", "cc_info"),
"native-cc-shared-library-info": NativeCcSymbolsWarning("CcSharedLibraryInfo", "cc_shared_library_info"),
"native-cc-shared-library-hint-info": NativeCcSymbolsWarning("CcSharedLibraryHintInfo", "cc_shared_library_hint_info"),
"native-java-binary": NativeJavaRulesWarning("java_binary"),
"native-java-import": NativeJavaRulesWarning("java_import"),
"native-java-library": NativeJavaRulesWarning("java_library"),
"native-java-plugin": NativeJavaRulesWarning("java_plugin"),
"native-java-test": NativeJavaRulesWarning("java_test"),
"native-java-package-config": NativeJavaToolchainRulesWarning("java_package_configuration"),
"native-java-runtime": NativeJavaToolchainRulesWarning("java_runtime"),
"native-java-toolchain": NativeJavaToolchainRulesWarning("java_toolchain"),
"native-java-common": NativeJavaSymbolsWarning("java_common", "java_common"),
"native-java-info": NativeJavaSymbolsWarning("JavaInfo", "java_info"),
"native-java-plugin-info": NativeJavaSymbolsWarning("JavaPluginInfo", "java_plugin_info"),
"native-proto": NativeProtoRulesWarning("proto_library"),
"native-java-proto": NativeProtoRulesWarning("java_proto_library"),
"native-java-lite-proto": NativeProtoRulesWarning("java_lite_proto_library"),
"native-cc-proto": NativeProtoRulesWarning("cc_proto_library"),
"native-proto-lang-toolchain": nativeProtoLangToolchainWarning,
"native-proto-info": nativeProtoSymbolsWarning("ProtoInfo", "proto_info.bzl"),
"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": NativeShellRulesWarning("sh_binary"),
"native-sh-library": NativeShellRulesWarning("sh_library"),
"native-sh-test": NativeShellRulesWarning("sh_test"),
"unnamed-macro": unnamedMacroWarning,
}

// nonDefaultWarnings contains warnings that are enabled by default because they're not applicable
Expand Down
31 changes: 27 additions & 4 deletions warn/warn_bazel_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -728,11 +728,34 @@ func nativeAndroidRulesWarning(f *build.File, fileReader *FileReader) []*LinterF
return NotLoadedFunctionUsageCheck(f, fileReader, tables.AndroidNativeRules, tables.AndroidLoadPath)
}

func nativeCcRulesWarning(f *build.File, fileReader *FileReader) []*LinterFinding {
if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
return nil
// NativeCcRulesWarning produces a warning for missing loads of cc rules
func NativeCcRulesWarning(rule string) func(f *build.File, fileReader *FileReader) []*LinterFinding {
return func(f *build.File, fileReader *FileReader) []*LinterFinding {
if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
return nil
}
return NotLoadedFunctionUsageCheck(f, fileReader, []string{rule}, tables.CcLoadPathPrefix+":"+rule+".bzl")
}
}

// NativeCcToolchainRulesWarning produces a warning for missing loads of cc toolchain rules
func NativeCcToolchainRulesWarning(rule string) func(f *build.File, fileReader *FileReader) []*LinterFinding {
return func(f *build.File, fileReader *FileReader) []*LinterFinding {
if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
return nil
}
return NotLoadedFunctionUsageCheck(f, fileReader, []string{rule}, tables.CcLoadPathPrefix+"/toolchains:"+rule+".bzl")
}
}

// NativeCcSymbolsWarning produces a warning for missing loads of cc top-level symbols
func NativeCcSymbolsWarning(symbol string, bzlfile string) func(f *build.File, fileReader *FileReader) []*LinterFinding {
return func(f *build.File, fileReader *FileReader) []*LinterFinding {
if f.Type != build.TypeBzl && f.Type != build.TypeBuild {
return nil
}
return NotLoadedSymbolUsageCheck(f, fileReader, []string{symbol}, tables.CcLoadPathPrefix+"/common:"+bzlfile+".bzl")
}
return NotLoadedFunctionUsageCheck(f, fileReader, tables.CcNativeRules, tables.CcLoadPath)
}

// NativeJavaRulesWarning produces a warning for missing loads of java rules
Expand Down
Loading

0 comments on commit 1d76340

Please sign in to comment.