Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: remove constraints on rules and token names (#1291) #1293

Merged
merged 2 commits into from
Dec 21, 2020
Merged

feat: remove constraints on rules and token names (#1291) #1293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a24068
feat: allow dash (-) in token name (#1291)
jlguenego Dec 16, 2020
0912562
fix: remove validate rule and token name. (#1291)
jlguenego Dec 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions packages/chevrotain/src/parse/cst/cst_visitor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
map
} from "../../utils/utils"
import { defineNameProp, functionName } from "../../lang/lang_extensions"
import { validTermsPattern } from "../grammar/checks"
import { ICstVisitor } from "../../../api"

export function defaultVisit<IN, OUT>(ctx: any, param: IN): OUT {
Expand Down Expand Up @@ -160,7 +159,6 @@ export function validateRedundantMethods(

for (let prop in visitorInstance) {
if (
validTermsPattern.test(prop) &&
isFunction(visitorInstance[prop]) &&
!contains(VALID_PROP_NAMES, prop) &&
!contains(ruleNames, prop)
Expand All @@ -170,8 +168,7 @@ export function validateRedundantMethods(
`Redundant visitor method: <${prop}> on ${functionName(
<any>visitorInstance.constructor
)} CST Visitor\n` +
`There is no Grammar Rule corresponding to this method's name.\n` +
`For utility methods on visitor classes use methods names that do not match /${validTermsPattern.source}/.`,
`There is no Grammar Rule corresponding to this method's name.\n`,
type: CstVisitorDefinitionError.REDUNDANT_METHOD,
methodName: prop
})
Expand Down
54 changes: 0 additions & 54 deletions packages/chevrotain/src/parse/grammar/checks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,10 @@ export function validateGrammar(
errMsgProvider
)

let tokenNameErrors: any = utils.map(tokenTypes, (currTokType) =>
validateTokenName(currTokType, errMsgProvider)
)

const tooManyAltsErrors = map(topLevels, (curRule) =>
validateTooManyAlts(curRule, errMsgProvider)
)

const ruleNameErrors = map(topLevels, (curRule) =>
validateRuleName(curRule, errMsgProvider)
)

const duplicateRulesError = map(topLevels, (curRule) =>
validateRuleDoesNotAlreadyExist(
curRule,
Expand All @@ -116,14 +108,12 @@ export function validateGrammar(
return <any>(
utils.flatten(
duplicateErrors.concat(
tokenNameErrors,
emptyRepetitionErrors,
leftRecursionErrors,
emptyAltErrors,
ambiguousAltsErrors,
termsNamespaceConflictErrors,
tooManyAltsErrors,
ruleNameErrors,
duplicateRulesError
)
)
Expand Down Expand Up @@ -228,50 +218,6 @@ export class OccurrenceValidationCollector extends GAstVisitor {
}
}

export const validTermsPattern = /^[a-zA-Z_]\w*$/

// TODO: remove this limitation now that we use recorders
export function validateRuleName(
rule: Rule,
errMsgProvider: IGrammarValidatorErrorMessageProvider
): IParserDefinitionError[] {
const errors = []
const ruleName = rule.name

if (!ruleName.match(validTermsPattern)) {
errors.push({
message: errMsgProvider.buildInvalidRuleNameError({
topLevelRule: rule,
expectedPattern: validTermsPattern
}),
type: ParserDefinitionErrorType.INVALID_RULE_NAME,
ruleName: ruleName
})
}
return errors
}

// TODO: remove this limitation now that we use recorders
export function validateTokenName(
tokenType: TokenType,
errMsgProvider: IGrammarValidatorErrorMessageProvider
): IParserDefinitionError[] {
const errors = []
const tokTypeName = tokenType.name

if (!tokTypeName.match(validTermsPattern)) {
errors.push({
message: errMsgProvider.buildTokenNameError({
tokenType: tokenType,
expectedPattern: validTermsPattern
}),
type: ParserDefinitionErrorType.INVALID_TOKEN_NAME
})
}

return errors
}

export function validateRuleDoesNotAlreadyExist(
rule: Rule,
allRules: Rule[],
Expand Down
75 changes: 0 additions & 75 deletions packages/chevrotain/test/parse/grammar/checks_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
validateGrammar,
validateRuleDoesNotAlreadyExist,
validateRuleIsOverridden,
validateRuleName,
validateTooManyAlts
} from "../../../src/parse/grammar/checks"
import { createToken } from "../../../src/scan/tokens_public"
Expand Down Expand Up @@ -153,44 +152,6 @@ describe("the grammar validations", () => {
expect(duplicateErr[0]).to.have.property("ruleName", "A")
})

it("only allows a subset of ECMAScript identifiers as rule names", () => {
let res1 = validateRuleName(
new Rule({ name: "1baa", definition: [] }),
defaultGrammarValidatorErrorProvider
)
expect(res1).to.have.lengthOf(1)
expect(res1[0]).to.have.property("message")
expect(res1[0]).to.have.property(
"type",
ParserDefinitionErrorType.INVALID_RULE_NAME
)
expect(res1[0]).to.have.property("ruleName", "1baa")

let res2 = validateRuleName(
new Rule({ name: "שלום", definition: [] }),
defaultGrammarValidatorErrorProvider
)
expect(res2).to.have.lengthOf(1)
expect(res2[0]).to.have.property("message")
expect(res2[0]).to.have.property(
"type",
ParserDefinitionErrorType.INVALID_RULE_NAME
)
expect(res2[0]).to.have.property("ruleName", "שלום")

let res3 = validateRuleName(
new Rule({ name: "$bamba", definition: [] }),
defaultGrammarValidatorErrorProvider
)
expect(res3).to.have.lengthOf(1)
expect(res3[0]).to.have.property("message")
expect(res3[0]).to.have.property(
"type",
ParserDefinitionErrorType.INVALID_RULE_NAME
)
expect(res3[0]).to.have.property("ruleName", "$bamba")
})

it("does not allow overriding a rule which does not already exist", () => {
let positive = validateRuleIsOverridden("AAA", ["BBB", "CCC"], "className")
expect(positive).to.have.lengthOf(1)
Expand Down Expand Up @@ -916,16 +877,6 @@ describe("The rule names validation full flow", () => {
expect(() => new DuplicateRulesParser()).to.throw("oops_duplicate")
})

it("will throw an error when trying to init a parser with an invalid rule names", () => {
expect(() => new InvalidRuleNameParser()).to.throw(
"it must match the pattern"
)
expect(() => new InvalidRuleNameParser()).to.throw(
"Invalid grammar rule name"
)
expect(() => new InvalidRuleNameParser()).to.throw("שלום")
})

it(
"won't throw an errors when trying to init a parser with definition errors but with a flag active to defer handling" +
"of definition errors (ruleName validation",
Expand Down Expand Up @@ -1483,32 +1434,6 @@ describe("The namespace conflict detection full flow", () => {
})
})

describe("The invalid token name validation", () => {
it("will throw an error when a Token is using an invalid name", () => {
class במבה {
static PATTERN = /NA/
}
class A {
static PATTERN = /NA/
}

class InvalidTokenName extends EmbeddedActionsParser {
constructor(input: IToken[] = []) {
super([במבה, A])
this.performSelfAnalysis()
this.input = input
}

public someRule = this.RULE("someRule", () => {
this.CONSUME(A)
})
}
expect(() => new InvalidTokenName([])).to.throw(
"Invalid Grammar Token name: ->במבה<- it must match the pattern: ->/^[a-zA-Z_]\\w*$/<-"
)
})
})

describe("The no non-empty lookahead validation", () => {
it("will throw an error when there are no non-empty lookaheads for AT_LEAST_ONE", () => {
class EmptyLookaheadParserAtLeastOne extends EmbeddedActionsParser {
Expand Down