rustype / typestate-rs

Proc-macro typestate DSL for Rust
https://rustype.github.io/typestate-rs/
Apache License 2.0
142 stars 11 forks source link

Should the inner state be public? #29

Open btrepp opened 2 years ago

btrepp commented 2 years ago

First off Amazing library!. I stumbled upon it when writing my own proc macros to try a similar goal, so very happy a much better solution already existed.

One thing I've noticed that access to the nested states and values typically does seem to be accessible.

This comes from https://github.com/rustype/typestate-rs/blob/83b0ef0a6dbd1658c283ec45be0ad611e73e8ff1/typestate-proc-macro/src/visitors/state.rs#L210 Most examples also have the inner 'states' having public variables, so these end up being accessible too.

In https://github.com/btrepp/typestate-rs/commit/eb92c15df5465a282471a21a9d9d7dadf900f22f I was able to remove pub, and fix examples to work. The biggest trick is making the implementations of the trait to be inside the mod itself, thus allowing access to any struct field without declaring them pub. This seems to keep everything nice and hidden, meaning we can only use our public apis to interact with the typestate machines. The downside is trait impls must be inside the mod block, which isn't as nice. There may be ways of working around that using a function macro instead of an attribute one.

I didn't open this as a PR yet, as perhaps there are valid reasons to have the inner state accessible, but at a glance I feel like the pattern should be forcing API consumers to follow the state transitions, and hiding it's inner properties.

Cargo expand on main lightbulb example

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2018::*;
#[macro_use]
extern crate std;
use light_bulb::*;
use typestate::typestate;
///```mermaid
///stateDiagram-v2
///[*] --> Off : screw
///Off --> On : turn_on
///Off --> [*] : unscrew
///On --> Off : turn_off
///```
mod light_bulb {
    pub struct LightBulb<State: LightBulbState> {
        pub state: State,
    }
    pub struct Off;
    pub trait OffState {
        fn screw() -> LightBulb<Off>;
        fn unscrew(self);
        #[must_use]
        fn turn_on(self) -> LightBulb<On>;
    }
    pub struct On;
    pub trait OnState {
        #[must_use]
        fn turn_off(self) -> LightBulb<Off>;
    }
    #[doc(hidden)]
    mod __private {
        pub trait LightBulbState {}
    }
    pub trait LightBulbState: __private::LightBulbState {}
    impl<__T: ?::core::marker::Sized> LightBulbState for __T where __T: __private::LightBulbState {}
    impl __private::LightBulbState for Off {}
    impl __private::LightBulbState for On {}
}
impl OffState for LightBulb<Off> {
    fn screw() -> LightBulb<Off> {
        Self { state: Off }
    }
    fn unscrew(self) {}
    fn turn_on(self) -> LightBulb<On> {
        LightBulb::<On> { state: On }
    }
}
impl OnState for LightBulb<On> {
    fn turn_off(self) -> LightBulb<Off> {
        LightBulb::<Off> { state: Off }
    }
}
fn main() {
    let bulb = LightBulb::<Off>::screw();
    let bulb = bulb.turn_on();
    let bulb = bulb.turn_off();
}

Cargo expand on my fork that changes pub state: State to state:State

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2018::*;
#[macro_use]
extern crate std;
use light_bulb::*;
use typestate::typestate;
///```mermaid
///stateDiagram-v2
///[*] --> Off : screw
///Off --> [*] : unscrew
///Off --> On : turn_on
///On --> Off : turn_off
///```
mod light_bulb {
    pub struct LightBulb<State: LightBulbState> {
        state: State,
    }
    pub struct Off;
    pub trait OffState {
        fn screw() -> LightBulb<Off>;
        fn unscrew(self);
        #[must_use]
        fn turn_on(self) -> LightBulb<On>;
    }
    pub struct On;
    pub trait OnState {
        #[must_use]
        fn turn_off(self) -> LightBulb<Off>;
    }
    impl OffState for LightBulb<Off> {
        fn screw() -> LightBulb<Off> {
            Self { state: Off }
        }
        fn unscrew(self) {}
        fn turn_on(self) -> LightBulb<On> {
            LightBulb::<On> { state: On }
        }
    }
    impl OnState for LightBulb<On> {
        fn turn_off(self) -> LightBulb<Off> {
            LightBulb::<Off> { state: Off }
        }
    }
    #[doc(hidden)]
    mod __private {
        pub trait LightBulbState {}
    }
    pub trait LightBulbState: __private::LightBulbState {}
    impl<__T: ?::core::marker::Sized> LightBulbState for __T where __T: __private::LightBulbState {}
    impl __private::LightBulbState for Off {}
    impl __private::LightBulbState for On {}
}
fn main() {
    let bulb = LightBulb::<Off>::screw();
    let bulb = bulb.turn_on();
    let bulb = bulb.turn_off();
}
jmg-duarte commented 2 years ago

Hello @btrepp! Glad to know that you found my crate interesting (I've spent a long time on it).

To provide you with a brief answer, the inner state can carry data thus I made it pub by default.

In a more complete reply - I agree that the inner data could be private, ideally, the user should have full control over it.

Addressing your impl idea. I have nothing against it. In fact, I wanted to do that from the begining. However, it raises some questions:

To materialize what I was talking about:

trait S {
    fn new() -> S;
}

impl S for Automaton<S> {
    // The signatures don't match pre-expansion and this might be confusing
    // One can also expand them so we could simply have 
    // ```
    // impl S { ... }
    // ```
    // But then the question is, what about the inside function?
    // How can the user *easily* return a valid `Automaton<S>`?
    fn new() -> Automaton<S> { 
        // ...
    }
}

Let me know your thoughts on the questions above!