Display "Go to latest stable" for pre-release

[robinhundt]

closes #1044

[jyn514]

I don't think this is true, we allow exact matches before checking semver:

docs.rs/src/web/mod.rs

Line 333 in fb06f95

// first check for exact match, we can't expect users to use semver in query

Suggested change

	let is_prerelease = semver::Version::parse(&version)
	// should be impossible unless there is a bug in match_version
	.map_err(|_| Nope::InternalServerError)?
	.is_prerelease();
	// If this is an exact match (not a valid semver), assume it's not a stable version.
	let is_prerelease = semver::Version::parse(&version).unwrap_or(true);

[robinhundt]

Oh, you're right. But the suggested change won't work, because then GET /sqlx/0.3 would be treated as a pre-release although the semver compatible version 0.3.5 would be returned.
I thought just parsing the Version here would be the least invasive change, but it seems like I'll have to get the actual version returned from the DB for the release serverd.

[robinhundt]

Mhh, i looked at it again. The version is actually guaranteed to be semver compatible, because in the case of an exact match the version is semver compatible (if not, the db contains an invalid semver version) and in the case of an inexact match rustdoc_html_server_handler will redirect to the exact version.

docs.rs/src/web/rustdoc.rs

Lines 303 to 310 in c444b63

	// Redirect when the requested version isn't correct
	MatchSemver::Semver((v, _)) => {
	// to prevent cloudfront caching the wrong artifacts on URLs with loose semver
	// versions, redirect the browser to the returned version instead of loading it
	// immediately
	return redirect(&name, &v, &req_path);
	}
	};

[jyn514]

I don't follow, "0.3" is a valid semver and would be parsed successfully. response to your previous comment

[jyn514]

if not, the db contains an invalid semver version

Hmm ... I don't think we enforce semantic versioning in the database, we depend on cargo to do it for us. In principle a 500 is probably the right thing to do, but it seems a shame to return that when it's so easy to return the page with a different text instead.

[robinhundt]

Would you rather add a new Nope variant for this case? I feel like in this case it should be fine to just return a 500. We do the same in the match_version func

docs.rs/src/web/mod.rs

Lines 348 to 360 in c444b63

	for version in versions.iter().filter(|(_, _, yanked)| !yanked) {
	// in theory a crate must always have a semver compatible version,
	// but check result just in case
	let version_sem = Version::parse(&version.0).map_err(|_| {
	log::error!(
	"invalid semver in database for crate {}: {}",
	name,
	version.0
	);
	Nope::InternalServerError
	})?;
	versions_sem.push((version_sem, version.1));
	}

[jyn514]

Hmm, the Nope error would just turn into 'InternalServerError', right? I think that would be useful after #1046 to print the log message automatically (in the from impl) but other than that I don't think it buys us much.

[jyn514]

LGTM with nits fixed

[jyn514]

nit: grammar

Suggested change

	{%- set tooltip = "You are seeing a yanked version of the " ~ krate.name ~ "crate. Click here to go to latest version." -%}
	{%- set title = "This release has been yanked, go to latest version" -%}
	{%- elif is_prerelease -%}
	{%- set tooltip = "You are seeing a pre-release version of the " ~ krate.name ~ "crate. Click here to go to latest stable version." -%}
	{%- set title = "Go to latest stable release" -%}
	{%- else -%}
	{%- set tooltip = "You are seeing an outdated version of the " ~ krate.name ~ "crate. Click here to go to latest version." -%}
	{%- set tooltip = "You are seeing a yanked version of the " ~ krate.name ~ "crate. Click here to go to the latest version." -%}
	{%- set title = "This release has been yanked, go to latest version" -%}
	{%- elif is_prerelease -%}
	{%- set tooltip = "You are seeing a pre-release version of the " ~ krate.name ~ "crate. Click here to go to the latest stable version." -%}
	{%- set title = "Go to latest stable release" -%}
	{%- else -%}
	{%- set tooltip = "You are seeing an outdated version of the " ~ krate.name ~ "crate. Click here to go to the latest version." -%}

[jyn514]

Thanks for working on this (and putting up with all my nits)!

[robinhundt]

No problem :) The code is better that way and you're reviewing really fast 😄 👍