Skip to content

Commit

Permalink
Only check for named fields in original template
Browse files Browse the repository at this point in the history
When generating templates, sometimes we might have a value provided
that looks like a named field, e.g. "::user_provided_value::".

A common example of this is in Rust, where double colons are used as
the namespace separator:
https://doc.rust-lang.org/book/appendix-02-operators.html#non-operator-symbols

BUG=319129315
PiperOrigin-RevId: 608628560
Change-Id: I21ca23e32773d1c008a0276515aeacdf92c07e38
  • Loading branch information
chris-campos authored and copybara-github committed Feb 21, 2024
1 parent eba0f65 commit 2aa4e5c
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.copybara.onboard.core.template;

import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.stream.Collectors.joining;

import com.google.common.base.Joiner;
Expand Down Expand Up @@ -88,6 +89,8 @@ public <T> T resolve(Input<T> input)
}
});
String config = template;
// TODO - b/326285980: Handle field values when they are the same format as the named field
// templates, e.g. ::foo::.
for (Entry<Field, Object> e : fields.entrySet()) {
if (e.getKey().location() == Location.NAMED) {
config = setNamedParam(config, e.getKey(), e.getValue());
Expand All @@ -109,7 +112,14 @@ public <T> T resolve(Input<T> input)
Matcher matcher = NAMED_FIELD.matcher(config);
Set<String> notReplaced = new HashSet<>();
while (matcher.find()) {
notReplaced.add(matcher.group());
String field = matcher.group();
// We only want to include named field matches that were present in the original template.
if (fields.keySet().stream()
.map(f -> String.format("::%s::", f.name()))
.collect(toImmutableSet())
.contains(field)) {
notReplaced.add(field);
}
}
if (!notReplaced.isEmpty()) {
throw new IllegalStateException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,37 @@ protected ImmutableMap<Field, Object> resolve(InputProviderResolver resolver) {
+ "required_keyword = \"something\",\n");
}

@Test
public void testOnlyMatchNamedFieldsFromTemplate()
throws CannotProvideException, InterruptedException {
TemplateConfigGenerator generator =
new TemplateConfigGenerator("" + "foo = \"::foo::\",\n" + "bar = ::bar::,\n" + "") {

@Override
public String name() {
return "test";
}

@Override
public ImmutableSet<Input<?>> consumes() {
return ImmutableSet.of();
}

@Override
public boolean isGenerator(InputProviderResolver resolver) {
return true;
}

@Override
protected ImmutableMap<Field, Object> resolve(InputProviderResolver resolver) {
return ImmutableMap.of(
Field.required("foo"), "::i_am_a_provided_value::", Field.required("bar"), "hello");
}
};
assertThat(generator.generate(RESOLVER))
.isEqualTo("" + "foo = \"::i_am_a_provided_value::\",\n" + "bar = hello,\n");
}

@Test
public void testLoadStatements() throws CannotProvideException, InterruptedException {
TemplateConfigGenerator generator =
Expand Down

0 comments on commit 2aa4e5c

Please # to comment.