Auto infer namespaces on struct and enum instantiations

It's been previously proposed that when the type is known, to allow eliding it with _ . E.g.

Great! Could you give me a link to this RFC?

Could you provide an example? I was unaware that struct pattern matching had no visibility restrictions.

This currently works:

pub mod a {
    pub enum A {
        Foo,
        Bar
    }
}

fn main() {
    let f = a::A::Foo;
    match f {
        Foo => println!("F"),
        Bar => println!("B"),
    }
}

EDIT: I misunderstood the warning description. This does not work.

The second arm is unreachable, because Foo and Bar are not matching on the variants.

2 Likes

The second arm is unreachable, because Foo and Bar are not matching on the variants.

Hmm, that's sad. It looks like I misunderstood the warning description. I fix the post.

I don't think it's ever made it to an RFC. Here's a recent discussion: Short enum variant syntax in some cases - #5 by scottmcm (with a link to the discussion before that...)

I've also pondered using . for it, kinda like Swift, to avoid it looking like "path elision" (which doesn't exist, and probably never should -- no std::_::Add, for example).

With that your example would then be

msg::Msg::ViewportMouse(.{ pos: MousePos, action: .Down })
2 Likes

Thanks, I have read the discussion. This swift syntax is even better than that I've proposed. I update the main post.

I see there aren't any clues about not having this feature, except one: I've read, changing the definition does not always produce errors in instantiations. For example:


// We define two similar but different enums:
enum IsSomething {
    True,
    False
}

enum IsSomethingElse {
    True,
    False
}

// We define a struct with one of the enums
struct A {
   is_something: IsSomething
}

// Later, we change the definition
struct A {
   is_something: IsSomethingElse
}

// The instantiation will not change:
A {
   is_something: .True
}

This is reported currently because we need to change the IsSomething to IsSomethingElse in every instantiation. After this change, this kind of modification will not be reported, which theoretically could be a problem because we said yes to a different subject.

In practice, is_something would possibly be changed as well to is_something_else, which would cause errors, so that would be good. We cannot be 100% sure tho because it is possible to change the type without the parameter name.

Besides this, I think the situation is rare. In contrast, instantiating an enum is something that every Rust program has, so I would still love to see this simplification.

A more common but similar situation is to rename a struct, which would not be a problem because the struct has the same meaning. That would be even better because we only change the definitions.

1 Like

The commonality of enums also works against this simplification. Given that most Rust code is written to avoid stuttering (Enum::Variant is preferable to Enum::EnumVariant), name collisions are not uncommon -- infact, very likely -- because people are designing enums under the assumption that the variant need not indicate which enum it belongs to, because it is always namespaced. So you're effectively going to reverse that preference.

('Going to' is perhaps too strong. But this would create a good reason to stutter where few currently exist.)

3 Likes

This is definitely my biggest concern with the enum version. It would be very possible for currently readable things to become a confusing call like process_request(.Skip, .Error) that made sense when the variants had their types there too.

(OTOH, people could currently do that by importing the variants, and they generally don't, so it might be ok to trust people to put the type names there if it's important for readability. And this isn't a problem in struct literals -- ProcessRequest { cookie_handling: .Skip, redirect_handling: .Error } doesn't have the problem that the function call does.)

1 Like

I can't speak to what people do, but this is some of my recent enum heavy code:

Example code
impl<T> Expr<T> where T: Eval {
    fn simplify(self) -> Self {
        use Expr::*;
        
        match self {
            Not(p) => match *p {
                Not(q) => q.simplify(),
                q      => Not(Box::new(q.simplify())),
            }
            And(a, b) => {
                let a = a.simplify();
                let b = b.simplify();
                if a == b { a } else { And(Box::new(a), Box::new(b)) }
            },
            Or(a, b) => {
                let a = a.simplify();
                let b = b.simplify();
                if a == b { a } else { Or(Box::new(a), Box::new(b)) }
            },
            _ => self,
        }
    }

