Skip to content

Commit

Permalink
Fix repr for enum
Browse files Browse the repository at this point in the history
Summary:
[repr(x) formats its argument as a string](https://github.com/bazelbuild/starlark/blob/master/spec.md#repr). However, for enum variants, it showed the internal value only but it should also show the surrounding enum's name.

I think under the hood, [str()](https://github.com/bazelbuild/starlark/blob/master/spec.md#str) which formats its argument as a string also calls `collect_repr` somehow so its behavior is affected (hence tests are replaced). This makes sense to me since they behave similarly but let me know if that is not the expected behavior.

Reviewed By: stepancheg, wendy728

Differential Revision: D50360889

fbshipit-source-id: 449add8f256fce69afbca1ea25cf66c51f5afa86
  • Loading branch information
ezgicicek authored and facebook-github-bot committed Oct 19, 2023
1 parent 9e25a13 commit 4c5236f
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ def test(x):

Max stack size: 1
Instructions:
0: EqConst &x "red" &1
0: EqConst &x MyEnum("red") &1
24: Return &1
32: End
2 changes: 1 addition & 1 deletion starlark/src/values/types/enumeration/enum_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ def g(x):
g(Season[0])
"#,
r#"Value `"SPRING"` of type `enum` does not match the type annotation `Color` for argument `x`"#,
r#"Value `Season("SPRING")` of type `enum` does not match the type annotation `Color` for argument `x`"#,
);
}

Expand Down
5 changes: 3 additions & 2 deletions starlark/src/values/types/enumeration/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ assert_eq([x.value for x in enum_type], ["option1","option2"])"#,
enum_type = enum("option1","option2")
x = enum_type("option1")
assert_eq(str(enum_type), "enum(\"option1\", \"option2\")")
assert_eq(str(x), "\"option1\"")
assert_eq(str(x), "enum_type(\"option1\")")
"#,
);
assert::pass(
Expand Down Expand Up @@ -191,7 +191,8 @@ assert_ne(rt("one"), diff("one"))
assert::pass(
r#"
enum_type = enum("option1", "option2")
assert_eq("\"option1\"", repr(enum_type("option1")))
assert_eq("enum_type(\"option1\")", repr(enum_type("option1")))
assert_eq("enum()(\"option1\")", repr(enum("option1", "option2")("option1")))
"#,
);
}
Expand Down
26 changes: 24 additions & 2 deletions starlark/src/values/types/enumeration/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,31 @@ pub struct EnumValueGen<V> {
pub(crate) id: TypeInstanceId,
}

impl<V: Display> Display for EnumValueGen<V> {
impl<'v, V: ValueLike<'v> + 'v> Display for EnumValueGen<V> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.value.fmt(f)
let ty_enum_data = match self.get_enum_type() {
Either::Left(x) => x.ty_enum_data(),
Either::Right(x) => x.ty_enum_data(),
};
match ty_enum_data {
Some(ty_enum_data) => {
{
write!(f, "{}", &ty_enum_data.name)?;
write!(f, "(")?;
Display::fmt(&self.value, f)?;
write!(f, ")")?
};
Ok(())
}
None => {
{
write!(f, "enum()(")?;
Display::fmt(&self.value, f)?;
write!(f, ")")?
};
Ok(())
}
}
}
}

Expand Down

0 comments on commit 4c5236f

Please sign in to comment.