Allow to shorten `use` statements in expressions

Problem

As we know in Rust there's a tendency to switch from bool to appropriately named enums, for example:

enum Something {
    Enabled(data),
    Disabled,
}

then we use if let expressions instead of if bool, which provides more context:

if let Something::Enabled(data) = something {
    ...
}

The problem is that in expressions like that enums must be imported first if they're defined in a different module or crate. Hence the above expression must be a bit more complicated:

use path_to_something::Something;
if let Something::Enabled(data) = something {
    ...
}

Another problem is that we must decide where to put this use statement: at the top of module or locally where an imported enum is used; local is better, but autocompletion always places items at the top.

Prior art

There have been proposals to automatically import enum variants in scope of patterns as well as to introduce a shortcut like _::Something(data) — both can solve problems with use statements easily. But they also introduces another set of problems either with implicitness or noisiness that not everyone likes to have.

And both proposals removes enum name, against which there have been many objections.

Solution

I propose use _::EnumName; syntax which could be use _::Struct; as well. It means that path to enum/struct has been inferred from context, so the above if let expression would be:

use _::Something;
if let Something::Enabled(data) = something {
    ...
}

There should be lints if such use statement is placed far away from usage of imported item. Also, placing such use statements shouldn't be possible in top level declarations for example at the top of module.

Advantages

Perhaps people would say that saving a few symbols is not worth the churn. But in fact there are other advantages in this way of writing the code:

  • It gives an opportunity to introduce alias like use _::Something as Smth to shorten enum name
    • This promotes to avoid glob-use e.g. use path_to_module::Something::*
  • It as well promotes to place use statements locally
    • This improves local reasoning because there's no need to refer to the top of module
    • This also improves local reasoning because with _:: we seek for the value from which path is inferred
    • And removes use statements from the top of module where currently IMO is a mess
  • Better code alignment with if let

Drawbacks

  • Obviously, it's another language item which should be implemented and taught to users.

  • Besides of that there's an opportunity to place it too far away from usage, which could make code too obscure.

  • Things like use _::Something as S looks quite verbose and complicated.

  • Also, placing multiple such use statements in a row looks bad, perhaps there should be a shortcut like:

    use _::{Something, SomethingFromOtherModule}
    

This use declaration is a separate item from the following expression, so when you say you're proposing a change of syntax "in expressions" it is confusing -- the use declaration is not part of the expression.

You're proposing a new form of use declarations that automatically figure out the module (crate?), it seemingly has nothing to do with expressions.

4 Likes

I mean something like the following shouldn't be possible to write:

mod foo {
    use _::Bar;
    ...
}

Because there would be a big gap between use _::Bar and Bar usage in the code.


Or perhaps there should be lints that prohibits the above.


Edit: changed OP to make it easier to understand.

That would essentially mean deprecating use statements. The entire point of an explicit use import is to specify where the item is coming from. That information is invaluable both for a human reader, and for automated tooling (which can typically find the correct import anyway, but would do it much faster given an explicit path). use _::Stuff; gives absolutely no information where the item is coming from, it could as well be removed entirely.

Also, what if there are multiple items with the same name at different paths? They could have the same kind (struct, enum, trait), or different, but in any case there is no way to disambiguate between them.

9 Likes

It gives an information that item's path has been inferred from the context. In that it's very similar to _::EnumVariant proposal.

When path is impossible to infer from the context then regular use statement is still useful. Also, we will limit use _:: to expressions, so regular use statements must be used at the top of module:


For example if there's snippet like this:

use _::Thing;
function(Thing::Variant);

then compiler will see which type is taken by function and will substitute it in use _::Thing.

Regardless that this enum is just another name for Option, how about an ad hoc method on it as a good workaround right now that doesn't require new syntax?

impl Something {
    pub fn is_enabled(&self) -> Option<&T> { // TODO: Fill in `T`
        match self {
            Self::Enabled(data) => Some(data),
            _ => None,
        }
    }
}

Using it in another module is just:

if let Some(data) = something.is_enabled() {
    // ...
}

I do not have any comment on the proposal as-is. Sprinkling use statements around never really bothered me, personally.

While this proposal mixes in some things that may divert attention, there is a notable asymmetry between struct and enum: When I get a value of some inferred type (e.g. a function return value) I can readily access the fields of a struct but I cannot readily destructure an enum.

The use diversion only enters this discussion because today it is necessary to fully qualify the variant names in if let, let else and match, which typically is shortened by importing the name of the enum.

