Type checking focused around element.py and shell testing

[nathanwilliams-ct]

Note: This adds a number of assertions where nullable values were assumed to be populated.

There should not be any functional changes.

[richardmaw-codethink]

I was curious and had some comments while reading through, hope you don't mind.

[richardmaw-codethink]

https://docs.python.org/3/library/__future__.html#future__.annotations is the typical solution recommended for circular definitions

but honestly it's due to become the default in the next release and the import deprecated to eventually become a syntax error so switching to it is going to cause more work down the line

[nathanwilliams-ct]

I went with the Manual String annotations as that is what is used elsewhere in the codebase at the moment.

Rabbit hole of documentation:

• https://docs.python.org/3/library/__future__.html#future__.annotations
• https://docs.python.org/3/reference/compound_stmts.html#annotations
• https://peps.python.org/pep-0749/
• https://docs.python.org/3/library/annotationlib.html#module-annotationlib

Which summarises to:

• For 3.14 annotations will be lazily evaluated.
• from __future__ import annotations sticks around until 3.13 reaches end of life.
• from __future__ import annotations has been around since 3.7

Buildstream currently supports the following python versions: 310,311,312,313,314

Using from __future__ import annotations would mean a simple find and replace could remove it in future. Using manual quotes like this would be a lot of manual re-work..

I think I will switch to from __future__ import annotations for this PR to reduce future work...?

[richardmaw-codethink]

Is this deferred import intentional? It's not going to have the problem of deferring finding import problems since Element._new_from_load_element will have needed to import it, but it does seem out of place.

[nathanwilliams-ct]

It's a cyclic import as JunctionElement imports Element already.

I am not a big fan of the cast here, but I am not sure on better way to do it with python's type checking. The cast is mainly to keep the type checker happy, and I am unsure if it's 'free' at runtime and makes the import unnecessary. I did debate wrapping it behind a if TYPE_CHECKING gate.

An assert isinstance(element,JunctionElement) also didn't play nice, as Element is it's base class.

I was trying to work out how I might get _new_from_load_element to just return a JunctionElement.

[richardmaw-codethink]

See previous comment about _get_cache_key

Suggested change

	def _get_cache_key(self, strength=_KeyStrength.STRONG) -> str | None:
	@overload
	def _get_cache_key(self, strength: Literal[_KeyStrength.WEAK]) -> str:
	...
	@overload
	def _get_cache_key(self, strength: Literal[_KeyStrength.STRONG) -> Optional[str]:
	...
	def _get_cache_key(self, strength: _KeyStrength = _KeyStrength.STRONG) -> Optional[str]:
	if strength == _KeyStrength.STRONG:
	return self.__cache_key
	else:
	return self.__weak_cache_key

[richardmaw-codethink]

See previous comment about walrus

Suggested change

	if owner in self.__proxies:
	return self.__proxies[owner]
	if (cached_proxy := self.__proxies.get(owner)) is not None:
	return cached_proxy