    fn pushdown_not(self) -> Self {
        use Expr::*;
        if let Not(expr) = self {
            match *expr {
                Bool(p) => Not(Box::new(Bool(p))),
                Not(p) => p.pushdown_not(),
                Or(a, b) => And(Box::new(Not(a)), Box::new(Not(b))),
                And(a, b) => Or(Box::new(Not(a)), Box::new(Not(b))),
            }
        } else {
            self
        }
    }

    fn distribute_and(self) -> Self {
        use Expr::*;
        if let And(a, b) = self {
            match (*a, *b) {
                (p, Or(q, r)) |
                (Or(q, r), p) => Or(
                    Box::new(And(Box::new(p.clone()), q)),
                    Box::new(And(Box::new(p), r))),
                (a, b) => And(Box::new(a), Box::new(b))
            }
        } else {
            self
        }
    }

    fn distribute_or(self) -> Self {
        use Expr::*;
        if let Or(a, b) = self {
            match (*a, *b) {
                (p, And(q, r)) |
                (And(q, r), p) => And(
                    Box::new(Or(Box::new(p.clone()), q)),
                    Box::new(Or(Box::new(p), r))),
                (a, b) => Or(Box::new(a), Box::new(b))
            }
        } else {
            self
        }
    }
}

It's very hard for me to see any advantage in trying to simplify this. It's already simple. Adding more symbols and contextual prefixes into the mix seems like it would increase the opportunity for beginner confusion and line noise, without making this kind of code any clearer. If I'm not importing the enum variants, it's either because (a) they are ambiguous in the first place, and this feature wouldn't solve that or (b) I'm not using very many variants, in which case, I'd rather have the enum type for clarity rather than save a few chars.

3 Likes

Here is an example of our case:

Example
fn handle_mouse_down(e: &web_sys::MouseEvent) -> Option<msg::Msg> {
    e.stop_propagation();
    let pos = get_mouse_point(e);
    let pos_on_window = get_mouse_point_window(e);
    let target = e.target().unwrap().unchecked_into::<web_sys::Element>();

    if let Some(draggable) = web_sys_utils::find_parent(target.clone(), "draggable") {
        let node_id = web_sys_utils::get_data::<usize>(&draggable, "id")
            .expect("Currently, every draggable element should have an id");
        if draggable.class_list().contains("node") {
            Some(msg::Msg::ItemMouse(item_mouse::Msg {
                pos,
                action: item_mouse::Action::Down,
                item_type: item_mouse::ItemType::NameDescriptionSpace,
                id: node_id,
            }))
        } else if draggable.class_list().contains("node__linker") {
            Some(msg::Msg::WindowMouse(window_mouse::Msg {
                pos: pos_on_window,
                action: window_mouse::Action::DownOnLinker(node_id),
            }))
        } else {
            unimplemented!("You can only drag nodes and linkers currently");
        }
    } else if let Some(relation) = web_sys_utils::find_parent(target, "relation") {
        let relation_id = web_sys_utils::get_data::<usize>(&relation, "id")
            .expect("Currently, every relation element should have an id");
        Some(msg::Msg::ItemMouse(item_mouse::Msg {
            pos,
            action: item_mouse::Action::Down,
            item_type: item_mouse::ItemType::Relation,
            id: relation_id,
        }))
    } else {
        Some(msg::Msg::ViewportMouse(viewport_mouse::Msg {
            pos,
            action: viewport_mouse::Action::Down,
        }))
    }
}

which could be:

