diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4a5eb316d..f5b44263a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -57,6 +57,8 @@ jobs: repository-cache: true - name: Scan run: bazel run //:periphery -- scan --bazel --quiet --strict --baseline baselines/${{ matrix.baseline }} + - name: Test Bazel rules + run: bazel test //bazel/tests/... macOS: name: macOS strategy: diff --git a/MODULE.bazel b/MODULE.bazel index 78421bb69..a9e15e6c2 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -27,3 +27,4 @@ use_repo(use_extension("//bazel:generated.bzl", "generated"), "periphery_generat # Bazel dev dependencies bazel_dep(name = "buildifier_prebuilt", version = "8.2.1.1", dev_dependency = True) +bazel_dep(name = "rules_shell", version = "0.6.1", dev_dependency = True) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 919b464ee..b1a7c258e 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -496,8 +496,8 @@ }, "@@swift-index-store+//:repositories.bzl%bzlmod_deps": { "general": { - "bzlTransitiveDigest": "5SgzvK5bW1AjBEGcIZ+V8t1hdrKQxcL5liC+blXrOeo=", - "usagesDigest": "6gSsMFoZD8MOT1d3qLGp8z92CBXKqQI0zULrhGHXrow=", + "bzlTransitiveDigest": "lQ53e8CJVUhAUlLtQHBMA6CHnfoEVqLeN/PpMp0m9jw=", + "usagesDigest": "f8v90Z0jhAD//t5+/A2VQp2N8NISaI/oFgntRKckdYI=", "recordedInputs": [ "REPO_MAPPING:swift-index-store+,bazel_tools bazel_tools" ], @@ -509,6 +509,10 @@ "sha256": "da69bab932357a817aa0756e400be86d7156040bfbea8eded7a3acc529320731", "build_file_content": "\nload(\"@build_bazel_rules_apple//apple:apple.bzl\", \"apple_static_xcframework_import\")\n\napple_static_xcframework_import(\n name = \"libIndexStore\",\n visibility = [\"//visibility:public\"],\n xcframework_imports = glob([\"libIndexStore.xcframework/**\"]),\n)\n " } + }, + "LinuxIndexStore": { + "repoRuleId": "@@swift-index-store+//:repositories.bzl%_linux_indexstore", + "attributes": {} } } } diff --git a/README.md b/README.md index 652f8ae63..052b5c9c7 100644 --- a/README.md +++ b/README.md @@ -472,6 +472,71 @@ Periphery's generated scan rule follows embedded bundle and plugin edges transit > > You can disable this behavior with the `--bazel-check-visibility` option. You must ensure the necessary targets are visible to Periphery's generated package, for example by adding the `@@+generated+periphery//bazel:generated` visibility label to your targets. +#### Declaring scan targets in your `BUILD.bazel` + +For deeper integration — for example, wiring Periphery into CI as a `bazel test`, or scanning a specific subset of your build graph without invoking the `--bazel` driver — you can declare scan targets directly in your `BUILD.bazel` files. Three rules are provided: + +- `scan` — an executable target that prints results when run with `bazel run`. +- `scan_test` — a test target that exits non-zero when unused code is found (via `--strict`), so `bazel test` fails. Use this for CI. +- `scan_report` — a build target that runs Periphery at build time and writes the formatted report to a file output. Use this when you need to feed the report into another rule via `data` deps or `srcs`. + +All three rules produce the same scan results as `bazel run @periphery -- scan --bazel`. Apply them to your top-level targets (applications, tests, command-line tools, etc.): + +```python +load("@periphery//bazel:rules.bzl", "scan", "scan_report", "scan_test") + +scan( + name = "scan", + testonly = True, + config = ".periphery.yml", + deps = [ + "//App:MyApp", + "//Tests:MyAppTests", + ], +) + +scan_test( + name = "scan_test", + config = ".periphery.yml", + deps = [ + "//App:MyApp", + "//Tests:MyAppTests", + ], +) + +scan_report( + name = "scan_report", + config = ".periphery.yml", + format = "json", + deps = [ + "//App:MyApp", + "//Tests:MyAppTests", + ], +) +``` + +Run the scan locally with: + +```sh +bazel run //:scan +``` + +Fail the build in CI with: + +```sh +bazel test //:scan_test +``` + +Build a report file: + +```sh +bazel build //:scan_report +``` + +The report file is exposed as the rule's default output, so any consumer rule can depend on `//:scan_report` and read the report via `$(execpath //:scan_report)`. + +`format` accepts any of Periphery's output formats: `xcode`, `csv`, `json`, `checkstyle`, `codeclimate`, `github-actions`, `github-markdown`, `gitlab-codequality`. + ### Other Periphery can analyze projects using other build systems, though it cannot drive them automatically like SPM, Xcode, and Bazel. Instead, you need to create a configuration file that specifies the location of indexstore and other resource files. The format is as follows: diff --git a/Sources/Frontend/Commands/ScanCommand.swift b/Sources/Frontend/Commands/ScanCommand.swift index d29156377..c136924bb 100644 --- a/Sources/Frontend/Commands/ScanCommand.swift +++ b/Sources/Frontend/Commands/ScanCommand.swift @@ -302,7 +302,7 @@ struct ScanCommand: ParsableCommand { logger.info(output, canQuiet: false) - if !filteredResults.isEmpty, let resultsPath = configuration.writeResults { + if let resultsPath = configuration.writeResults { var output = output if colored { diff --git a/bazel/internal/scan/BUILD.bazel b/bazel/internal/scan/BUILD.bazel index 3b371ef73..2c2088f0a 100644 --- a/bazel/internal/scan/BUILD.bazel +++ b/bazel/internal/scan/BUILD.bazel @@ -1,3 +1,4 @@ exports_files([ "scan_template.sh", + "scan_test_template.sh", ]) diff --git a/bazel/internal/scan/scan.bzl b/bazel/internal/scan/scan.bzl index da231fd9f..c41af73c1 100644 --- a/bazel/internal/scan/scan.bzl +++ b/bazel/internal/scan/scan.bzl @@ -1,5 +1,6 @@ """ - Declares the `scan` rule which acts as the primary entry point for the Periphery Bazel integration. + Declares the `scan` and `scan_test` rules which act as the primary entry points + for the Periphery Bazel integration. """ load("@bazel_skylib//lib:sets.bzl", "sets") @@ -39,20 +40,40 @@ def _periphery_info_providers(rule_attr): providers.append(target[PeripheryInfo]) return providers -def _force_indexstore_impl(settings, _attr): +_SWIFT_COPT_SETTING = "@rules_swift//swift:copt" + +def _periphery_deps_transition_impl(settings, _attr): + # Periphery's analysis relies on: + # + # - `swift.index_while_building`, so an indexstore is written during + # compilation. + # - Whole-module optimization, so each module compiles in a single + # frontend invocation and emits a single consolidated indexstore that + # captures cross-file references. Per-file compilation otherwise splits + # the indexstore in ways that confuse periphery's reference resolution. + # + # rules_swift's toolchain scans the `@rules_swift//swift:copt` build + # setting and auto-enables `swift._wmo_in_swiftcopts` whenever it sees + # `-wmo`, `-whole-module-optimization`, or `-force-single-frontend-invocation` + # (see `swift/internal/wmo.bzl`). Setting `-wmo` here triggers that path + # without forcing `compilation_mode = opt`, which would also enable + # inlining, dead-code elimination, and other transforms we don't want. return { "//command_line_option:features": settings["//command_line_option:features"] + [ "swift.index_while_building", ], + _SWIFT_COPT_SETTING: settings[_SWIFT_COPT_SETTING] + ["-wmo"], } -force_indexstore = transition( - implementation = _force_indexstore_impl, +periphery_deps_transition = transition( + implementation = _periphery_deps_transition_impl, inputs = [ "//command_line_option:features", + _SWIFT_COPT_SETTING, ], outputs = [ "//command_line_option:features", + _SWIFT_COPT_SETTING, ], ) @@ -153,8 +174,13 @@ def _scan_inputs_aspect_impl(target, ctx): ), ] -# buildifier: disable=function-docstring -def scan_impl(ctx): +def _path_for(file, is_test): + # When invoked via `bazel test` the script runs from the runfiles directory, + # so paths must be runfiles-relative (short_path). For `bazel run` the script + # runs from the workspace root, so the exec-root relative path is correct. + return file.short_path if is_test else file.path + +def _scan_impl_common(ctx, is_test): swift_srcs_set = sets.make() indexstores_set = sets.make() plists_set = sets.make() @@ -180,23 +206,23 @@ def scan_impl(ctx): xcmappingmodels = sets.to_list(xcmappingmodels_set) test_targets = sets.to_list(test_targets_set) - indexstores_config = [file.path for file in indexstores] + indexstores_config = [_path_for(file, is_test) for file in indexstores] if ctx.attr.global_indexstore: indexstores_config = [ctx.attr.global_indexstore] project_config = struct( indexstores = indexstores_config, - plists = [file.path for file in plists], - xibs = [file.path for file in xibs], - xcdatamodels = [file.path for file in xcdatamodels], - xcmappingmodels = [file.path for file in xcmappingmodels], + plists = [_path_for(file, is_test) for file in plists], + xibs = [_path_for(file, is_test) for file in xibs], + xcdatamodels = [_path_for(file, is_test) for file in xcdatamodels], + xcmappingmodels = [_path_for(file, is_test) for file in xcmappingmodels], test_targets = test_targets, ) periphery = ctx.attr.periphery[DefaultInfo].files_to_run.executable project_config_json = json.encode_indent(project_config) - project_config_file = ctx.actions.declare_file("project_config.json") + project_config_file = ctx.outputs.project_config ctx.actions.write(project_config_file, project_config_json) ctx.actions.expand_template( @@ -205,20 +231,120 @@ def scan_impl(ctx): substitutions = { "%periphery_path%": periphery.short_path, "%config_path%": ctx.attr.config, - "%project_config_path%": project_config_file.path, + "%project_config_path%": _path_for(project_config_file, is_test), }, ) + runfiles_files = swift_srcs + indexstores + plists + xibs + xcdatamodels + xcmappingmodels + [periphery] + if is_test: + # The generated config file is referenced by the script via its runfiles + # path, so it must be included in the test's runfiles. + runfiles_files = runfiles_files + [project_config_file] + return DefaultInfo( executable = ctx.outputs.scan, runfiles = ctx.runfiles( # Swift sources are not included in the generated project file, yet they are referenced # in the indexstores and will be read by Periphery, and therefore must be present in # the runfiles. - files = swift_srcs + indexstores + plists + xibs + xcdatamodels + xcmappingmodels + [periphery], + files = runfiles_files, ), ) +# buildifier: disable=function-docstring +def scan_impl(ctx): + return _scan_impl_common(ctx, is_test = False) + +# buildifier: disable=function-docstring +def scan_test_impl(ctx): + return _scan_impl_common(ctx, is_test = True) + +def _collect_inputs(ctx): + swift_srcs_set = sets.make() + indexstores_set = sets.make() + plists_set = sets.make() + xibs_set = sets.make() + xcdatamodels_set = sets.make() + xcmappingmodels_set = sets.make() + test_targets_set = sets.make() + + for dep in ctx.attr.deps: + swift_srcs_set = sets.union(swift_srcs_set, sets.make(dep[PeripheryInfo].swift_srcs.to_list())) + indexstores_set = sets.union(indexstores_set, sets.make(dep[PeripheryInfo].indexstores.to_list())) + plists_set = sets.union(plists_set, sets.make(dep[PeripheryInfo].plists.to_list())) + xibs_set = sets.union(xibs_set, sets.make(dep[PeripheryInfo].xibs.to_list())) + xcdatamodels_set = sets.union(xcdatamodels_set, sets.make(dep[PeripheryInfo].xcdatamodels.to_list())) + xcmappingmodels_set = sets.union(xcmappingmodels_set, sets.make(dep[PeripheryInfo].xcmappingmodels.to_list())) + test_targets_set = sets.union(test_targets_set, sets.make(dep[PeripheryInfo].test_targets.to_list())) + + return struct( + swift_srcs = sets.to_list(swift_srcs_set), + indexstores = sets.to_list(indexstores_set), + plists = sets.to_list(plists_set), + xibs = sets.to_list(xibs_set), + xcdatamodels = sets.to_list(xcdatamodels_set), + xcmappingmodels = sets.to_list(xcmappingmodels_set), + test_targets = sets.to_list(test_targets_set), + ) + +# buildifier: disable=function-docstring +def scan_report_impl(ctx): + inputs = _collect_inputs(ctx) + + indexstores_config = [file.path for file in inputs.indexstores] + if ctx.attr.global_indexstore: + indexstores_config = [ctx.attr.global_indexstore] + + project_config = struct( + indexstores = indexstores_config, + plists = [file.path for file in inputs.plists], + xibs = [file.path for file in inputs.xibs], + xcdatamodels = [file.path for file in inputs.xcdatamodels], + xcmappingmodels = [file.path for file in inputs.xcmappingmodels], + test_targets = inputs.test_targets, + ) + + project_config_file = ctx.outputs.project_config + ctx.actions.write(project_config_file, json.encode_indent(project_config)) + + periphery = ctx.attr.periphery[DefaultInfo].files_to_run.executable + report_file = ctx.outputs.report + + args = ctx.actions.args() + args.add("scan") + args.add("--disable-update-check") + args.add("--format", ctx.attr.format) + args.add("--write-results", report_file.path) + args.add("--generic-project-config", project_config_file.path) + if ctx.attr.config: + args.add("--config", ctx.attr.config) + + action_inputs = ( + inputs.swift_srcs + + inputs.indexstores + + inputs.plists + + inputs.xibs + + inputs.xcdatamodels + + inputs.xcmappingmodels + + [project_config_file] + ) + + ctx.actions.run( + executable = periphery, + arguments = [args], + inputs = action_inputs, + outputs = [report_file], + mnemonic = "PeripheryScan", + progress_message = "Generating Periphery report for %{label}", + ) + + return [ + DefaultInfo(files = depset([report_file])), + OutputGroupInfo( + project_config = depset([project_config_file]), + ), + ] + scan_inputs_aspect = aspect( _scan_inputs_aspect_impl, attr_aspects = _TRANSITIVE_ATTRS, diff --git a/bazel/internal/scan/scan_template.sh b/bazel/internal/scan/scan_template.sh index fed7a2c86..9a923a590 100644 --- a/bazel/internal/scan/scan_template.sh +++ b/bazel/internal/scan/scan_template.sh @@ -1 +1,7 @@ -%periphery_path% scan --config "%config_path%" --generic-project-config "%project_config_path%" +#!/bin/bash +set -eo pipefail +if [ -n "%config_path%" ]; then + exec "%periphery_path%" scan --config "%config_path%" --generic-project-config "%project_config_path%" +else + exec "%periphery_path%" scan --generic-project-config "%project_config_path%" +fi diff --git a/bazel/internal/scan/scan_test_template.sh b/bazel/internal/scan/scan_test_template.sh new file mode 100755 index 000000000..3f60e3597 --- /dev/null +++ b/bazel/internal/scan/scan_test_template.sh @@ -0,0 +1,8 @@ +#!/bin/bash +set -eo pipefail +cd "${TEST_SRCDIR}/${TEST_WORKSPACE}" +if [ -n "%config_path%" ]; then + exec "%periphery_path%" scan --strict --disable-update-check --project-root "$(pwd)" --config "%config_path%" --generic-project-config "%project_config_path%" +else + exec "%periphery_path%" scan --strict --disable-update-check --project-root "$(pwd)" --generic-project-config "%project_config_path%" +fi diff --git a/bazel/rules.bzl b/bazel/rules.bzl index 4e6f48830..d1a1ad438 100644 --- a/bazel/rules.bzl +++ b/bazel/rules.bzl @@ -2,32 +2,97 @@ Periphery public rules. """ -load("//bazel/internal/scan:scan.bzl", "force_indexstore", "scan_impl", "scan_inputs_aspect") +load( + "//bazel/internal/scan:scan.bzl", + "periphery_deps_transition", + "scan_impl", + "scan_inputs_aspect", + "scan_report_impl", + "scan_test_impl", +) + +_COMMON_ATTRS = { + "deps": attr.label_list( + cfg = periphery_deps_transition, + mandatory = True, + aspects = [scan_inputs_aspect], + doc = "Top-level project targets to scan.", + ), + "config": attr.string(doc = "Path to the periphery.yml configuration file."), + "global_indexstore": attr.string(doc = "Path to a global index store."), + "periphery": attr.label( + doc = "The periphery executable target.", + default = "@periphery//:periphery", + ), +} scan = rule( doc = "Scans the top-level deps and their transitive deps for unused code.", - attrs = { - "deps": attr.label_list( - cfg = force_indexstore, - mandatory = True, - aspects = [scan_inputs_aspect], - doc = "Top-level project targets to scan.", - ), - "config": attr.string(doc = "Path to the periphery.yml configuration file."), - "global_indexstore": attr.string(doc = "Path to a global index store."), - "periphery": attr.label( - doc = "The periphery executable target.", - default = "@periphery//:periphery", - ), + attrs = dict(_COMMON_ATTRS, **{ "_template": attr.label( allow_single_file = True, default = "@periphery//bazel/internal/scan:scan_template.sh", ), - }, + }), outputs = { - "project_config": "project_config.json", - "scan": "scan.sh", + "project_config": "%{name}_project_config.json", + "scan": "%{name}.sh", }, implementation = scan_impl, executable = True, ) + +scan_test = rule( + doc = """\ +Scans the top-level deps and their transitive deps for unused code as a test target. + +The test fails if Periphery reports any unused declarations (`--strict` is enabled). +Use this rule to wire Periphery into CI via `bazel test`.\ +""", + attrs = dict(_COMMON_ATTRS, **{ + "_template": attr.label( + allow_single_file = True, + default = "@periphery//bazel/internal/scan:scan_test_template.sh", + ), + }), + outputs = { + "project_config": "%{name}_project_config.json", + "scan": "%{name}.sh", + }, + implementation = scan_test_impl, + test = True, +) + +_REPORT_FORMATS = [ + "xcode", + "csv", + "json", + "checkstyle", + "codeclimate", + "github-actions", + "github-markdown", + "gitlab-codequality", +] + +scan_report = rule( + doc = """\ +Scans the top-level deps and their transitive deps for unused code and writes the +formatted report to a file output. + +Unlike `scan` and `scan_test`, this rule runs Periphery at build time and exposes +the report as a regular Bazel file artifact, so it can be consumed by other rules +via `data` deps or `srcs`.\ +""", + attrs = dict(_COMMON_ATTRS, **{ + "format": attr.string( + doc = "Output format for the report. One of: " + ", ".join(_REPORT_FORMATS) + ".", + default = "json", + values = _REPORT_FORMATS, + ), + }), + outputs = { + "report": "%{name}.report", + "project_config": "%{name}_project_config.json", + }, + implementation = scan_report_impl, +) diff --git a/bazel/tests/BUILD.bazel b/bazel/tests/BUILD.bazel new file mode 100644 index 000000000..8edd6dca8 --- /dev/null +++ b/bazel/tests/BUILD.bazel @@ -0,0 +1,51 @@ +""" + Harness targets that exercise the public Periphery Bazel rules end-to-end. +""" + +load("@rules_shell//shell:sh_test.bzl", "sh_test") +load("@rules_swift//swift:swift_binary.bzl", "swift_binary") +load("//bazel:rules.bzl", "scan", "scan_report", "scan_test") + +# Fixture with no unused declarations. +swift_binary( + name = "Clean", + srcs = ["Clean.swift"], +) + +# Exercises the `scan` rule (executable). Built but not run by default. +scan( + name = "clean_scan", + deps = [":Clean"], +) + +# Exercises the `scan_test` rule. Passes because the fixture has no unused code. +scan_test( + name = "clean_scan_test", + deps = [":Clean"], +) + +# Exercises the `scan_report` rule against clean code (empty findings path). +scan_report( + name = "clean_scan_report", + deps = [":Clean"], +) + +# Fixture with an intentionally unused declaration. +swift_binary( + name = "Unused", + srcs = ["Unused.swift"], +) + +# `scan_report` against code with findings; consumed by the verification test below. +scan_report( + name = "unused_scan_report", + deps = [":Unused"], +) + +# Asserts that scan_report's output actually contains the expected unused symbol. +sh_test( + name = "unused_scan_report_test", + srcs = ["verify_unused_report.sh"], + args = ["$(rootpath :unused_scan_report)"], + data = [":unused_scan_report"], +) diff --git a/bazel/tests/Clean.swift b/bazel/tests/Clean.swift new file mode 100644 index 000000000..1a12b835b --- /dev/null +++ b/bazel/tests/Clean.swift @@ -0,0 +1,11 @@ +@main +struct CleanEntry { + static func main() { + let helper = CleanHelper() + print(helper.greet()) + } +} + +struct CleanHelper { + func greet() -> String { "hi" } +} diff --git a/bazel/tests/Unused.swift b/bazel/tests/Unused.swift new file mode 100644 index 000000000..4dcbb5c96 --- /dev/null +++ b/bazel/tests/Unused.swift @@ -0,0 +1,16 @@ +@main +struct UnusedEntry { + static func main() { + let used = UsedSymbol() + print(used.referenced()) + } +} + +struct UsedSymbol { + func referenced() -> Int { 42 } +} + +// Intentionally never referenced. The harness asserts Periphery's report flags this symbol. +struct UnusedSymbol { + func unused() -> Int { 0 } +} diff --git a/bazel/tests/verify_unused_report.sh b/bazel/tests/verify_unused_report.sh new file mode 100755 index 000000000..917bc4274 --- /dev/null +++ b/bazel/tests/verify_unused_report.sh @@ -0,0 +1,17 @@ +#!/bin/bash +set -euo pipefail + +report="$1" + +if [ ! -s "$report" ]; then + echo "ERROR: report file is missing or empty at $report" >&2 + exit 1 +fi + +if ! grep -q "UnusedSymbol" "$report"; then + echo "ERROR: report did not contain expected unused symbol 'UnusedSymbol'" >&2 + echo "--- report content ---" >&2 + cat "$report" >&2 + echo "--- end ---" >&2 + exit 1 +fi