Skip to content

Commit

Permalink
Implement "all" sentinel
Browse files Browse the repository at this point in the history
  • Loading branch information
fmeum committed Apr 4, 2023
1 parent 6d5cb9b commit ace1a4e
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ public boolean isDevDependency(TypeCheckedTag tag) {
defaultValue = "None",
allowedTypes = {
@ParamType(type = Sequence.class, generic1 = String.class),
@ParamType(type = String.class),
@ParamType(type = NoneType.class)}),
@Param(
name = "root_module_direct_dev_deps",
Expand All @@ -142,32 +143,12 @@ public boolean isDevDependency(TypeCheckedTag tag) {
defaultValue = "None",
allowedTypes = {
@ParamType(type = Sequence.class, generic1 = String.class),
@ParamType(type = String.class),
@ParamType(type = NoneType.class)}),
})
public ModuleExtensionMetadata extensionMetadata(Object rootModuleDirectDepsUnchecked,
Object rootModuleDirectDevDepsUnchecked) throws EvalException {
Sequence<String> rootModuleDirectDeps = rootModuleDirectDepsUnchecked == Starlark.NONE ? null
: Sequence.cast(rootModuleDirectDepsUnchecked, String.class, "root_module_direct_deps");
Sequence<String> rootModuleDirectDevDeps =
rootModuleDirectDevDepsUnchecked == Starlark.NONE ? null
: Sequence.cast(rootModuleDirectDevDepsUnchecked, String.class,
"root_module_direct_dev_deps");
return ModuleExtensionMetadata.create(rootModuleDirectDeps, rootModuleDirectDevDeps);
}

