suspicious lint: functions ending in _unchecked that are not marked as unsafe

[gilescope]

What it does

check function names that end _unchecked and then squark if they are not marked as unsafe.

Maybe I'm totally out of line here but I think if you're saying 'trust me' it should be marked unsafe so you can say why it's safe to use the unchecked version.

Lint Name

safe_unchecked

Category

correctness, suspicious

Advantage

The suggestion would be to add unsafe to the function call. That could cause knock on compilation failures but they would have to be for the dev to fix manually as we can't say why they think it's safe.

If we have codebases where unsafe is swept under the carpet then all of rust is built on shaky ground.

Drawbacks

False positives?

Example

pub fn byte_to_str_unchecked(bytes: &[u8]) -> &str {
	unsafe {
		#[allow(clippy::transmute_bytes_to_str)]
		mem::transmute(bytes)
	}
}

Could be written as:

pub unsafe fn byte_to_str_unchecked(bytes: &[u8]) -> &str {
	unsafe {
		#[allow(clippy::transmute_bytes_to_str)]
		mem::transmute(bytes)
	}
}

[gilescope]

I'm totally expecting some strong views on this one way or another.

[kraktus]

If a function breaks the business logic but does not joepardize the memory safety, should it still be marked as unsafe? Many _unchecked functions are in that case.

[llogiq]

This could still be a pedantic lint, I feel it fits perfectly in that category.

[blyxyas]

@rustbot claim

[blyxyas]

What would be the "Why is this bad" field?

[kraktus]

_unchecked functions do not check some invariants which could result in business logic, memory safety or undefined behaviour if the user does not guarantee them

Something like that

[blyxyas]

Trying this again 196 later (I already have the lint made) @rustbot claim

[robertbastian]

I feel like unsafe should be reserved for memory unsafety. Using it as a general might-violate-some–invariant marker will lead to more code being marked unsafe than necessary. Crates sometimes for example avoid unsafe code (and use the no-unsafe clippy lint), so if a dependency followed this lint's suggestion, that wouldn't work, even though the code might be completely safe.

[llogiq]

Fully agreed. unsafe is for memory unsafety only. _unchecked on the other hand does not carry the same meaning. Just that some invariant is not checked and there likely is another version of the code that does the check.

And this also is the reason why this lint should be allow-by-default. We do not and cannot detect intent, so if someone writes their next Checkbox::is_unchecked method, we don't want to bother them with a spurious warning.

[blyxyas]

If someone is having this issue and needs a solution, you can use Marker, a linting interface for Rust developed by our team member @xFrednet so anyone can implement a lint without it being having to be implemented on Clippy.