However, I agree with the OP that there is no additional information or value in this precise naming, at least with the type system features we have today. When I have a value of type T in hand, its variants can only be the variants of T — there are no ambiguities.

My syntax proposal would thus be to only add _::Enabled(x) pattern syntax — which reminded me that this has been discussed previously.


The above argument changes markedly if Rust were to ever add union types (i.e. a type A | B that can be either an A or a B). In that case, it would be necessary to disambiguate variant names in patterns.

This is not to be confused with sum types (which may use the same notation) which need to distinguish both sides of A | A and therefore need notation for left/right in any case.

3 Likes

While there's no value in precise naming, I do believe that there's some value in at least some naming.

_::Enabled(x) may erase a valuable piece of information e.g.

if let Some(_::Enabled(x)) = rx.recv()

this don't gives any hint about what's exactly enabled here.


That's why I want to limit this proposal to paths only

Just want to post an alternative:

if let Something.Enabled(data) = something {
    ...
}

Here we have type annotation and hint that path has been inferred, but no use statements.


I want to add that use statements are useless for reader when they're placed too far away from enum usage or even may worsen readability because there's less information available locally.

Let take this chunk of code from alacritty:

    /// Logging filter level.
    pub fn log_level(&self) -> LevelFilter {
        match (self.quiet, self.verbose) {
            // Force at least `Info` level for `--print-events`.
            (_, 0) if self.print_events => LevelFilter::Info,

            // Default.
            (0, 0) => LevelFilter::Warn,

            // Verbose.
            (_, 1) => LevelFilter::Info,
            (_, 2) => LevelFilter::Debug,
            (0, _) => LevelFilter::Trace,

            // Quiet.
            (1, _) => LevelFilter::Error,
            (..) => LevelFilter::Off,
        }
    }

Here it's not obvious from where LevelFilter comes from.

And now with this proposal:

    /// Logging filter level.
    pub fn log_level(&self) -> log::LevelFilter {
        match (self.quiet, self.verbose) {
            // Force at least `Info` level for `--print-events`.
            (_, 0) if self.print_events => LevelFilter.Info,

            // Default.
            (0, 0) => LevelFilter.Warn,

            // Verbose.
            (_, 1) => LevelFilter.Info,
            (_, 2) => LevelFilter.Debug,
            (0, _) => LevelFilter.Trace,

            // Quiet.
            (1, _) => LevelFilter.Error,
            (..) => LevelFilter.Off,
        }
    }

That's not a possible syntax[1] because it's already valid syntax. LevelFilter.Warn means access the value named LevelFilter and get its Warn member.

struct nonstandard_style {
    Warn: i32,
}

let LevelFilter = nonstandard_style {
    Warn: 0xcad97,
}

dbg!(LevelFilter.Warn);

  1. Technically speaking, it's not available in expression position but could technically be allowed as a pattern. However, making syntax valid in both pattern and expression position without maintaining the dual semantics is still almost certainly a nonstarter. ↩︎

1 Like

Cannot we resolve this ambiguity somehow? For example when compiler sees that this is an enum then it tries to infer path and access it's variant, otherwise field access is implied

Although, enabling the same syntax in expressions might be bad idea. Again here's the snippet from alacritty:

impl From<Event> for WinitEvent<'_, Event> {
    fn from(event: Event) -> Self {
        WinitEvent::UserEvent(event)
    }
}

the proposed syntax suggests that it should be this instead:

impl From<Event> for winit::event::Event<'_, Event> {
    fn from(event: Event) -> Self {
        Event.UserEvent(event)
    }
}

and this becomes quite confusing TBH, so it's better to rewrite it back.


That said, too much friction on the same place.

Path inference for enums in pattern position

Summary

This proposal introduces EnumName.Variant patterns where . means that path to EnumName has been inferred from context so there's no use path::to::EnumName at the top.

Motivation

For example we can readily check on struct fields:

if some_struct.field {
    ...
}

but we cannot readily check on enum variants without use statements:

use path::to::EnumName:
if let EnumName::Variant = some_enum {
    ...
}

That becomes quite annoying, especially because it's considered a good practice to switch from bool to appropriately named enums like:

enum Something {
    Enabled(data),
    Disabled,
}

In addition IDE autocompletion usually places imported items at the top of module, so it may become contaminated with enums while diverting attention from more important items.


This proposal makes matching on values of enums as easy as matching on values of structs:

if let EnumName.Variant = some_enum {
    ...
}

There already are places where dual semantics isn't preserved, for example ref isn't available in expressions as well as .. has different implications in both places.

Since the proposed syntax is the most useful in patterns and leads to confusing code in expressions I think it's fine for it being restricted to patterns only.


Perhaps the biggest problem would be preserving dual semantics with structs, for example someone may want:

let Struct. { fields, .. } = some_struct;

and that doesn't look pretty

I was initially skeptical that this feature would substantially improve anything. But I have found a case in the wild where the existing behavior could be potentially categorized as a bug, and this feature would work around it. (Another alternative solution for the bug is adding a compiler warning or lint when the condition is detected.) I spent some time searching through clippy and rust issues and Tracking issue for RFC #2145: Type privacy and private-in-public lints · Issue #48054 · rust-lang/rust · GitHub is the relevant tracking issue AFAICT. Specifically, "lint #3 unnameable_types".

When a function is made public, it requires that all types in its signature are also public. This works in most cases, but fails when the type is an enum that is public to a sub-module but not exported from the crate. A simple minimal repro is something like this:

pub use crate::sub::some_function;

mod sub {
    #[derive(Debug, PartialEq)]
    pub enum SomeEnum {
        One,
        Two,
    }

    pub fn some_function(a: i32, b: i32) -> SomeEnum {
        if a == b {
            SomeEnum::One
        } else {
            SomeEnum::Two
        }
    }
}

When using this crate as a dependency, one cannot use pattern matching on the enum or its PartialEq impl to check the return value.

I don't know if the unnameable_types lint has been discussed WRT enums, but this is a fairly disruptive issue given how it can silently creep in by accident.

2 Likes

Rustc provides an opt-in #![warn(unreachable_pub)] to help protect against cases like this (though there are still false positives when chaining reëxports) though that requires using pub(crate) when you do want the type to not be extern-public, which some people don't like using.

An unnameable_types lint will have the same false positives that unreachable_pub has, so tracking those down would be important for unnameable_types.

Additionally, unnameable types are semi-regularly deliberately used to control how an API can be used, so I'd be cautious about making unnameable types constructable. Though those cases I think all use structs, so while I'd perhaps worry about making unit structs constructable, it's probably fine for enums.

priv_in_pub workarounds are on fairly shaky grounds anyway if they're being used for soundness rather than just making undocumented-but-pub-for-impl-reasons API more difficult to use accidentally.

Honestly, I can't tell if you brought up #![warn(unreachable_pub)] because it's supposed to create a warning for the repro (it doesn't AFAICT) or because it has some similarities with unnameable_types. Regardless, I'm not trying to rehash old topics or argue against the false positives that these lints have. But I thought the distinction between unnameable enums and unnameable structs is important enough to call it out explicitly. (And I have not seen this mentioned before; please correct me if I am mistaken.) In particular, the OP describes the issue precisely:

Combine this with the fact that unnameable enums cannot be imported by definition. This logic leads me to believe that unnameable enums are in a special position to provide adequate motivation for this feature request [1], versus "it saves typing a use statement" in the original theory. Primarily because unnameable enums are easy to produce by mistake and there is no mechanism for warning of their existence.

I am attempting to make the case that unnameable enums (ignoring unnameable structs for the moment) lend some weight and credibility to this proposal that otherwise is very weak, IMHO. I started my last reply saying that I discovered this situation in a public crate in the wild. And having been aware of this thread at the time, I knew that the feature would have made the unnameable enum a non-issue. No need to send a PR, no need to introduce a potentially breaking change to the crate [2]. Just pattern-match the unnameable enum with special syntax that does not require naming it.


  1. Disregarding for brevity that trait implementations and inherent methods on an unnameable enum can still be used after gaining access to one. The enum cannot be pattern matched, constructed, or destructured for comparison purposes without a use statement. If you're lucky, the crate might give you a Default or PartialEq that may be used in limited cases. Or perhaps string comparisons with Display or Debug could work around the issue. Failing that, your only other option is if the enum has inherent methods that can take any of the particular roles of construction or comparison. But these will be even less ergonomic and less idiomatic than pattern matching and comparison operators WRT PartialEq and PartialOrd. And, well, construction of unnameable types may be disallowed for other purposes. ↩︎

  2. This is also rife for false positives. Exporting a previously private symbol can create new problems in specific circumstances. For example, if the type is private on purpose for soundness reasons as you proposed in one scenario. ↩︎

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.