Example after this feature
fn handle_mouse_down(e: &web_sys::MouseEvent) -> Option<Msg> {
    e.stop_propagation();
    let pos = get_mouse_point(e);
    let pos_on_window = get_mouse_point_window(e);
    let target = e.target().unwrap().unchecked_into::<web_sys::Element>();

    if let Some(draggable) = web_sys_utils::find_parent(target.clone(), "draggable") {
        let node_id = web_sys_utils::get_data::<usize>(&draggable, "id")
            .expect("Currently, every draggable element should have an id");
        if draggable.class_list().contains("node") {
            Some(Msg::ItemMouse(. {
                pos,
                action: .Down,
                item_type: .NameDescriptionSpace,
                id: node_id,
            }))
        } else if draggable.class_list().contains("node__linker") {
            Some(Msg::WindowMouse(. {
                pos: pos_on_window,
                action: .DownOnLinker(node_id),
            }))
        } else {
            unimplemented!("You can only drag nodes and linkers currently");
        }
    } else if let Some(relation) = web_sys_utils::find_parent(target, "relation") {
        let relation_id = web_sys_utils::get_data::<usize>(&relation, "id")
            .expect("Currently, every relation element should have an id");
        Some(Msg::ItemMouse(. {
            pos,
            action: .Down,
            item_type: .Relation,
            id: relation_id,
        }))
    } else {
        Some(Msg::ViewportMouse(. {
            pos,
            action: .Down,
        }))
    }
}

The second case would be much more readable.

In the first case, it is hard to see the logic behind the code, because there are so many meaningless words. When I read the code it is quite similar to when I open /dev/sda1 in less, I would probably see some meaningful ASCII words, but the rest is something like this:

t^@^B^H ^@^B^@^@^@^@<F8>^@^@ 
^@@^@^@^H^@^@^@^@^P^@^@^D^@^@^@^@^@^@^B^@^@^@^A^@^F^@^@^@^@^@^@^
@^@^@^@^@^@^@<80>^A)=<B4>^F<D0>NO NAME    FAT32   ^N^_<BE>w|<AC>"
<C0>t^KV<B4>^N<BB>^G^@<CD>^P^<EB><F0>2<<E4><CD>^V<CD>^Y<EB><FE>T

(Of course, it is much better in our case.)

I think it's worth allowing this. Some other code examples would be beneficial tho.

This is how I would write that code:

Rewritten
use web_sys::MouseEvent;
use web_sys::Element;
use msg::Msg;

// These should be scoped to the function unless they're used in multiple places.
use window_mouse::Msg as WindowMouseMsg;
use window_mouse::Action as WindowMouseAction;
use item_mouse::Msg as ItemMouseMsg;
use item_mouse::Action as ItemMouseAction;
use item_mouse::ItemType;
use viewport_mouse::Msg as ViewportMouseMsg;
use viewport_mouse::Action as ViewportMouseAction;
use web_sys_utils::get_data;
use web_sys_utils::find_parent;


fn handle_mouse_down(e: &MouseEvent) -> Option<Msg> {
    e.stop_propagation();
    let pos = get_mouse_point(e);
    let pos_on_window = get_mouse_point_window(e);
    let target = e.target().unwrap().unchecked_into::<Element>();

    if let Some(draggable) = find_parent(target.clone(), "draggable") {
        let node_id = get_data::<usize>(&draggable, "id")
            .expect("Currently, every draggable element should have an id");
            
        if draggable.class_list().contains("node") {
            Some(Msg::ItemMouse(ItemMouseMsg {
                pos,
                action: ItemMouseAction::Down,
                item_type: ItemType::NameDescriptionSpace,
                id: node_id,
            }))
        } else if draggable.class_list().contains("node__linker") {
            Some(Msg::WindowMouse(WindowMouseMsg {
                pos: pos_on_window,
                action: WindowMouseAction::DownOnLinker(node_id),
            }))
        } else {
            unimplemented!("You can only drag nodes and linkers currently");
        }

    } else if let Some(relation) = find_parent(target, "relation") {
        let relation_id = get_data::<usize>(&relation, "id")
            .expect("Currently, every relation element should have an id");
        Some(Msg::ItemMouse(ItemMouseMsg {
            pos,
            action: ItemMouseAction::Down,
            item_type: ItemType::Relation,
            id: relation_id,
        }))

    } else {
        Some(Msg::ViewportMouse(ViewportMouseMsg {
            pos,
            action: ViewportMouseAction::Down,
        }))
    }
}

