Maybe wrong return type of ParserMethod

[medihack]

I wonder if ParserMethod has the wrong return type and null should be added because when a parser failed (and even recovery failed if enabled) it will return null.

Currently it is
export type ParserMethod<ARGS extends unknown[], R> = (...args: ARGS) => R
but maybe it should be
export type ParserMethod<ARGS extends unknown[], R> = (...args: ARGS) => R | null

[bd82]

ParserMethod is used in both the top level rules signatures RULE and in the subrule signatures, so maybe
better types signatures would require more sub-types here. There also seems to be a distinction in the runtime behavior for the top level invocation versus nested invocations of rules. and in CST vs Embedded Actions.

Perhaps the code should first be refactored to distinguish better between the different scenarios (CST vs EmbeddedActions at least) as I started listing the flows and got confused myself. 😄

I think some of the complexity is due to the CST flow having been added at a later time on the same base parser class.
(more features --> more complexity).

Relevant code:

defineRule runtime behavior

chevrotain/packages/chevrotain/src/parse/parser/traits/recognizer_engine.ts

Lines 217 to 248 in 58b0a98

	if (this.outputCst === true) {
	invokeRuleWithTry = function invokeRuleWithTry(
	this: MixedInParser,
	...args: ARGS
	): R {
	try {
	this.ruleInvocationStateUpdate(shortName, ruleName, this.subruleIdx)
	impl.apply(this, args)
	const cst = this.CST_STACK[this.CST_STACK.length - 1]
	this.cstPostRule(cst)
	return cst as unknown as R
	} catch (e) {
	return this.invokeRuleCatch(e, resyncEnabled, recoveryValueFunc) as R
	} finally {
	this.ruleFinallyStateUpdate()
	}
	}
	} else {
	invokeRuleWithTry = function invokeRuleWithTryCst(
	this: MixedInParser,
	...args: ARGS
	): R {
	try {
	this.ruleInvocationStateUpdate(shortName, ruleName, this.subruleIdx)
	return impl.apply(this, args)
	} catch (e) {
	return this.invokeRuleCatch(e, resyncEnabled, recoveryValueFunc) as R
	} finally {
	this.ruleFinallyStateUpdate()
	}
	}
	}

invokeRuleCatch

chevrotain/packages/chevrotain/src/parse/parser/traits/recognizer_engine.ts

Lines 258 to 310 in 58b0a98

	invokeRuleCatch(
	this: MixedInParser,
	e: Error,
	resyncEnabledConfig: boolean,
	recoveryValueFunc: Function
	): unknown {
	const isFirstInvokedRule = this.RULE_STACK.length === 1
	// note the reSync is always enabled for the first rule invocation, because we must always be able to
	// reSync with EOF and just output some INVALID ParseTree
	// during backtracking reSync recovery is disabled, otherwise we can't be certain the backtracking
	// path is really the most valid one
	const reSyncEnabled =
	resyncEnabledConfig && !this.isBackTracking() && this.recoveryEnabled
	if (isRecognitionException(e)) {
	const recogError: any = e
	if (reSyncEnabled) {
	const reSyncTokType = this.findReSyncTokenType()
	if (this.isInCurrentRuleReSyncSet(reSyncTokType)) {
	recogError.resyncedTokens = this.reSyncTo(reSyncTokType)
	if (this.outputCst) {
	const partialCstResult: any =
	this.CST_STACK[this.CST_STACK.length - 1]
	partialCstResult.recoveredNode = true
	return partialCstResult
	} else {
	return recoveryValueFunc()
	}
	} else {
	if (this.outputCst) {
	const partialCstResult: any =
	this.CST_STACK[this.CST_STACK.length - 1]
	partialCstResult.recoveredNode = true
	recogError.partialCstResult = partialCstResult
	}
	// to be handled Further up the call stack
	throw recogError
	}
	} else if (isFirstInvokedRule) {
	// otherwise a Redundant input error will be created as well and we cannot guarantee that this is indeed the case
	this.moveToTerminatedState()
	// the parser should never throw one of its own errors outside its flow.
	// even if error recovery is disabled
	return recoveryValueFunc()
	} else {
	// to be recovered Further up the call stack
	throw recogError
	}
	} else {
	// some other Error type which we don't know how to handle (for example a built in JavaScript Error)
	throw e
	}
	}

Default RecoveryValueFunc (may only be relevant for CST flow)

chevrotain/packages/chevrotain/src/parse/parser/parser.ts

Line 73 in 58b0a98

recoveryValueFunc: () => undefined,

• By default this returns undefined but can be customized by the user