Skip to content

Commit 1aced65

Browse files
authored
Remove Label wrappers for target string literals (#1728)
Removes all the `Label` wrappers added in #1696 that aren't strictly necessary. Removed instances: ```sh $ git show -p | grep '^- .*Label(' | wc -l 88 ``` Remaining instances: ```sh $ git ls-files | xargs grep '[^A-Za-z0-9]Label(' | wc -l 31 ``` Inspired by research I did while writing a blog post reflecting upon the motivations behind #1696. Bazel evaluates target string literals in the context of the repository that appears to contain the assignment of the literal to a `Label` attribute. `Label` wrappers are necessary for target string literals within macros, passed to external functions, and included in files generated by a repository rule. This ensures that the targets refer to the originating repository by preventing Bazel from interpreting them within the context of the repository using them. Keeping the `Label` wrappers doesn't hurt, but leaving only the ones that are required helps those instances stand out.
1 parent ba13626 commit 1aced65

34 files changed

+104
-118
lines changed

jmh/jmh.bzl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@ scala_generate_benchmark = rule(
3939
"_generator": attr.label(
4040
executable = True,
4141
cfg = "exec",
42-
default = Label(
43-
"//src/scala/io/bazel/rules_scala/jmh_support:benchmark_generator",
42+
default = (
43+
"//src/scala/io/bazel/rules_scala/jmh_support:benchmark_generator"
4444
),
4545
),
4646
"runtime_jdk": attr.label(
47-
default = Label("@rules_java//toolchains:current_java_runtime"),
47+
default = "@rules_java//toolchains:current_java_runtime",
4848
providers = [java_common.JavaRuntimeInfo],
4949
),
5050
},

jmh/toolchain/toolchain.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ jmh_toolchain = rule(
3939
},
4040
)
4141

42-
_toolchain_type = Label("//jmh/toolchain:jmh_toolchain_type")
42+
_toolchain_type = "//jmh/toolchain:jmh_toolchain_type"
4343

4444
def _export_toolchain_deps_impl(ctx):
4545
return expose_toolchain_deps(ctx, _toolchain_type)
@@ -64,7 +64,7 @@ def setup_jmh_toolchain(name):
6464
native.toolchain(
6565
name = name,
6666
toolchain = ":%s_impl" % name,
67-
toolchain_type = _toolchain_type,
67+
toolchain_type = Label(_toolchain_type),
6868
visibility = ["//visibility:public"],
6969
)
7070

scala/plusone.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ PlusOneDeps = provider(
1010
)
1111

1212
def _collect_plus_one_deps_aspect_impl(target, ctx):
13-
toolchain = ctx.toolchains[Label("//scala:toolchain_type")]
13+
toolchain = ctx.toolchains["//scala:toolchain_type"]
1414
if toolchain.dependency_mode != "plus-one":
1515
return []
1616

@@ -26,5 +26,5 @@ def _collect_plus_one_deps_aspect_impl(target, ctx):
2626
collect_plus_one_deps_aspect = aspect(
2727
implementation = _collect_plus_one_deps_aspect_impl,
2828
attr_aspects = ["deps", "exports"],
29-
toolchains = [Label("//scala:toolchain_type")],
29+
toolchains = ["//scala:toolchain_type"],
3030
)

scala/private/common_attributes.bzl

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ common_attrs_for_plugin_bootstrapping = {
2929
"resource_strip_prefix": attr.string(),
3030
"resource_jars": attr.label_list(allow_files = True),
3131
"java_compile_toolchain": attr.label(
32-
default = Label("@rules_java//toolchains:current_java_toolchain"),
32+
default = "@rules_java//toolchains:current_java_toolchain",
3333
providers = [java_common.JavaToolchainInfo],
3434
),
3535
"scalacopts": attr.string_list(),
@@ -56,8 +56,8 @@ common_attrs.update(common_attrs_for_plugin_bootstrapping)
5656

5757
common_attrs.update({
5858
"_dependency_analyzer_plugin": attr.label(
59-
default = Label(
60-
"//third_party/dependency_analyzer/src/main:dependency_analyzer",
59+
default = (
60+
"//third_party/dependency_analyzer/src/main:dependency_analyzer"
6161
),
6262
allow_files = [".jar"],
6363
mandatory = False,
@@ -73,9 +73,7 @@ common_attrs.update({
7373
),
7474
"unused_dependency_checker_ignored_targets": attr.label_list(default = []),
7575
"_code_coverage_instrumentation_worker": attr.label(
76-
default = Label(
77-
"//src/java/io/bazel/rulesscala/coverage/instrumenter",
78-
),
76+
default = "//src/java/io/bazel/rulesscala/coverage/instrumenter",
7977
allow_files = True,
8078
executable = True,
8179
cfg = "exec",
@@ -84,35 +82,35 @@ common_attrs.update({
8482

8583
implicit_deps = {
8684
"_java_runtime": attr.label(
87-
default = Label("@rules_java//toolchains:current_java_runtime"),
85+
default = "@rules_java//toolchains:current_java_runtime",
8886
),
8987
"_java_host_runtime": attr.label(
90-
default = Label("@rules_java//toolchains:current_host_java_runtime"),
88+
default = "@rules_java//toolchains:current_host_java_runtime",
9189
),
9290
"_scalac": attr.label(
9391
executable = True,
9492
cfg = "exec",
95-
default = Label("//src/java/io/bazel/rulesscala/scalac"),
93+
default = "//src/java/io/bazel/rulesscala/scalac",
9694
allow_files = True,
9795
),
9896
"_exe": attr.label(
9997
executable = True,
10098
cfg = "exec",
101-
default = Label("//src/java/io/bazel/rulesscala/exe:exe"),
99+
default = "//src/java/io/bazel/rulesscala/exe:exe",
102100
),
103101
}
104102

105103
launcher_template = {
106104
"_java_stub_template": attr.label(
107-
default = Label("//java_stub_template/file"),
105+
default = "//java_stub_template/file",
108106
),
109107
}
110108

111109
# Single dep to allow IDEs to pickup all the implicit dependencies.
112110
resolve_deps = {
113111
"_scala_toolchain": attr.label_list(
114112
default = [
115-
Label("//scala/private/toolchain_deps:scala_library_classpath"),
113+
"//scala/private/toolchain_deps:scala_library_classpath",
116114
],
117115
allow_files = False,
118116
),

scala/private/coverage_replacements_provider.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ def _aspect_impl(target, ctx):
6767
_aspect = aspect(
6868
attr_aspects = _dependency_attributes,
6969
implementation = _aspect_impl,
70-
toolchains = [Label("//scala:toolchain_type")],
70+
toolchains = ["//scala:toolchain_type"],
7171
)
7272

7373
coverage_replacements_provider = struct(

scala/private/dependency.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,13 @@ def get_strict_deps_mode(ctx):
4343
if not hasattr(ctx.attr, "_dependency_analyzer_plugin"):
4444
return "off"
4545

46-
return ctx.toolchains[Label("//scala:toolchain_type")].strict_deps_mode
46+
return ctx.toolchains["//scala:toolchain_type"].strict_deps_mode
4747

4848
def get_compiler_deps_mode(ctx):
4949
if not hasattr(ctx.attr, "_dependency_analyzer_plugin"):
5050
return "off"
5151

52-
return ctx.toolchains[Label("//scala:toolchain_type")].compiler_deps_mode
52+
return ctx.toolchains["//scala:toolchain_type"].compiler_deps_mode
5353

5454
def _is_strict_deps_on(ctx):
5555
return get_strict_deps_mode(ctx) != "off"

scala/private/phases/phase_compile.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ def phase_compile_common(ctx, p):
9191
return _phase_compile_default(ctx, p)
9292

9393
def _phase_compile_default(ctx, p, _args = struct()):
94-
toolchain = ctx.toolchains[Label("//scala:toolchain_type")]
94+
toolchain = ctx.toolchains["//scala:toolchain_type"]
9595
buildijar_default_value = toolchain.scala_version.startswith("2.")
9696

9797
return _phase_compile(

scala/private/phases/phase_coverage_runfiles.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def phase_coverage_runfiles(ctx, p):
2121
coverage_replacements[jar] if jar in coverage_replacements else jar
2222
for jar in rjars.to_list()
2323
])
24-
jacocorunner = ctx.toolchains[Label("//scala:toolchain_type")].jacocorunner
24+
jacocorunner = ctx.toolchains["//scala:toolchain_type"].jacocorunner
2525
coverage_runfiles = jacocorunner.files.to_list() + ctx.files._lcov_merger + coverage_replacements.values()
2626
return struct(
2727
coverage_runfiles = coverage_runfiles,

scala/private/phases/phase_dependency.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def _phase_dependency(
3535
p,
3636
unused_deps_always_off,
3737
strict_deps_always_off):
38-
toolchain = ctx.toolchains[Label("//scala:toolchain_type")]
38+
toolchain = ctx.toolchains["//scala:toolchain_type"]
3939

4040
target_label = str(ctx.label)
4141

@@ -81,7 +81,7 @@ def _get_unused_deps_mode(ctx):
8181
if ctx.attr.unused_dependency_checker_mode:
8282
return ctx.attr.unused_dependency_checker_mode
8383
else:
84-
return ctx.toolchains[Label("//scala:toolchain_type")].unused_dependency_checker_mode
84+
return ctx.toolchains["//scala:toolchain_type"].unused_dependency_checker_mode
8585

8686
def _is_target_included(target, includes, excludes):
8787
for exclude in excludes:

scala/private/phases/phase_scalac_provider.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ load("//scala/private/toolchain_deps:toolchain_deps.bzl", "find_deps_info_on")
77
load("//scala:providers.bzl", _ScalacProvider = "ScalacProvider")
88

99
def phase_scalac_provider(ctx, p):
10-
toolchain_type_label = Label("//scala:toolchain_type")
10+
toolchain_type_label = "//scala:toolchain_type"
1111

1212
library_classpath = find_deps_info_on(ctx, toolchain_type_label, "scala_library_classpath").deps
1313
compile_classpath = find_deps_info_on(ctx, toolchain_type_label, "scala_compile_classpath").deps

0 commit comments

Comments
 (0)