Skip to content

Commit

Permalink
refactor(chevrotain): fix errors from strictNullChecks (#1696)
Browse files Browse the repository at this point in the history
  • Loading branch information
NaridaL authored Nov 9, 2021
1 parent 8663cba commit 82f1a33
Show file tree
Hide file tree
Showing 35 changed files with 279 additions and 266 deletions.
6 changes: 3 additions & 3 deletions packages/chevrotain/src/parse/cst/cst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { CstNode, CstNodeLocation, IToken } from "@chevrotain/types"
*/
export function setNodeLocationOnlyOffset(
currNodeLocation: CstNodeLocation,
newLocationInfo: IToken
newLocationInfo: Required<Pick<IToken, "startOffset" | "endOffset">>
): void {
// First (valid) update for this cst node
if (isNaN(currNodeLocation.startOffset) === true) {
Expand All @@ -23,7 +23,7 @@ export function setNodeLocationOnlyOffset(
// any farther updates as the Token vector is sorted.
// We still have to check this this condition for every new possible location info
// because with error recovery enabled we may encounter invalid tokens (NaN location props)
else if (currNodeLocation.endOffset < newLocationInfo.endOffset === true) {
else if (currNodeLocation.endOffset! < newLocationInfo.endOffset === true) {
currNodeLocation.endOffset = newLocationInfo.endOffset
}
}
Expand Down Expand Up @@ -55,7 +55,7 @@ export function setNodeLocationFull(
// any farther updates as the Token vector is sorted.
// We still have to check this this condition for every new possible location info
// because with error recovery enabled we may encounter invalid tokens (NaN location props)
else if (currNodeLocation.endOffset < newLocationInfo.endOffset === true) {
else if (currNodeLocation.endOffset! < newLocationInfo.endOffset! === true) {
currNodeLocation.endOffset = newLocationInfo.endOffset
currNodeLocation.endColumn = newLocationInfo.endColumn
currNodeLocation.endLine = newLocationInfo.endLine
Expand Down
15 changes: 10 additions & 5 deletions packages/chevrotain/src/parse/cst/cst_visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ import compact from "lodash/compact"
import isArray from "lodash/isArray"
import map from "lodash/map"
import forEach from "lodash/forEach"
import filter from "lodash/filter"
import keys from "lodash/keys"
import isFunction from "lodash/isFunction"
import isUndefined from "lodash/isUndefined"
import includes from "lodash/includes"
import { defineNameProp } from "../../lang/lang_extensions"
import { CstNode, ICstVisitor } from "@chevrotain/types"

export function defaultVisit<IN, OUT>(ctx: any, param: IN): OUT {
export function defaultVisit<IN>(ctx: any, param: IN): void {
const childrenNames = keys(ctx)
const childrenNamesLength = childrenNames.length
for (let i = 0; i < childrenNamesLength; i++) {
Expand All @@ -26,7 +27,6 @@ export function defaultVisit<IN, OUT>(ctx: any, param: IN): OUT {
}
}
// defaultVisit does not support generic out param
return undefined
}

export function createBaseSemanticVisitorConstructor(
Expand Down Expand Up @@ -132,8 +132,13 @@ export function validateMissingCstMethods(
visitorInstance: ICstVisitor<unknown, unknown>,
ruleNames: string[]
): IVisitorDefinitionError[] {
const errors: IVisitorDefinitionError[] = map(ruleNames, (currRuleName) => {
if (!isFunction((visitorInstance as any)[currRuleName])) {
const missingRuleNames = filter(ruleNames, (currRuleName) => {
return isFunction((visitorInstance as any)[currRuleName]) === false
})

const errors: IVisitorDefinitionError[] = map(
missingRuleNames,
(currRuleName) => {
return {
msg: `Missing visitor method: <${currRuleName}> on ${<any>(
visitorInstance.constructor.name
Expand All @@ -142,7 +147,7 @@ export function validateMissingCstMethods(
methodName: currRuleName
}
}
})
)

return compact<IVisitorDefinitionError>(errors)
}
Expand Down
6 changes: 3 additions & 3 deletions packages/chevrotain/src/parse/errors_public.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const defaultParserErrorProvider: IParserErrorMessageProvider = {
}): string {
const errPrefix = "Expecting: "
// TODO: issue: No Viable Alternative Error may have incomplete details. #502
const actualText = first(actual).image
const actualText = first(actual)!.image
const errSuffix = "\nbut found: '" + actualText + "'"

if (customUserDescription) {
Expand Down Expand Up @@ -82,7 +82,7 @@ export const defaultParserErrorProvider: IParserErrorMessageProvider = {
}): string {
const errPrefix = "Expecting: "
// TODO: issue: No Viable Alternative Error may have incomplete details. #502
const actualText = first(actual).image
const actualText = first(actual)!.image
const errSuffix = "\nbut found: '" + actualText + "'"

if (customUserDescription) {
Expand Down Expand Up @@ -142,7 +142,7 @@ export const defaultGrammarValidatorErrorProvider: IGrammarValidatorErrorMessage
}

const topLevelName = topLevelRule.name
const duplicateProd = first(duplicateProds)
const duplicateProd = first(duplicateProds)!
const index = duplicateProd.idx
const dslName = getProductionDslName(duplicateProd)
const extraArgument = getExtraProductionArgument(duplicateProd)
Expand Down
111 changes: 52 additions & 59 deletions packages/chevrotain/src/parse/grammar/checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
} from "./types"
import dropRight from "lodash/dropRight"
import compact from "lodash/compact"
import { tokenStructuredMatcher } from "../../scan/tokens"

export function validateGrammar(
topLevels: Rule[],
Expand Down Expand Up @@ -381,21 +382,20 @@ export function validateEmptyOrAlternative(
topLevelRule.accept(orCollector)
const ors = orCollector.alternations

const errors = reduce(
const errors = flatMap<Alternation, IParserEmptyAlternativeDefinitionError>(
ors,
(errors, currOr) => {
(currOr) => {
const exceptLast = dropRight(currOr.definition)
const currErrors = map(
exceptLast,
(currAlternative: IProduction, currAltIdx) => {
const possibleFirstInAlt = nextPossibleTokensAfter(
[currAlternative],
[],
null,
1
)
if (isEmpty(possibleFirstInAlt)) {
return {
return flatMap(exceptLast, (currAlternative, currAltIdx) => {
const possibleFirstInAlt = nextPossibleTokensAfter(
[currAlternative],
[],
tokenStructuredMatcher,
1
)
if (isEmpty(possibleFirstInAlt)) {
return [
{
message: errMsgProvider.buildEmptyAlternationError({
topLevelRule: topLevelRule,
alternation: currOr,
Expand All @@ -406,14 +406,12 @@ export function validateEmptyOrAlternative(
occurrence: currOr.idx,
alternative: currAltIdx + 1
}
} else {
return null
}
]
} else {
return []
}
)
return errors.concat(compact(currErrors))
},
[]
})
}
)

return errors
Expand All @@ -432,34 +430,30 @@ export function validateAmbiguousAlternationAlternatives(
// - https://github.com/chevrotain/chevrotain/issues/869
ors = reject(ors, (currOr) => currOr.ignoreAmbiguities === true)

const errors = reduce(
ors,
(result, currOr: Alternation) => {
const currOccurrence = currOr.idx
const actualMaxLookahead = currOr.maxLookahead || globalMaxLookahead
const alternatives = getLookaheadPathsForOr(
currOccurrence,
topLevelRule,
actualMaxLookahead,
currOr
)
const altsAmbiguityErrors = checkAlternativesAmbiguities(
alternatives,
currOr,
topLevelRule,
errMsgProvider
)
const altsPrefixAmbiguityErrors = checkPrefixAlternativesAmbiguities(
alternatives,
currOr,
topLevelRule,
errMsgProvider
)
const errors = flatMap(ors, (currOr: Alternation) => {
const currOccurrence = currOr.idx
const actualMaxLookahead = currOr.maxLookahead || globalMaxLookahead
const alternatives = getLookaheadPathsForOr(
currOccurrence,
topLevelRule,
actualMaxLookahead,
currOr
)
const altsAmbiguityErrors = checkAlternativesAmbiguities(
alternatives,
currOr,
topLevelRule,
errMsgProvider
)
const altsPrefixAmbiguityErrors = checkPrefixAlternativesAmbiguities(
alternatives,
currOr,
topLevelRule,
errMsgProvider
)

return result.concat(altsAmbiguityErrors, altsPrefixAmbiguityErrors)
},
[]
)
return altsAmbiguityErrors.concat(altsPrefixAmbiguityErrors)
})

return errors
}
Expand Down Expand Up @@ -496,24 +490,23 @@ export function validateTooManyAlts(
topLevelRule.accept(orCollector)
const ors = orCollector.alternations

const errors = reduce(
ors,
(errors, currOr) => {
if (currOr.definition.length > 255) {
errors.push({
const errors = flatMap(ors, (currOr) => {
if (currOr.definition.length > 255) {
return [
{
message: errMsgProvider.buildTooManyAlternativesError({
topLevelRule: topLevelRule,
alternation: currOr
}),
type: ParserDefinitionErrorType.TOO_MANY_ALTS,
ruleName: topLevelRule.name,
occurrence: currOr.idx
})
}
return errors
},
[]
)
}
]
} else {
return []
}
})

return errors
}
Expand Down Expand Up @@ -653,7 +646,7 @@ export function checkPrefixAlternativesAmbiguities(
const alternativeGast = alternation.definition[currPathAndIdx.idx]
// ignore (skip) ambiguities with this alternative
if (alternativeGast.ignoreAmbiguities === true) {
return
return []
}
const targetIdx = currPathAndIdx.idx
const targetPath = currPathAndIdx.path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,22 @@ import {
IParserDefinitionError
} from "../types"

export function resolveGrammar(options: {
type ResolveGrammarOpts = {
rules: Rule[]
errMsgProvider?: IGrammarResolverErrorMessageProvider
}): IParserDefinitionError[] {
options = defaults(options, {
}
export function resolveGrammar(
options: ResolveGrammarOpts
): IParserDefinitionError[] {
const actualOptions: Required<ResolveGrammarOpts> = defaults(options, {
errMsgProvider: defaultGrammarResolverErrorProvider
})

const topRulesTable: { [ruleName: string]: Rule } = {}
forEach(options.rules, (rule) => {
topRulesTable[rule.name] = rule
})
return orgResolveGrammar(topRulesTable, options.errMsgProvider)
return orgResolveGrammar(topRulesTable, actualOptions.errMsgProvider)
}

export function validateGrammar(options: {
Expand Down
22 changes: 9 additions & 13 deletions packages/chevrotain/src/parse/grammar/interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ export abstract class AbstractNextPossibleTokensWalker extends RestWalker {
this.nextProductionOccurrence = 0
this.isAtEndOfPath = true
} else {
this.nextProductionName = this.ruleStack.pop()
this.nextProductionOccurrence = this.occurrenceStack.pop()
this.nextProductionName = this.ruleStack.pop()!
this.nextProductionOccurrence = this.occurrenceStack.pop()!
}
}
}
Expand Down Expand Up @@ -135,21 +135,17 @@ export class NextAfterTokenWalker extends AbstractNextPossibleTokensWalker {
export type AlternativesFirstTokens = TokenType[][]

export interface IFirstAfterRepetition {
token: TokenType
occurrence: number
isEndOfRule: boolean
token: TokenType | undefined
occurrence: number | undefined
isEndOfRule: boolean | undefined
}

/**
* This walker only "walks" a single "TOP" level in the Grammar Ast, this means
* it never "follows" production refs
*/
export class AbstractNextTerminalAfterProductionWalker extends RestWalker {
protected result: {
token: TokenType | undefined
occurrence: number | undefined
isEndOfRule: boolean | undefined
} = {
protected result: IFirstAfterRepetition = {
token: undefined,
occurrence: undefined,
isEndOfRule: undefined
Expand Down Expand Up @@ -383,13 +379,13 @@ export function nextPossibleTokensAfter(
})

while (!isEmpty(possiblePaths)) {
const currPath = possiblePaths.pop()
const currPath = possiblePaths.pop()!

// skip alternatives if no more results can be found (assuming deterministic grammar with fixed lookahead)
if (currPath === EXIT_ALTERNATIVE) {
if (
foundCompletePath &&
last(possiblePaths).idx <= minimalAlternativesIndex
last(possiblePaths)!.idx <= minimalAlternativesIndex
) {
// remove irrelevant alternative
possiblePaths.pop()
Expand Down Expand Up @@ -422,7 +418,7 @@ export function nextPossibleTokensAfter(
if (currIdx < tokenVectorLength - 1) {
const nextIdx = currIdx + 1
const actualToken = tokenVector[nextIdx]
if (tokMatcher(actualToken, prod.terminalType)) {
if (tokMatcher!(actualToken, prod.terminalType)) {
const nextPath = {
idx: nextIdx,
def: drop(currDef),
Expand Down
Loading

0 comments on commit 82f1a33

Please sign in to comment.