Add a lint for calling any safe functions at all within an unsafe block

[bstrie]

In today's Rust we warn if your unsafe block contains no unsafe code whatsoever:

fn main() {
    unsafe {
        std::io::println("Hello?");
    }
}

un.rs:2:5: 4:6 warning: unnecessary `unsafe` block, #[warn(unused_unsafe)] on by default
un.rs:2     unsafe {
un.rs:3         std::io::println("Hello?");
un.rs:4     }

This is really great. However, the following program produces no warning:

fn main() {
    let x = &["One", "Two", "Three"];
    unsafe {
        std::io::println(*x.unsafe_ref(0));
    }
}

...despite the fact that std::io::println is not an unsafe function. In the interest of reducing the scope of unsafe code, I propose a lint that would warn on the above program. And yes, satisfying it would require uglifying the code like so:

fn main() {
    let x = &["One", "Two", "Three"];
    let y;
    unsafe {
        y = *x.unsafe_ref(0);
    }
    std::io::println(y);
}

...but who ever said that we had an obligation to make unsafe code pretty?

---

As an alternative, if people find the above proposal to be too extreme, then I would ask them to consider a weaker lint that merely warned if an entire statement within an unsafe block contained no unsafe code. This weaker lint would warn on the following program which compiles without warning today:

fn main() {
    let x = &["One", "Two", "Three"];
    unsafe {
        std::io::println("Hello?");  // warn on this line, if you want a weaker lint
        std::io::println(*x.unsafe_ref(0));
    }
}

[bstrie]

A commenter on IRC has reminded me that unsafe blocks can have return values as well (it's a mystery as to why I thought they didn't), and thus my "uglified" example only needs to be reduced to the following:

fn main() {
    let x = &["One", "Two", "Three"];
    std::io::println(unsafe { *x.unsafe_ref(0) });
}

...which, in fact, is hardly ugly at all.

[brson]

@pcwalton and I have previously agreed that minimizing unsafe scope is an antipattern and its better to prefer bigger scopes that contain all the unsafety than to clutter a function up with many unsafe blocks.

[brson]

That is particularly true when one function does several unsafe ops on the same data. Better to relate all the unsafety to each other than giving the impression they are different unsafeties.

[alexcrichton]

cc @steveklabnik, can this move to the RFC repo? New features such as this should go through an RFC now.

[steveklabnik]

Since new lints have a big impact on users of rustc, the policy is that they should go through the RFC process like other user-facing changes. As such, I'm going to give this one a close, but if anyone comes across this ticket and wants this lint, consider adding it to clippy and/or writing up an RFC. Thanks!