@StarlarkMethod(
name = "use_all_repos",
doc = "Return the value created by this method from a module extension to indicate that all "
+ "repositories instantiated by it should be imported via <code>use_repo</code> by all "
+ "modules using this extension.",
parameters = {
@Param(
name = "dev_dependency",
doc = "Whether the repositories instantiated by this extension should be imported "
+ "as dev dependencies.",
allowedTypes = {@ParamType(type = boolean.class)})
})
public ModuleExtensionMetadata useAllRepos(boolean devDependency) {
return ModuleExtensionMetadata.createForUseAllRepos(devDependency);
return ModuleExtensionMetadata.create(rootModuleDirectDepsUnchecked,
rootModuleDirectDevDepsUnchecked);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import javax.annotation.Nullable;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkValue;
import net.starlark.java.syntax.Location;
Expand All @@ -45,15 +46,34 @@ private ModuleExtensionMetadata(@Nullable Set<String> explicitRootModuleDirectDe
this.useAllRepos = useAllRepos;
}

static ModuleExtensionMetadata create(List<String> rootModuleDirectDeps,
@Nullable List<String> rootModuleDirectDevDeps) throws EvalException {
if ((rootModuleDirectDeps == null) != (rootModuleDirectDevDeps == null)) {
static ModuleExtensionMetadata create(Object rootModuleDirectDepsUnchecked,
Object rootModuleDirectDevDepsUnchecked) throws EvalException {
if (rootModuleDirectDepsUnchecked == Starlark.NONE
&& rootModuleDirectDevDepsUnchecked == Starlark.NONE) {
return new ModuleExtensionMetadata(null, null, UseAllRepos.NO);
}

if ("all".equals(rootModuleDirectDepsUnchecked)
&& rootModuleDirectDevDepsUnchecked == Starlark.NONE) {
return new ModuleExtensionMetadata(null, null, UseAllRepos.REGULAR);
}

if ("all".equals(rootModuleDirectDevDepsUnchecked)
&& rootModuleDirectDepsUnchecked == Starlark.NONE) {
return new ModuleExtensionMetadata(null, null, UseAllRepos.DEV);
}

if ((rootModuleDirectDepsUnchecked == Starlark.NONE) != (rootModuleDirectDevDepsUnchecked
== Starlark.NONE)) {
throw Starlark.errorf("root_module_direct_deps and root_module_direct_dev_deps must both be "
+ "specified or both be unspecified");
}
if (rootModuleDirectDeps == null) {
return new ModuleExtensionMetadata(null, null, UseAllRepos.NO);
}

Sequence<String> rootModuleDirectDeps = Sequence.cast(rootModuleDirectDepsUnchecked,
String.class, "root_module_direct_deps");
Sequence<String> rootModuleDirectDevDeps =
Sequence.cast(rootModuleDirectDevDepsUnchecked, String.class,
"root_module_direct_dev_deps");

Set<String> explicitRootModuleDirectDeps = new LinkedHashSet<>();
for (String dep : rootModuleDirectDeps) {
Expand Down Expand Up @@ -87,11 +107,6 @@ static ModuleExtensionMetadata create(List<String> rootModuleDirectDeps,
explicitRootModuleDirectDevDeps, UseAllRepos.NO);
}

static ModuleExtensionMetadata createForUseAllRepos(Boolean devDependency) {
return new ModuleExtensionMetadata(null, null, devDependency ? UseAllRepos.DEV
: UseAllRepos.REGULAR);
}

public void evaluate(Collection<ModuleExtensionUsage> usages, Set<String> allRepos,
EventHandler handler) throws EvalException {
generateFixupMessage(usages, allRepos).ifPresent(handler::handle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.events.EventKind;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkList;
import net.starlark.java.syntax.Location;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -19,14 +21,14 @@ public class ModuleExtensionMetadataTest {
public void testCreate_exactlyOneArgIsNull() {
Exception e = assertThrows(
EvalException.class,
() -> ModuleExtensionMetadata.create(null, ImmutableList.of()));
() -> ModuleExtensionMetadata.create(Starlark.NONE, StarlarkList.immutableOf()));
assertThat(e).hasMessageThat()
.isEqualTo(
"root_module_direct_deps and root_module_direct_dev_deps must both be specified or both be unspecified");

e = assertThrows(
EvalException.class,
() -> ModuleExtensionMetadata.create(ImmutableList.of(), null));
() -> ModuleExtensionMetadata.create(StarlarkList.immutableOf(), Starlark.NONE));
assertThat(e).hasMessageThat()
.isEqualTo(
"root_module_direct_deps and root_module_direct_dev_deps must both be specified or both be unspecified");
Expand All @@ -36,7 +38,8 @@ public void testCreate_exactlyOneArgIsNull() {
public void testCreate_invalidRepoName() {
Exception e = assertThrows(
EvalException.class,
() -> ModuleExtensionMetadata.create(ImmutableList.of("~invalid"), ImmutableList.of()));
() -> ModuleExtensionMetadata.create(StarlarkList.immutableOf("~invalid"),
StarlarkList.immutableOf()));
assertThat(e).hasMessageThat()
.isEqualTo(
"in root_module_direct_deps: invalid user-provided repo name '~invalid': valid names may contain only A-Z, a-z, 0-9, '-', '_', '.', and must start with a letter");
Expand All @@ -46,7 +49,8 @@ public void testCreate_invalidRepoName() {
public void testCreate_invalidDevRepoName() {
Exception e = assertThrows(
EvalException.class,
() -> ModuleExtensionMetadata.create(ImmutableList.of(), ImmutableList.of("~invalid")));
() -> ModuleExtensionMetadata.create(StarlarkList.immutableOf(),
StarlarkList.immutableOf("~invalid")));
assertThat(e).hasMessageThat()
.isEqualTo(
"in root_module_direct_dev_deps: invalid user-provided repo name '~invalid': valid names may contain only A-Z, a-z, 0-9, '-', '_', '.', and must start with a letter");
Expand All @@ -56,7 +60,8 @@ public void testCreate_invalidDevRepoName() {
public void testCreate_duplicateRepo() {
Exception e = assertThrows(
EvalException.class,
() -> ModuleExtensionMetadata.create(ImmutableList.of("dep", "dep"), ImmutableList.of()));
() -> ModuleExtensionMetadata.create(StarlarkList.immutableOf("dep", "dep"),
StarlarkList.immutableOf()));
assertThat(e).hasMessageThat()
.isEqualTo("duplicate root_module_direct_deps entry 'dep'");
}
Expand All @@ -65,7 +70,8 @@ public void testCreate_duplicateRepo() {
public void testCreate_duplicateRepoAcrossTypes() {
Exception e = assertThrows(
EvalException.class,
() -> ModuleExtensionMetadata.create(ImmutableList.of("dep"), ImmutableList.of("dep")));
() -> ModuleExtensionMetadata.create(StarlarkList.immutableOf("dep"),
StarlarkList.immutableOf("dep")));
assertThat(e).hasMessageThat()
.isEqualTo("root_module_direct_dev_deps entry 'dep' is also in root_module_direct_deps");
}
Expand All @@ -83,8 +89,9 @@ public void testGenerateFixupMessage() throws EvalException {
.build();
var allRepos = ImmutableSet.of("direct_dep", "direct_dev_dep", "indirect_dep",
"indirect_dev_dep");
var moduleExtensionMetadata = ModuleExtensionMetadata.create(ImmutableList.of("direct_dep"),
ImmutableList.of("direct_dev_dep"));
var moduleExtensionMetadata = ModuleExtensionMetadata.create(
StarlarkList.immutableOf("direct_dep"),
StarlarkList.immutableOf("direct_dev_dep"));
var fixupMessage = moduleExtensionMetadata.generateFixupMessage(ImmutableList.of(rootUsage),
allRepos);

Expand Down Expand Up @@ -124,7 +131,7 @@ public void testGenerateFixupMessage_useAllRepos() throws EvalException {
.build();
var allRepos = ImmutableSet.of("direct_dep", "direct_dev_dep", "indirect_dep",
"indirect_dev_dep");
var moduleExtensionMetadata = ModuleExtensionMetadata.createForUseAllRepos(false);
var moduleExtensionMetadata = ModuleExtensionMetadata.create("all", Starlark.NONE);
var fixupMessage = moduleExtensionMetadata.generateFixupMessage(ImmutableList.of(rootUsage),
allRepos);

Expand Down Expand Up @@ -160,7 +167,7 @@ public void testGenerateFixupMessage_useAllRepos_dev() throws EvalException {
.build();
var allRepos = ImmutableSet.of("direct_dep", "direct_dev_dep", "indirect_dep",
"indirect_dev_dep");
var moduleExtensionMetadata = ModuleExtensionMetadata.createForUseAllRepos(true);
var moduleExtensionMetadata = ModuleExtensionMetadata.create(Starlark.NONE, "all");
var fixupMessage = moduleExtensionMetadata.generateFixupMessage(ImmutableList.of(rootUsage),
allRepos);

Expand Down

0 comments on commit ace1a4e

Please sign in to comment.