This is the rewrite that your proposed feature is supposed to improve. I argue that it doesn't, and that this code is as readable as any Rust code out there.

Rust already has a notoriety for line noise. It doesn't need another noisy syntax and inference mechanism to solve a superficial problem.

Which you've replaced with meaningless symbols. But those words aren't meaningless, they tell the reader what your data looks like and where to go to understand it. Hiding that does not make the code more readable, it just makes it more contextual and more terse.

Where in the code am I supposed to find out what a Msg::ViewportMouse takes for an argument? I can see in your original code that it takes a viewport_mouse::Msg. In your rewrite, I see that it takes a .. Off to read the docs we go.

More over, things like Msg::ItemMouse(ItemMouseMsg { .. }) Should be replaced with Msg::from calls. It is obviously trivial to convert an item_mouse::Msg into a msg::Msg, so implement that conversion. That gets rid of all of your redundant wrapping.

Overall, the point is thus: you have many ways to improve your code. Use them first, then after you can clearly demonstrate that none of them work, propose a language change.

1 Like

Thanks for improving my code.

Rust already has a notoriety for line noise. It doesn't need another noisy syntax and inference mechanism to solve a superficial problem.

Is that mean any syntactic sugar proposal would be discarded?

it just makes it more contextual and more terse.

Indeed, however, I think it is part of the readability.

Where in the code am I supposed to find out what a Msg::ViewportMouse takes for an argument? I can see in your original code that it takes a viewport_mouse::Msg . In your rewrite, I see that it takes a . . Off to read the docs we go.

IDE could know that what is the ., and the parent is always there.

Overall, the point is thus: you have many ways to improve your code. Use them first, then after you can clearly demonstrate that none of them work, propose a language change.

Although it works, the proposed syntax is less verbose, and faster than aliasing every type.

None of the code presented in this thread is being read in an IDE. None of the code I write is written in an IDE. If Rust changes such that I can't read or write reliable and clear code without one, I'm not getting an IDE, I'm finding a better language. (Anyway, If your IDE is a solution, make it patch up your unreadable code for you. Make it hide the redundancies and silently swap . for inferred types & variants. You don't need the compiler to do it.) This is a general rule: any argument for improving the language is only weakened by the willingness to assume an IDE is available; because now you can always consider improving the IDE instead.

Besides, you're contradicting yourself. The code cannot be more readable and also need an IDE to better understand. Those are in direct conflict with each other.

2 Likes

There's two levels of readability involved here.

  1. I want a high-level overview of what the code is doing. Here, the takes_enum(Enum::Variant(EnumVariant { .. })) is line noise compared to takes_enum(.Variant(_ { .. })), because takes_enum is enough context to infer the enum argument, and EnumVariant is only used for the definition of that one enum variant.

  2. I want to understand the exact types involved, so that I can locate and edit them as necessary, and trace the exact execution path of the code. Here, having the types spelled out helps.

But I think we should career to 1 over 2. Specifically, because we already have type inference. What is the type of eggs in let eggs = foo_and_then_bar(&mut spam);? You don't know until you check the signature of foo_and_then_bar.

