Review `# type: ignore` comments across the code base.

[jenstroeger]

@behnazh-w I am a little confused about this comment. For one, it’s a blanket ignore and it doesn’t specify what error is being ignored here; this line should be

status_on_skipped,  # type: ignore[arg-type]

But then the question arises: why is this argument type being ignored? Looking at the signature for the initializer

macaron/src/macaron/slsa_analyzer/checks/base_check.py

Lines 24 to 34 in 109edd2

	class BaseCheck:
	"""This abstract class is used to implement Checks in Macaron."""
	def __init__(
	self,
	check_id: str = "",
	description: str = "",
	depends_on: list[tuple[str, CheckResultType]] | None = None,
	eval_reqs: list[ReqName] | None = None,
	result_on_skip: CheckResultType = CheckResultType.SKIPPED,
	) -> None:

the type for the status_on_skipped variable should be

macaron/src/macaron/slsa_analyzer/checks/check_result.py

Lines 15 to 26 in 109edd2

	class CheckResultType(str, Enum):
	"""This class contains the types of a check result."""
	PASSED = "PASSED"
	FAILED = "FAILED"
	# A check is skipped from another check's result.
	SKIPPED = "SKIPPED"
	# A check is disabled from the user configuration.
	DISABLED = "DISABLED"
	# The result of the check is unknown or Macaron cannot resolve the
	# implementation of this check.
	UNKNOWN = "UNKNOWN"

However, that status_on_skipped variable is the test function‘s argument

macaron/tests/slsa_analyzer/checks/test_registry.py

Lines 156 to 162 in 109edd2

	@given(
	lists(
	one_of(none(), text(), integers(), tuples(), binary(), booleans()),
	min_size=1,
	)
	)
	def test_exit_on_invalid_status_on_skipped(self, status_on_skipped: SearchStrategy) -> None:

declared as a Hypothesis SearchStrategy. But that’s not quite right because @given feeds values from strategies into the test function and not the strategy object. Thus, the function argument receives a list[None | str | int | tuple | bytes | bool] but that still doesn’t match the signature’s CheckResultType. So shouldn’t this test look something like

@given(st.sampled_from(list(CheckResultType)))
def test_exit_on_invalid_status_on_skipped(self, status_on_skipped: CheckResultType) -> None:
    ...

I’ve not tested this.

---

I then grepped through the code base and there are heaps of blanket type ignores. I think it would therefore make sense to open an issue to review all these blanket ignores, attempt to address them by fixing up the code (see above), and if that’s not possible constrain the ignore to only the error that’s being ignored. By doing so, other type errors will be surfaced if they occur instead of silently swallowing all errors.

Originally posted by @jenstroeger in #1419 (comment)