Skip to content

Commit 1766473

Browse files
WyveraldWyverald
committed
Enforce file name format for MODULE.bazel includes
They must end in `.MODULE.bazel`. Follow-up for #17880 Closes #22075. PiperOrigin-RevId: 627136756 Change-Id: If9b1797f2e7ddc1aebd929646329e832288bfd8a
1 parent d251cc3 commit 1766473

File tree

5 files changed

+48
-21
lines changed

5 files changed

+48
-21
lines changed

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/CompiledModuleFile.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public void visit(AssignmentStatement node) {
142142
@Override
143143
public void visit(DotExpression node) {
144144
visit(node.getObject());
145-
if (includeWasAssigned || !node.getField().getName().equals(INCLUDE_IDENTIFIER)) {
145+
if (!node.getField().getName().equals(INCLUDE_IDENTIFIER)) {
146146
// This is fine: `whatever.include`
147147
// (so `include` can be used as a tag class name)
148148
visit(node.getField());

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,14 @@ private static ImmutableList<IncludeStatement> advanceHorizon(
351351
includeStatement.includeLabel(),
352352
includeStatement.location());
353353
}
354+
if (!includeStatement.includeLabel().endsWith(".MODULE.bazel")) {
355+
throw errorf(
356+
Code.BAD_MODULE,
357+
"bad include label '%s' at %s: the file to be included must have a name ending in"
358+
+ " '.MODULE.bazel'",
359+
includeStatement.includeLabel(),
360+
includeStatement.location());
361+
}
354362
try {
355363
includeLabels.add(Label.parseCanonical(includeStatement.includeLabel()));
356364
} catch (LabelSyntaxException e) {

‎src/test/java/com/google/devtools/build/lib/bazel/bzlmod/CompiledModuleFileTest.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,16 @@ public void checkSyntax_good_benignUsageOfInclude() throws Exception {
9191
public void checkSyntax_good_includeIdentifierReassigned() throws Exception {
9292
String program =
9393
"""
94+
include('world')
9495
include = print
9596
# from this point on, we no longer check anything about `include` usage.
9697
include('hello')
9798
str(include)
9899
exclude = include
99100
""";
100-
assertThat(checkSyntax(program)).isEmpty();
101+
assertThat(checkSyntax(program))
102+
.containsExactly(
103+
new IncludeStatement("world", Location.fromFileLineColumn("test file", 1, 1)));
101104
}
102105

103106
@Test

‎src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -334,22 +334,22 @@ public void testRootModule_include_good() throws Exception {
334334
scratch.overwriteFile(
335335
rootDirectory.getRelative("MODULE.bazel").getPathString(),
336336
"module(name='aaa')",
337-
"include('//java:MODULE.bazel.segment')",
337+
"include('//java:java.MODULE.bazel')",
338338
"bazel_dep(name='foo', version='1.0')",
339339
"register_toolchains('//:whatever')",
340-
"include('//python:MODULE.bazel.segment')");
340+
"include('//python:python.MODULE.bazel')");
341341
scratch.overwriteFile(rootDirectory.getRelative("java/BUILD").getPathString());
342342
scratch.overwriteFile(
343-
rootDirectory.getRelative("java/MODULE.bazel.segment").getPathString(),
343+
rootDirectory.getRelative("java/java.MODULE.bazel").getPathString(),
344344
"bazel_dep(name='java-foo', version='1.0')");
345345
scratch.overwriteFile(rootDirectory.getRelative("python/BUILD").getPathString());
346346
scratch.overwriteFile(
347-
rootDirectory.getRelative("python/MODULE.bazel.segment").getPathString(),
347+
rootDirectory.getRelative("python/python.MODULE.bazel").getPathString(),
348348
"bazel_dep(name='py-foo', version='1.0', repo_name='python-foo')",
349349
"single_version_override(module_name='java-foo', version='2.0')",
350-
"include('//python:toolchains/MODULE.bazel.segment')");
350+
"include('//python:toolchains/toolchains.MODULE.bazel')");
351351
scratch.overwriteFile(
352-
rootDirectory.getRelative("python/toolchains/MODULE.bazel.segment").getPathString(),
352+
rootDirectory.getRelative("python/toolchains/toolchains.MODULE.bazel").getPathString(),
353353
"register_toolchains('//:python-whatever')");
354354
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
355355
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
@@ -387,7 +387,7 @@ public void testRootModule_include_bad_otherRepoLabel() throws Exception {
387387
scratch.overwriteFile(
388388
rootDirectory.getRelative("MODULE.bazel").getPathString(),
389389
"module(name='aaa')",
390-
"include('@haha//java:MODULE.bazel.segment')");
390+
"include('@haha//java:java.MODULE.bazel')");
391391
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
392392
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
393393

@@ -403,7 +403,7 @@ public void testRootModule_include_bad_relativeLabel() throws Exception {
403403
scratch.overwriteFile(
404404
rootDirectory.getRelative("MODULE.bazel").getPathString(),
405405
"module(name='aaa')",
406-
"include(':MODULE.bazel.segment')");
406+
"include(':relative.MODULE.bazel')");
407407
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
408408
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
409409

@@ -414,12 +414,28 @@ public void testRootModule_include_bad_relativeLabel() throws Exception {
414414
assertThat(result.getError().toString()).contains("starting with double slashes");
415415
}
416416

417+
@Test
418+
public void testRootModule_include_bad_notEndingInModuleBazel() throws Exception {
419+
scratch.overwriteFile(
420+
rootDirectory.getRelative("MODULE.bazel").getPathString(),
421+
"module(name='aaa')",
422+
"include('//:MODULE.bazel.segment')");
423+
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
424+
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
425+
426+
EvaluationResult<RootModuleFileValue> result =
427+
evaluator.evaluate(
428+
ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext);
429+
assertThat(result.hasError()).isTrue();
430+
assertThat(result.getError().toString()).contains("have a name ending in '.MODULE.bazel'");
431+
}
432+
417433
@Test
418434
public void testRootModule_include_bad_badLabelSyntax() throws Exception {
419435
scratch.overwriteFile(
420436
rootDirectory.getRelative("MODULE.bazel").getPathString(),
421437
"module(name='aaa')",
422-
"include('//haha/:::')");
438+
"include('//haha/:::.MODULE.bazel')");
423439
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
424440
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
425441

@@ -435,10 +451,10 @@ public void testRootModule_include_bad_badLabelSyntax() throws Exception {
435451
public void testRootModule_include_bad_moduleAfterInclude() throws Exception {
436452
scratch.overwriteFile(
437453
rootDirectory.getRelative("MODULE.bazel").getPathString(),
438-
"include('//java:MODULE.bazel.segment')");
454+
"include('//java:java.MODULE.bazel')");
439455
scratch.overwriteFile(rootDirectory.getRelative("java/BUILD").getPathString());
440456
scratch.overwriteFile(
441-
rootDirectory.getRelative("java/MODULE.bazel.segment").getPathString(),
457+
rootDirectory.getRelative("java/java.MODULE.bazel").getPathString(),
442458
"module(name='bet-you-didnt-expect-this-didya')",
443459
"bazel_dep(name='java-foo', version='1.0', repo_name='foo')");
444460
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
@@ -457,15 +473,15 @@ public void testRootModule_include_bad_repoNameCollision() throws Exception {
457473
scratch.overwriteFile(
458474
rootDirectory.getRelative("MODULE.bazel").getPathString(),
459475
"module(name='aaa')",
460-
"include('//java:MODULE.bazel.segment')",
461-
"include('//python:MODULE.bazel.segment')");
476+
"include('//java:java.MODULE.bazel')",
477+
"include('//python:python.MODULE.bazel')");
462478
scratch.overwriteFile(rootDirectory.getRelative("java/BUILD").getPathString());
463479
scratch.overwriteFile(
464-
rootDirectory.getRelative("java/MODULE.bazel.segment").getPathString(),
480+
rootDirectory.getRelative("java/java.MODULE.bazel").getPathString(),
465481
"bazel_dep(name='java-foo', version='1.0', repo_name='foo')");
466482
scratch.overwriteFile(rootDirectory.getRelative("python/BUILD").getPathString());
467483
scratch.overwriteFile(
468-
rootDirectory.getRelative("python/MODULE.bazel.segment").getPathString(),
484+
rootDirectory.getRelative("python/python.MODULE.bazel").getPathString(),
469485
"bazel_dep(name='python-foo', version='1.0', repo_name='foo')");
470486
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
471487
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
@@ -484,10 +500,10 @@ public void testRootModule_include_bad_tryingToLeakBindings() throws Exception {
484500
rootDirectory.getRelative("MODULE.bazel").getPathString(),
485501
"module(name='aaa')",
486502
"FOO_NAME = 'foo'",
487-
"include('//java:MODULE.bazel.segment')");
503+
"include('//java:java.MODULE.bazel')");
488504
scratch.overwriteFile(rootDirectory.getRelative("java/BUILD").getPathString());
489505
scratch.overwriteFile(
490-
rootDirectory.getRelative("java/MODULE.bazel.segment").getPathString(),
506+
rootDirectory.getRelative("java/java.MODULE.bazel").getPathString(),
491507
"bazel_dep(name=FOO_NAME, version='1.0')");
492508
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
493509
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

‎src/test/py/bazel/bzlmod/bazel_module_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -906,12 +906,12 @@ def testInclude(self):
906906
[
907907
'module(name="foo")',
908908
'bazel_dep(name="bbb", version="1.0")',
909-
'include("//java:MODULE.bazel.segment")',
909+
'include("//java:java.MODULE.bazel")',
910910
],
911911
)
912912
self.ScratchFile('java/BUILD')
913913
self.ScratchFile(
914-
'java/MODULE.bazel.segment',
914+
'java/java.MODULE.bazel',
915915
[
916916
'bazel_dep(name="aaa", version="1.0", repo_name="lol")',
917917
],

0 commit comments

Comments
 (0)