It doesn't hurt the explicitness of Rust to extend inference to the types of arguments. Explicit is not noisy, burdensome, not even local, after all. In addition, Rust does not have function overloading, so it's always clear from the function in question what the type of the argument is (otherwise type inference wouldn't kick in in the first place).

It comes down to that if you elide too much information, your code is harder to read. It's not the language's fault if you do that, though. The inference/elision discussed in these proposals is clearly beneficial in some notable cases (the biggest one being giving us effective kwargs for free and in a backwards compatible way), and when names are too generic, continue to spell out the paths/types/etc.

Does a type in the following example add any useful information to the code useful to understanding what it does? I would argue, no, it doesn't, unless your specific purpose is to edit the type, in which case you'll want to edit the function as well, so jumping to the function first doesn't even add any extra steps.

let call = RemoteCall::from_config(_ {
    target: "https://example.com/api",
    fallback: "https://example.com/bpi",
    timeout: Duration::from_secs(10),
    ..
})?;
let result = call.submit().await?;
The best typed without new inference placeholders alternative, without adding Ng builder types
let call: RemoteCall = remote_call::Config {
    target: "https://example.com/api",
    fallback: "https://example.com/bpi",
    timeout: Duration::from_secs(10),
    ..
}.try_into()?;
let result = call.submit().await()?;

Not horrible, but also lacks the power of the dot as well as doc discovery ("I'm on the page for RemoteCall, but how do I make one?").

4 Likes

That's something that could be fixed in the docs. I would say, it should be, because Rust's docs are awful in a number of ways: chief among them is the inability to build a prioritized and cohesive picture of how to use an object or trait, but it is also difficult to hide irrelevant details and show important ones. For instance, I went here to implement this trait, but the declaration is hidden by default, (sometimes doesn't even include the full definition), and I'm stuck staring at a worthless example of how to use it if I had already had an implementation instead. Traits should have an example implementation 'template', impls should be grouped, the sidebar should have better grouping, and I shouldn't have to click "unhide" multiple times to see something new, and anything under a doc header should probably be hidden by default unless there is nothing above it.

Traits are used much more often then they're implemented, of course the default page layout is going to be biased for that. Functions on the page are presented in the order they are in the source file (exception: required trait methods are hoisted above provided methods), and (for structs) grouped by what impl block they're in, complete with the comment on said impl block.

Rust's generated documentation is miles above what any scripting language has (basically, just semi structured ways to describe your API in prose), and the only thing I could argue that javadoc/doxygen do better is putting a summary of all the methods' signatures front and center, but that's partly because that's most of the useful information you'll get out of a javadoc/doxygen generated documentation.

I fail to see how both of these things can be true simultaneously. Either it's collapsed by default and you need to unhide it, or you don't need to unhide it, it's uncollapsed.

Either way, this isn't the thread to discuss rustdoc. If you have concrete constructive proposals on how to improve rustdoc and/or concrete stdlib documentation, please do open a new thread, though. As a community we're highly invested in making our docs as good as they can be. (Just do keep in mind that rustdoc docs are API reference docs, and their design caters to that over a more guide style book like is common for JS libs on e.g. readthedocs.)

1 Like

See here:

There are two [-] expansion boxes. Clicking on the first reveals nothing but the other. One shouldn't have to expand two different frames to see only a single line of text.

3 Likes

Yes, I'd love this. Swift has shown that this can be used to make very nice APIs.

6 Likes

To echo @skysch's reply, I read quite a lot of code on websites (mainly when reviewing code, but also occasionally found through searching). There are no IDE facilities in such places, so expecting IDE support for readability is not something I'd like to see at least.

6 Likes

The problem of enum variants being used out of context already exist via use (and re-exports can further obfuscate what is used). Code authors can always make bad choices and write hard to understand code, but when someone publishes code on a website, it's in their interest to make it readable, so you can expect them to choose a syntax that is clear. The long fully-qualified version will always exist, and be available where needed.

When the abbreviated syntax is a normal part of the language, you can expect authors to start taking it into account. When the current syntax is EnumName::Variant, then people designing APIs might want to avoid repeated words, and maybe name variants like AreDogsGood::Yes. But when the alternative syntax becomes a supported option, authors will have to consider it, and name things accordingly, like WhoIsGood::DogsAreGood, so that users can write .DogsAreGood instead of .Yes. This is already the case in Swift — . syntax is common, so APIs don't suffer because of it, the APIs are designed to be most clear with it.

The important aspect is that it can improve readability. It can even improve readability over current Rust status quo, because better enum ergonomics may encourage API designers use enum instead of bool and other alternatives.

6 Likes

Thanks for the replies. I updated the main post with the current cons and pros.