Add CloneGetters

[SLUCHABLUB]

I copied the code for CopyGetters, changing copy to clone. I have not written any additional documentation.

closes #111

[jbaublitz]

Thanks for the submission! If you're okay with not currently supporting Arcs, I'm willing to take a look into this a bit more, but it seems like that is the main reason you submitted this PR, so if that is the driving motivation, we should probably work out how to add a feature like that that covers all use cases.

[SLUCHABLUB]

the Arc clone method can only be invoked via Arc::clone() not self.clone().

This is not the case.

You can clone an arc by invoking .clone() on a value.

let arc: Arc<str> = Arc::from("hi");
    
// one way to clone it
let clone_1: Arc<str> = Arc::clone(&arc);
    
// since `Arc::clone` comes from the `Clone` trait this is the same as above
let clone_2: Arc<str> = Clone::clone(&arc);
    
// using the clone trait againt, but with method syntax
let clone_3: Arc<str> = arc.clone();

The above code (playground) compiles without a problem. Furthermore, the clone_getters.rs test passes.

Arc docs § Cloning refrences

[SLUCHABLUB]

Correction: the clone_getters.rs test doesn't actually use Arc. (I could add that test case to the pr). However, Arc still works as motivated above.

[jbaublitz]

This may have been a change since the last time I looked at it. As I recall, the documentation used to specifically call out Arc::clone() syntax as the way to do it largely because at the time, there was a concern about the interaction with Deref and the implementation of Clone on Arc. I see that the documentation no longer states that this is the case so my information is probably just for an older version of Rust. I want to ask around a bit about whether this impacts any other reference counters and get a little bit more information on what exactly changed, and then I'll come back and review the rest of this PR. In the meantime, if you could update the documentation and tests to include the Arc case, that would be great. Thanks!

[SLUCHABLUB]

Great, will do.

[steveklabnik]

From https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#cloning-references

// The two syntaxes below are equivalent.
let a = foo.clone();
let b = Arc::clone(&foo);

What yinz were probably thinking of is the methods that use this: is a receiver (like downgrade: https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.downgrade) , which does force you to use them as an associated function, rather than as a method.

You may have been thinking of this clippy lint, which suggests using the associated function syntax, so that things are more clear: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_ref_ptr

Or, in the way olden days before 1.0, this may have been the case for a bit as well, I can't quite remember myself, but I did have to double check stuff here to post this comment!

[jbaublitz]

Thanks for clearing that up @steveklabnik! I think we can proceed with your solution @SLUCHABLUB. There is a possibility that I'm basing my recommendation off of pre-1.0 Rust, but regardless, I see from the documentation that this is not a limitation and it likely hasn't been for a while. Given that .clone() will always choose the reference counter and that is the desired behavior, I feel comfortable taking this approach.

[guzman-raphael]

🎉 A big thanks to @jbaublitz, @SLUCHABLUB, and all contributors! 🎉

This feature caught my eye for a project recently and I tried out @SLUCHABLUB's branch. I can confirm it worked as expected. Here is a small sample how we used it.

#[derive(Debug, Clone, Default, CloneGetters)]
#[getset(get_clone)]
pub struct PodJob {
    pub pod: Arc<Pod>,
}

Additionally, our project required attaching additional attributes to the generated impl block since one of our dependencies required it. Took a stab at how this could be done and planning on contributing. Curious on your thoughts and will follow-up with a PR.

[SLUCHABLUB]

Sorry it took so long, the tests are now updated.

[jbaublitz]

@SLUCHABLUB Can you take a look at CI? It looks like it's failing in a few places.

[SLUCHABLUB]

I had forgot to run cargo fmt but the errors from CI do not seem to be from the actual tests. cargo test passes and cargo clippy only emits warnings for code unrelated to this pr.

The actual error message is: Error: Missing download info for actions/cache@v1. Looking at the actions/cache repository, it seems that v1 was discontinued earlier this year.

[jbaublitz]

@SLUCHABLUB I noticed the same. Working on fixing it in #115. Once I merge that, I'm going to ask you to rebase and squash the format commit. My guess is PR review will not take long. I will try to get out a release after I merge this as soon as possible, possibly even today if the turnaround for review is quick enough.

[jbaublitz]

#115 passed and is merged. Please rebase.

[jbaublitz]

I just need to queue up a PR to update the documentation and then I'll merge.