srcref diff is shown for S7 classes if there are other differences

[moodymudskipper]

# as expected
library(S7)
#> Warning: package 'S7' was built under R version 4.4.1
fun <- function() {}
waldo::compare(fun, structure(fun, srcref = 1))
#> ✔ No differences
waldo::compare(fun, structure(fun, srcref = 1, foo = 2))
#> `attr(old, 'foo')` is absent
#> `attr(new, 'foo')` is a double vector (2)
waldo::compare(S7_object, structure(S7_object, srcref = 1))
#> ✔ No differences
# unexpected, shouldn't show srcref diff
waldo::compare(S7_object, structure(S7_object, srcref = 1, foo = 2))
#> `attr(old, 'foo')` is absent
#> `attr(new, 'foo')` is a double vector (2)
#> 
#> `attr(old, 'srcref')` is absent
#> `attr(new, 'srcref')` is a double vector (1)

^{Created on 2025-06-03 with reprex v2.1.0}

if there is no difference apart from srcref in compare_structure() we don't go further than the is_identical() check but if we do we enter the S7 handling part and attrs() doesn't dismiss the "srcref" from the inspection.

Happy to do a PR if it helps.

[hadley]

Can you give me a bit more context here? waldo currently only removes srcrefs from functions (which I think makes sense).

[moodymudskipper]

It doesn't remove srcref from all functions however, S7_Object is a function and its srcref is not removed in waldo::compare(S7_object, structure(S7_object, srcref = 1, foo = 2)).

Maybe you intended is.object(x) to test typeof(x) == "object" ? is.object() tests for the OBJECT bit that is always set for classed objects, so in fact your condition is.object(x) && inherits(x, "S7_object") is redundant (is.object(structure(NA, class = "S7_object")) is TRUE).

In compare_structure() we find in order:

• the is_identical() check, that checks if closures (S7 or not) are identical when we remove the srcref, causing waldo::compare(S7_object, structure(S7_object, srcref = 1)) to show no diff
• the is.object(x) && inherits(x, "S7_object") check, that does not remove the srcref, causingwaldo::compare(S7_object, structure(S7_object, srcref = 1, foo = 2)) to show shows both the foo difference but also the srcref diff despite S7_object being a closure.
• the is.closure() check that does remove the srcref, but not all closures get there

IMO srcrefs should be removed from all closures by default, S7 or not, and if we don't do it I believe is_identical() should have an exception.

[moodymudskipper]

I was wrong above to assume is.object() was redundant, inherits() fails on CHARSXP so it was indeed necessary.