Auto infer namespaces on struct and enum instantiations

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.

The comments about making rust-analyzer support this even if the language doesn't made me realize that rustc could support this, even if Rust doesn't, in the same way that you can use -> _ to let rustc tell you what the type should be based on the body. :thinking:

Best part is if the lang team ever decides to go for this, the implementation will already be ready. :slightly_smiling_face:

7 Likes

It turned out that aliasing the types is worse than just keeping them namespaced.

Here is a list of my problems with use some_mod::SomeStruct as SomeModSomeStruct

  • Copying the expression SomeModSomeStruct {..}.into() to another file is painful, because IDE does not know anything about these types. - They are defined on top of another file. - Whereas the some_mod::SomeStruct {..} symbols are known in the entire project because they are public. However, .{..} expressions would be perfectly fine in this case.

  • It doesn't add too much. A big heap of code compared to a bit of readability. For other files, I kept the structs namespaced because I didn't want to see 10 or more lines of aliases. Again, .{..} expressions would be perfectly fine in this case.

  • Nothing ensures consistency. Obviously, I would use those aliases everywhere wherever I use those types, and I don't want to see, e.g., 3 different symbols for the same struct. Moreover, not every file needs all the aliases, so it's even harder to maintain because, for every file, I will see different definitions. But again, .{..} expressions would be perfectly fine in this case.

I also tried to improve the code size with the * operator, and I tried to move the expressions closer to where I use them. And here is another problem. The use SomeEnum::*; is also problematic, because if I match with one of its variants, and I delete it, I won't get a compile error.

I also improved our code by using .into(). It works perfectly for enums, but I have to define the From trait to the desired type. This is not a big deal, but with .{..} I wouldn't need to implement it.

How would you not get a compile error? Do you have an example where deleting a variant of an enum all of a sudden matches on some other thing silently?

enum Foo {
    Shared,
}

struct Shared { … }

fn match_on_foo(var: ???) {
    using Foo::*;
    match var {
        Shared => …,
    }
}

In this code, deleting Foo::Shared could conflate with Shared all of a sudden, but…how does this not also have compilation errors of its own? What type for var would possibly make it still work? I guess if it was some type which impl From<ThatType> for Foo and Shared and it was match var.into(), it'd be a problem, but…that certainly seems weird.

I'm all OK with preferring namespaces, but this claim seemed odd since I couldn't see how the code could (sensibly) be structured and not be confused in some other way..

Try this out:

Code example

enum Something {
    Foo,
    Baz
}

fn main() {
    use Something::*;
    let something = Something::Baz;
    match something {
        Foo => println!("foo"),
        Bar => println!("bar"),
        Baz => println!("baz")
    }
}

You will get a warning, but in our case, we have a lot of warnings that we haven't fixed yet so I only see the errors.

I'm all OK with preferring namespaces, but this claim seemed odd since I couldn't see how the code could (sensibly) be structured and not be confused in some other way..

Unfortunately, the project that I'm working on is not public, making a new big example project is really time-consuming. HelloWorld apps could probably be simplified to println!("Hello World"), so those aren't the bests either. However, I think I can share some code sections from the previous versions:

Interesting code parts

A.rs:

use render::msg::{
    keyboard::Msg as KeyboardMsg,
    window_mouse::{Action as WindowMouseAction, Msg as WindowMouseMsg},
    Msg,
};
...

B.rs:

use render::msg::{
    item_mouse::{Action as ItemMouseAction, ItemType, Msg as ItemMouseMsg},
    viewport_mouse::{
        Action as ViewportMouseAction, Msg as ViewportMouseMsg, OuterDragAction,
        OuterDraggableElement,
    },
    window_mouse::{Action as WindowMouseAction, Msg as WindowMouseMsg},
    Msg,
};

...

WindowMouseMsg {
    pos: window_pos,
    action: WindowMouseAction::DownOnLinker(item.get_id()),
}.into()

C.rs:

use render::{
    msg::{item_mouse, keyboard, viewport_mouse, window_mouse, Msg},
    Render,
};

...

use viewport_mouse::{OuterDragAction::*, OuterDraggableElement::*};
match outer_drag {
    Over => {}
    Drop(element_type) => match element_type {
        ...
    },
};

I hope this helps. This is definitely more sensible, but of course, this is not too much. If you are thinking about something that would probably help I would answer whether I could do that or not, or what's my problem with that.

1 Like