Clearer error message when line-length goes beyond threshold

[ShaharNaveh]

Fixes #20823

Summary

Implemented code from original issue

Test Plan

Added test

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[chirizxc]

My implementation example doesn't seem quite right.

I don't think we should prohibit people from setting line-length to more than 320, maybe we should just give a warning? (but it will change the current behavior)

[ShaharNaveh]

My implementation example doesn't seem quite right.

I don't think we should prohibit people from setting line-length to more than 320, maybe we should just give a warning? (but it will change the current behavior)

IMO, there's no need for having artificial constraints about the max size of the line-length, or a need to set a warning, the way that I see it, if a user explicitly set line-length to 10000 he knows what he's doing (I hope so). I am not saying that there's a need for increasing the possible length (by utilizing NonZerou32).

---

One possible solution would be to remove

ruff/crates/ruff_linter/src/line_width.rs

Lines 24 to 25 in 64ab79e

	/// Maximum allowed value for a valid [`LineLength`]
	pub const MAX: u16 = 320;

And relying on the constrains of NonZerou16 at:

ruff/crates/ruff_linter/src/line_width.rs

Lines 94 to 103 in 64ab79e

	impl TryFrom<u16> for LineLength {
	type Error = LineLengthFromIntError;
	fn try_from(value: u16) -> Result<Self, Self::Error> {
	match NonZeroU16::try_from(value) {
	Ok(value) if value.get() <= Self::MAX => Ok(LineLength(value)),
	Ok(_) | Err(_) => Err(LineLengthFromIntError(value)),
	}
	}
	}

[chirizxc]

It seems that the root of the problem lies in crate toml.

[chirizxc]

although it seems that the TOML specification mentions this

Non-negative integer values may also be expressed in hexadecimal, octal, or binary. In these formats, leading + is not allowed and leading zeros are allowed (after the prefix). Hex values are case-insensitive. Underscores are allowed between digits (but not between the prefix and the value).

# hexadecimal with prefix `0x`
hex1 = 0xDEADBEEF
hex2 = 0xdeadbeef
hex3 = 0xdead_beef

# octal with prefix `0o`
oct1 = 0o01234567
oct2 = 0o755 # useful for Unix file permissions

# binary with prefix `0b`
bin1 = 0b11010110

Arbitrary 64-bit signed integers (from −2^63 to 2^63−1) should be accepted and handled losslessly. If an integer cannot be represented losslessly, an error must be thrown.

[ShaharNaveh]

If an integer cannot be represented losslessly, an error must be thrown.

So if I understand you correctly it will be something like:

let raw = match i64::deserialize(deserializer) {
  Ok(v) => v,
  Err(_) => {
    let s: &str = Deserializer::deserialize(deserializer)?;
    i64::from_str(s).map_err(ParseLineWidthError::ParseError)?
  }
}

let value = u16::try_from(raw).map_err(???)

The main issue with this approach is that

ruff/crates/ruff_linter/src/line_width.rs

Lines 90 to 93 in 64ab79e

	/// Error type returned when converting a u16 to a [`LineLength`] fails
	#[derive(Clone, Copy, Debug)]
	pub struct LineLengthFromIntError(pub u16);

can only store u16 and in the best case with this approach we'll have i64. It doesn't make much sense to change the type to i64. what does deserlizing to i64 first gives us? why can't we just do what you suggested initially?

[MichaReiser]

Nit: Maybe something like this:

Suggested change

	let value = NonZeroU16::try_from(value).map_err(|_| {
	serde::de::Error::custom(format!(
	"line-length must be between 1 and {} (got {value})",
	Self::MAX,
	))
	})?;
	Self::try_from(value.get()).map_err(serde::de::Error::custom)
	let value = Self::try_from(value).map_err(|_| {
	serde::de::Error::custom(format!(
	"line-length must be between 1 and {} (got {value})",
	Self::MAX,
	))
	})

[MichaReiser]

Thank you