fix(ast): fix ContentEq and ContentHash impls for literal types (#8426)

Fix implementations of `ContentEq` and `ContentHash` for literal AST types:

* `NullLiteral::content_hash` is a no-op, same as other types which only contain a `Span`.
* `NumericLiteral::content_eq` and `content_hash` ignore `base` field.
* `NumericLiteral::content_hash` works around `0.0 == -0.0`.
* `BigIntLiteral::content_eq` and `content_hash` ignore `base` field.
* `StringLiteral::content_hash` ignore `raw` field.
* `RegExpLiteral::content_eq` and `content_hash` consider 2 `RegExp`s to be equal if they are printed the same (regardless of whether they were parsed by `oxc_regular_expression` or not).

Additionally, implement `StringLiteral::content_eq` manually to avoid "special case" logic in `oxc_ast_tools`.
This commit is contained in:
overlookmotel 2025-01-11 02:42:05 +00:00
parent 9c1844a8a7
commit 97a7992335
5 changed files with 107 additions and 95 deletions

View file

@ -33,7 +33,7 @@ pub struct BooleanLiteral {
/// <https://tc39.es/ecma262/#sec-null-literals>
#[ast(visit)]
#[derive(Debug, Clone)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)]
#[estree(type = "Literal", via = crate::serialize::ESTreeLiteral, add_ts = "value: null, raw: \"null\" | null")]
pub struct NullLiteral {
/// Node location in source code
@ -45,7 +45,7 @@ pub struct NullLiteral {
/// <https://tc39.es/ecma262/#sec-literals-numeric-literals>
#[ast(visit)]
#[derive(Debug, Clone)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ESTree)]
#[estree(type = "Literal", via = crate::serialize::ESTreeLiteral)]
pub struct NumericLiteral<'a> {
/// Node location in source code
@ -66,7 +66,7 @@ pub struct NumericLiteral<'a> {
/// <https://tc39.es/ecma262/#sec-literals-string-literals>
#[ast(visit)]
#[derive(Debug, Clone)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ESTree)]
#[estree(type = "Literal", via = crate::serialize::ESTreeLiteral)]
pub struct StringLiteral<'a> {
/// Node location in source code
@ -85,7 +85,7 @@ pub struct StringLiteral<'a> {
/// BigInt literal
#[ast(visit)]
#[derive(Debug, Clone)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ESTree)]
#[estree(type = "Literal", via = crate::serialize::ESTreeLiteral, add_ts = "value: null, bigint: string")]
pub struct BigIntLiteral<'a> {
/// Node location in source code
@ -103,7 +103,7 @@ pub struct BigIntLiteral<'a> {
/// <https://tc39.es/ecma262/#sec-literals-regular-expression-literals>
#[ast(visit)]
#[derive(Debug)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ContentEq, ContentHash, ESTree)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut, ESTree)]
#[estree(
type = "Literal",
via = crate::serialize::ESTreeLiteral,
@ -141,7 +141,7 @@ pub struct RegExp<'a> {
/// This pattern may or may not be parsed.
#[ast]
#[derive(Debug)]
#[generate_derive(CloneIn, ContentEq, ContentHash, ESTree)]
#[generate_derive(CloneIn, ESTree)]
pub enum RegExpPattern<'a> {
/// Unparsed pattern. Contains string slice of the pattern.
/// Pattern was not parsed, so may be valid or invalid.

View file

@ -30,13 +30,6 @@ impl fmt::Display for BooleanLiteral {
}
}
impl ContentHash for NullLiteral {
#[inline]
fn content_hash<H: Hasher>(&self, state: &mut H) {
Hash::hash(&Option::<bool>::None, state);
}
}
impl fmt::Display for NullLiteral {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@ -82,10 +75,45 @@ impl NumericLiteral<'_> {
}
}
impl ContentEq for NumericLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
// Note: `f64::content_eq` uses `==` equality.
// `f64::NAN != f64::NAN` and `0.0 == -0.0`, so we follow the same here.
// If we change that behavior, we should alter `content_hash` too.
ContentEq::content_eq(&self.value, &other.value)
}
}
impl ContentHash for NumericLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.base, state);
ContentHash::content_hash(&self.raw, state);
// `f64` does not implement `Hash` due to ambiguity over what is the right way to hash NaN
// and +/- zero values.
//
// # NaN
// IEEE 754 defines a whole range of NaN values.
// https://doc.rust-lang.org/std/primitive.f64.html#associatedconstant.NAN
// We can ignore that complication here as the rule is that 2 types which are equal (`==`)
// must hash the same.
// `ContentEq` uses `==` equality, and even the same NaN doesn't equal itself!
// `f64::NAN != f64::NAN`
// So it doesn't matter if two NaNs have the same hash or not.
//
// # Zero
// `ContentEq` uses `==` equality, which considers `0.0` and `-0.0` equal, so we must make
// them hash the same.
// But `(0.0).to_bits() != (-0.0).to_bits()`, so we need to convert `-0.0` to `0.0`.
//
// # Infinity
// `f64::INFINITY != -f64::INFINITY` and `f64::INFINITY.to_bits() != (-f64::INFINITY).to_bits()`
// so infinity needs no special handling.
//
// Whatever the value, we convert to `u64` using `to_bits`, and hash that.
let mut value = self.value;
if value == -0.0 {
value = 0.0;
}
let value = value.to_bits();
std::hash::Hash::hash(&value, state);
}
}
@ -121,6 +149,18 @@ impl StringLiteral<'_> {
}
}
impl ContentEq for StringLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.value, &other.value)
}
}
impl ContentHash for StringLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.value, state);
}
}
impl AsRef<str> for StringLiteral<'_> {
#[inline]
fn as_ref(&self) -> &str {
@ -142,12 +182,36 @@ impl BigIntLiteral<'_> {
}
}
impl ContentEq for BigIntLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.raw, &other.raw)
}
}
impl ContentHash for BigIntLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.raw, state);
}
}
impl fmt::Display for BigIntLiteral<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.raw.fmt(f)
}
}
impl ContentEq for RegExpLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.regex, &other.regex)
}
}
impl ContentHash for RegExpLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.regex, state);
}
}
impl fmt::Display for RegExp<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "/{}/{}", self.pattern, self.flags)
@ -203,6 +267,32 @@ impl<'a> RegExpPattern<'a> {
}
}
impl ContentEq for RegExpPattern<'_> {
fn content_eq(&self, other: &Self) -> bool {
let self_str = match self {
Self::Raw(s) | Self::Invalid(s) => *s,
Self::Pattern(p) => &p.to_string(),
};
let other_str = match other {
Self::Raw(s) | Self::Invalid(s) => *s,
Self::Pattern(p) => &p.to_string(),
};
self_str == other_str
}
}
impl ContentHash for RegExpPattern<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
let self_str = match self {
Self::Raw(s) | Self::Invalid(s) => s,
Self::Pattern(p) => &&*p.to_string(),
};
ContentHash::content_hash(self_str, state);
}
}
impl fmt::Display for RegExpPattern<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {

View file

@ -27,33 +27,6 @@ impl ContentEq for NullLiteral {
}
}
impl ContentEq for NumericLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.value, &other.value)
&& ContentEq::content_eq(&self.base, &other.base)
}
}
impl ContentEq for StringLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.value, &other.value)
}
}
impl ContentEq for BigIntLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.raw, &other.raw)
&& ContentEq::content_eq(&self.base, &other.base)
}
}
impl ContentEq for RegExpLiteral<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.regex, &other.regex)
&& ContentEq::content_eq(&self.raw, &other.raw)
}
}
impl ContentEq for RegExp<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.pattern, &other.pattern)
@ -61,25 +34,6 @@ impl ContentEq for RegExp<'_> {
}
}
impl ContentEq for RegExpPattern<'_> {
fn content_eq(&self, other: &Self) -> bool {
match self {
Self::Raw(it) => match other {
Self::Raw(other) if ContentEq::content_eq(it, other) => true,
_ => false,
},
Self::Invalid(it) => match other {
Self::Invalid(other) if ContentEq::content_eq(it, other) => true,
_ => false,
},
Self::Pattern(it) => match other {
Self::Pattern(other) if ContentEq::content_eq(it, other) => true,
_ => false,
},
}
}
}
impl ContentEq for Program<'_> {
fn content_eq(&self, other: &Self) -> bool {
ContentEq::content_eq(&self.source_type, &other.source_type)

View file

@ -23,25 +23,8 @@ impl ContentHash for BooleanLiteral {
}
}
impl ContentHash for StringLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.value, state);
ContentHash::content_hash(&self.raw, state);
}
}
impl ContentHash for BigIntLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.raw, state);
ContentHash::content_hash(&self.base, state);
}
}
impl ContentHash for RegExpLiteral<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.regex, state);
ContentHash::content_hash(&self.raw, state);
}
impl ContentHash for NullLiteral {
fn content_hash<H: Hasher>(&self, _: &mut H) {}
}
impl ContentHash for RegExp<'_> {
@ -51,17 +34,6 @@ impl ContentHash for RegExp<'_> {
}
}
impl ContentHash for RegExpPattern<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&discriminant(self), state);
match self {
Self::Raw(it) => ContentHash::content_hash(it, state),
Self::Invalid(it) => ContentHash::content_hash(it, state),
Self::Pattern(it) => ContentHash::content_hash(it, state),
}
}
}
impl ContentHash for Program<'_> {
fn content_hash<H: Hasher>(&self, state: &mut H) {
ContentHash::content_hash(&self.source_type, state);

View file

@ -87,10 +87,6 @@ fn derive_struct(def: &StructDef) -> (&str, TokenStream) {
.iter()
.filter(|field| {
let Some(name) = field.name.as_ref() else { return false };
if name == "raw" && matches!(def.name.as_str(), "NumericLiteral" | "StringLiteral")
{
return false;
}
!IGNORE_FIELDS
.iter()
.any(|it| name == it.0 && field.typ.name().inner_name() == it.1)