False positive of typescript:S3801 with function that never returns

[julrich_bee360 (Jochen Ulrich)]

I think I found a false positive of typescript:S3801.
The code looks like this:

function prepareDateTimeValue(value: ValueType | number): string {
	if (value instanceof Date) {
		return value.toISOString();
	}

	if (typeof value === "string") {
		const date = new Date(value);
		if (isNaN(date.valueOf())) {
			throw new Error("Invalid date time string: " + JSON.stringify(value));
		}
		return date.toISOString();
	}

	throwUnexpectedValueTypeError(value, "a Date object or a date time string");
}

function throwUnexpectedValueTypeError(
	value: ValueType | number,
	expectedTypeDescription?: string
): never {
	throw new Error(
		`Unexpected value type. Value has type '${getHumanReadableObjectTypeName(
			value
		)}'.` + (expectedTypeDescription ? ` Expected ${expectedTypeDescription}.` : "")
	);
}

SonarQube reports typescript:S3801 (Refactor this function to use “return” consistently) on prepareDateTimeValue() because the last branch does not return. So it seems it doesn’t take into account that throwUnexpectedValueTypeError() never returns.

SonarQube Version: 9.7

[gab (Gabriel Vivas)]

Hi Jochen,

Thanks for bringing this up.

It seems that the return type of prepareDateTimeValue function is inaccurate, which is throwing us off, pun intended

I think it should be a union of string | never.

This should fix the issue you are seeing.

Nevertheless, I have created a ticket to improve the rule description so that it mentions the importance of the return type:

FP in S3801 when `throw` is in child functions

I also noticed that this is in fact a false positive in JavaScript, so I included that in the ticket too. Thank you again.

Cheers,
Gabriel