[red-knot] Add a test to ensure that KnownClass::try_from_file_and_name() is kept up to date

[AlexWaygood]

Summary

We keep on adding new variants to the KnownClass enum but forgetting to update this method, because the Rust compiler only enforces exhaustiveness over enums when the enum variants appear on the left-hand side of the match statement:

ruff/crates/red_knot_python_semantic/src/types.rs

Lines 3183 to 3231 in b312b53

	pub fn try_from_file_and_name(db: &dyn Db, file: File, class_name: &str) -> Option<Self> {
	// Note: if this becomes hard to maintain (as rust can't ensure at compile time that all
	// variants of `Self` are covered), we might use a macro (in-house or dependency)
	// See: https://stackoverflow.com/q/39070244
	let candidate = match class_name {
	"bool" => Self::Bool,
	"object" => Self::Object,
	"bytes" => Self::Bytes,
	"tuple" => Self::Tuple,
	"type" => Self::Type,
	"int" => Self::Int,
	"float" => Self::Float,
	"complex" => Self::Complex,
	"str" => Self::Str,
	"set" => Self::Set,
	"frozenset" => Self::FrozenSet,
	"dict" => Self::Dict,
	"list" => Self::List,
	"slice" => Self::Slice,
	"range" => Self::Range,
	"BaseException" => Self::BaseException,
	"BaseExceptionGroup" => Self::BaseExceptionGroup,
	"GenericAlias" => Self::GenericAlias,
	"NoneType" => Self::NoneType,
	"ModuleType" => Self::ModuleType,
	"FunctionType" => Self::FunctionType,
	"MethodType" => Self::MethodType,
	"MethodWrapperType" => Self::MethodWrapperType,
	"WrapperDescriptorType" => Self::WrapperDescriptorType,
	"TypeAliasType" => Self::TypeAliasType,
	"TypeVar" => Self::TypeVar,
	"ChainMap" => Self::ChainMap,
	"Counter" => Self::Counter,
	"defaultdict" => Self::DefaultDict,
	"deque" => Self::Deque,
	"OrderedDict" => Self::OrderedDict,
	"_Alias" => Self::StdlibAlias,
	"_SpecialForm" => Self::SpecialForm,
	"_NoDefaultType" => Self::NoDefaultType,
	"SupportsIndex" => Self::SupportsIndex,
	"_version_info" => Self::VersionInfo,
	"ellipsis" if Program::get(db).python_version(db) <= PythonVersion::PY39 => {
	Self::EllipsisType
	}
	"EllipsisType" if Program::get(db).python_version(db) >= PythonVersion::PY310 => {
	Self::EllipsisType
	}
	_ => return None,
	};

That leads to subtle and confusing bugs that can be hard to debug, and that sometimes go unnoticed for several weeks.

This PR adds a test that will fail if we add a variant to the KnownClass enum and forget to add a case to KnownClass::try_from_file_and_name(). It also adds tests for the similar methods on the KnownModule and KnownFunction enums.

How it works

strum and strum_macros are added as test-only dependencies for red-knot (they are added to the dev-dependencies table in the red_knot_python_semantic crate's Cargo.toml). If the test feature is enabled, we derive the EnumIter trait for the KnownClass enum. In the tests for KnownClass, we then use the generated iter() method to iterate over all KnownClass variants and verify that each variant has its own branch in the KnownClass::try_from_file_and_name() function.

Following review, this has been changed:

• strum and strum_macros are now just regular dependencies for red-knot, in production as well as if the test feature is enabled.
• For KnownFunction and KnownModule, we use the EnumString macro from strum_macros to simplify the deserialization constructors and ensure that they are exhaustive over the right-hand side. (This unfortunately doesn't work for KnownClass because of the fact that the EllipsisType branch deserializes dynamically depending on the target Python version.)
• For all three enums, we add tests to ensure that the deserialization functions are both exhaustive and correct. This is done using strum's EnumIter macro.

Limitations

Unfortunately, this approach doesn't work for the KnownInstanceType enum. KnownInstanceType is generic over a lifetime, and the EnumIter trait cannot be derived for enums generic over a lifetime. I therefore haven't touched that enum in this PR.

This approach also necessitated some small refactoring of the KnownFunction enum. EnumIter and EnumString couldn't be derived for the enum as-is because of the fact that the ConstraintFunction variant wrapped inner data. I solved this by replacing the ConstraintFunction variant (which wrapped an inner enum) with two variants (KnownFunction::IsSubclass and KnownFunction::IsInstance), and slightly reworking the KnownFunction::constraint_function() method (which is renamed to KnownFunction::into_constraint_function())

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

[MichaReiser]

Could we use strum::EnumString https://github.com/Peternator7/strum/wiki/Derive-EnumString if we're adding a strum dependency anyway?

[AlexWaygood]

Could we use strum::EnumString https://github.com/Peternator7/strum/wiki/Derive-EnumString if we're adding a strum dependency anyway?

This PR currently only adds strum as a dev-dependency. But I can make that change if we're happy to have the extra dependency in production as well.

[sharkdp]

Could we use strum::EnumString https://github.com/Peternator7/strum/wiki/Derive-EnumString if we're adding a strum dependency anyway?

I also played around a bit with a solution to this TODO on the weekend. One thing that makes this more complicated are branches like this, where we have additional logic in the enum->string function:

            Self::EllipsisType => {
                // Exposed as `types.EllipsisType` on Python >=3.10;
                // backported as `builtins.ellipsis` by typeshed on Python <=3.9
                if Program::get(db).python_version(db) >= PythonVersion::PY310 {
                    "EllipsisType"
                } else {
                    "ellipsis"
                }
            }

[MichaReiser]

This PR currently only adds strum as a dev-dependency. But I can make that change if we're happy to have the extra dependency in production as well.

There shouldn't really be any difference. strum is 99% macros and traits only

[MichaReiser]

This is great. Thank you (and sorry for the merge conflict :()