Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

Commit 33c7b6e

Browse files
feich-msfeich-ms
authored
Fix lu converter to handle required feature at top level and whitespaces at child entity name correctly (#1139)
* fix lu converter * add validation for invalid intent and phraselist name and move them out
1 parent 7004e0b commit 33c7b6e

File tree

15 files changed

+305
-37
lines changed

15 files changed

+305
-37
lines changed

‎packages/lu/src/parser/lufile/entitySection.js‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ const EntitySectionContext = require('./generated/LUFileParser').LUFileParser.En
22
const DiagnosticSeverity = require('./diagnostic').DiagnosticSeverity;
33
const BuildDiagnostic = require('./diagnostic').BuildDiagnostic;
44
const LUSectionTypes = require('./../utils/enums/lusectiontypes');
5-
const InvalidCharsInIntentOrEntityName = require('./../utils/enums/invalidchars').InvalidCharsInIntentOrEntityName;
65
const BaseSection = require('./baseSection');
76
const Range = require('./diagnostic').Range;
87
const Position = require('./diagnostic').Position;
@@ -35,14 +34,7 @@ class EntitySection extends BaseSection {
3534
}));
3635
}
3736

38-
if (entityName && InvalidCharsInIntentOrEntityName.some(x => entityName.includes(x))) {
39-
this.Errors.push(BuildDiagnostic({
40-
message: `Invalid entity line, entity name ${entityName} cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]`,
41-
context: parseTree.newEntityDefinition().newEntityLine()
42-
}));
43-
} else {
44-
return entityName;
45-
}
37+
return entityName;
4638
}
4739

4840
ExtractType(parseTree) {

‎packages/lu/src/parser/lufile/newEntitySection.js‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ const NewEntitySectionContext = require('./generated/LUFileParser').LUFileParser
22
const DiagnosticSeverity = require('./diagnostic').DiagnosticSeverity;
33
const BuildDiagnostic = require('./diagnostic').BuildDiagnostic;
44
const LUSectionTypes = require('./../utils/enums/lusectiontypes');
5-
const InvalidCharsInIntentOrEntityName = require('./../utils/enums/invalidchars').InvalidCharsInIntentOrEntityName;
65
const BaseSection = require('./baseSection');
76
const Range = require('./diagnostic').Range;
87
const Position = require('./diagnostic').Position;
@@ -42,14 +41,7 @@ class NewEntitySection extends BaseSection {
4241
}))
4342
}
4443

45-
if (entityName && InvalidCharsInIntentOrEntityName.some(x => entityName.includes(x))) {
46-
this.Errors.push(BuildDiagnostic({
47-
message: `Invalid entity line, entity name ${entityName} cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]`,
48-
context: parseTree.newEntityDefinition().newEntityLine()
49-
}));
50-
} else {
51-
return entityName;
52-
}
44+
return entityName;
5345
}
5446

5547
ExtractType(parseTree) {

‎packages/lu/src/parser/lufile/parseFileContents.js‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const luisEntityTypeMap = require('./../utils/enums/luisEntityTypeNameMap');
2727
const qnaContext = require('../qna/qnamaker/qnaContext');
2828
const qnaPrompt = require('../qna/qnamaker/qnaPrompt');
2929
const LUResource = require('./luResource');
30+
const InvalidCharsInIntentOrEntityName = require('./../utils/enums/invalidchars').InvalidCharsInIntentOrEntityName;
3031

3132
const plAllowedTypes = ["composite", "ml"];
3233
const featureTypeEnum = {
@@ -924,6 +925,16 @@ const parseAndHandleSimpleIntentSection = function (parsedContent, luResource, c
924925
let references = luResource.Sections.filter(s => s.SectionType === SectionType.REFERENCESECTION);
925926
for (const intent of intents) {
926927
let intentName = intent.Name;
928+
if (InvalidCharsInIntentOrEntityName.some(x => intentName.includes(x))) {
929+
let errorMsg = `Invalid intent line, intent name ${intentName} cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]`;
930+
let error = BuildDiagnostic({
931+
message: errorMsg,
932+
line: intent.Range.Start.Line
933+
})
934+
935+
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
936+
}
937+
927938
// insert only if the intent is not already present.
928939
addItemIfNotPresent(parsedContent.LUISJsonStructure, LUISObjNameEnum.INTENT, intentName);
929940
for (const utteranceAndEntities of intent.UtteranceAndEntitiesMap) {
@@ -953,6 +964,22 @@ const parseAndHandleSimpleIntentSection = function (parsedContent, luResource, c
953964
// examine and add these to filestoparse list.
954965
parsedContent.additionalFilesToParse.push(new fileToParse(parsedLinkUriInUtterance.fileName, false));
955966
}
967+
968+
(utteranceAndEntities.entities || []).forEach(entity => {
969+
let errors = []
970+
if (InvalidCharsInIntentOrEntityName.some(x => entity.entity.includes(x))) {
971+
let errorMsg = `Invalid utterance line, entity name ${entity.entity} in this utterance cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]`;
972+
let error = BuildDiagnostic({
973+
message: errorMsg,
974+
range: utteranceAndEntities.range
975+
});
976+
errors.push(error);
977+
}
978+
979+
if (errors.length > 0) {
980+
throw (new exception(retCode.errorCode.INVALID_LINE, errors.map(error => error.toString()).join('\n'), errors));
981+
}
982+
})
956983

957984
if (utteranceAndEntities.entities.length > 0) {
958985
let entitiesFound = utteranceAndEntities.entities;
@@ -1358,6 +1385,15 @@ const parseAndHandleEntityV2 = function (parsedContent, luResource, log, locale,
13581385
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
13591386
};
13601387

1388+
if (entityType !== EntityTypeEnum.PHRASELIST && InvalidCharsInIntentOrEntityName.some(x => entityName.includes(x))) {
1389+
let errorMsg = `Invalid entity line, entity name ${entityName} cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]`;
1390+
let error = BuildDiagnostic({
1391+
message: errorMsg,
1392+
line: entity.Range.Start.Line
1393+
})
1394+
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
1395+
}
1396+
13611397
if (entityType === entityName) {
13621398
let errorMsg = `Entity name "${entityName}" cannot be the same as entity type "${entityType}"`;
13631399
let error = BuildDiagnostic({
@@ -1455,7 +1491,7 @@ const handleNDepthEntity = function(parsedContent, entityName, entityRoles, enti
14551491
entityLines.forEach(child => {
14561492
currentParentEntity = undefined;
14571493
defLine++;
1458-
let captureGroups = /^((?<leadingSpaces>[ ]*)|(?<leadingTabs>[\t]*))-\s*@\s*(?<instanceOf>[^\s]+) (?<entityName>(?:'[^']+')|(?:"[^"]+")|(?:[^ '"=]+))(?: uses?[fF]eatures? (?<features>[^=]+))?\s*=?\s*$/g;
1494+
let captureGroups = /^((?<leadingSpaces>[ ]*)|(?<leadingTabs>[\t]*))-\s*@\s*(?<instanceOf>(?:'[^']+')|(?:"[^"]+")|(?:[^ '"=]+)) (?<entityName>(?:'[^']+')|(?:"[^"]+")|(?:[^ '"=]+))(?: uses?[fF]eatures? (?<features>[^=]+))?\s*=?\s*$/g;
14591495
let groupsFound = captureGroups.exec(child);
14601496
if (!groupsFound) {
14611497
let errorMsg = `Invalid child entity definition found for "${child.trim()}". Child definitions must start with '- @' and only include a type, name and optionally one or more usesFeature(s) definition.`;
@@ -1466,7 +1502,7 @@ const handleNDepthEntity = function(parsedContent, entityName, entityRoles, enti
14661502
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
14671503
}
14681504
let childEntityName = groupsFound.groups.entityName.replace(/^['"]/g, '').replace(/['"]$/g, '');
1469-
let childEntityType = groupsFound.groups.instanceOf.trim();
1505+
let childEntityType = groupsFound.groups.instanceOf.trim().replace(/^['"]/g, '').replace(/['"]$/g, '');
14701506
let childFeatures = groupsFound.groups.features ? groupsFound.groups.features.trim().split(/[,;]/g).map(item => item.trim()).filter(s => s) : undefined;
14711507

14721508
// Get current tab level
@@ -1640,6 +1676,16 @@ const handlePhraseList = function(parsedContent, entityName, entityType, entityR
16401676
});
16411677
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
16421678
}
1679+
1680+
// phraselist name can only contain letters (a-z, A-Z), numbers (0-9) and symbols @ # _ . , ^ \\ [ ]
1681+
if (!/[a-zA-Z0-9@#_,.,^\\\[\]]+$/.test(entityName.toLowerCase().includes('interchangeable') ? entityName.split(/\(.*\)/g)[0] : entityName)) {
1682+
const error = BuildDiagnostic({
1683+
message: `Invalid phraselist line, phraselist name ${entityName} can only contain letters (a-z, A-Z), numbers (0-9) and symbols @ # _ . , ^ \\ [ ]`,
1684+
line: range.Start.Line
1685+
});
1686+
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
1687+
};
1688+
16431689
let isPLEnabledForAllModels = undefined;
16441690
let isPLEnabled = undefined;
16451691
if (entityRoles.length !== 0) {
@@ -1946,6 +1992,16 @@ const parseAndHandleEntitySection = function (parsedContent, luResource, log, lo
19461992
for (const entity of entities) {
19471993
let entityName = entity.Name;
19481994
let entityType = entity.Type;
1995+
1996+
if (entityType !== EntityTypeEnum.PHRASELIST && InvalidCharsInIntentOrEntityName.some(x => entityName.includes(x))) {
1997+
let errorMsg = `Invalid entity line, entity name ${entityName} cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]`;
1998+
let error = BuildDiagnostic({
1999+
message: errorMsg,
2000+
line: entity.Range.Start.Line
2001+
})
2002+
throw (new exception(retCode.errorCode.INVALID_INPUT, error.toString(), [error]));
2003+
}
2004+
19492005
let parsedRoleAndType = helpers.getRolesAndType(entityType);
19502006
let entityRoles = parsedRoleAndType.roles;
19512007
entityType = parsedRoleAndType.entityType;

‎packages/lu/src/parser/lufile/visitor.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,6 @@ class Visitor {
4646
static recurselyResolveTokenizedUtterance(tokUtt, entities, errorMsgs, srcUtterance) {
4747
for (const item of tokUtt) {
4848
if (item === Object(item)) {
49-
let entityName = item.entityName.trim()
50-
if (entityName && InvalidCharsInIntentOrEntityName.some(x => entityName.includes(x))) {
51-
errorMsgs.push(`Invalid utterance line, entity name ${entityName} cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]`);
52-
continue;
53-
}
54-
5549
if (item.entityValue === undefined) {
5650
// we have a pattern.any entity
5751
const patternStr = item.role ? `{${item.entityName}:${item.role}}` : `{${item.entityName}}`

‎packages/lu/src/parser/luis/luConverter.js‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ const parseEntitiesToLu = function(luisJson){
234234
}
235235
fileContent += NEWLINE + NEWLINE;
236236
}
237-
fileContent += `@ ${getEntityType(entity.features)} ${writeEntityName(entity.name)}`;
237+
fileContent += `@ ml ${writeEntityName(entity.name)}`;
238238
fileContent += addRolesAndFeatures(entity);
239239
fileContent += NEWLINE + NEWLINE;
240240
} else {
@@ -417,7 +417,7 @@ const addAppMetaData = function(LUISJSON) {
417417
const handleNDepthEntity = function(entity) {
418418
let fileContent = '';
419419
const BASE_TAB_STOP = 1;
420-
fileContent += `@ ${getEntityType(entity.features)} ${writeEntityName(entity.name)}`;
420+
fileContent += `@ ml ${writeEntityName(entity.name)}`;
421421
fileContent += addRolesAndFeatures(entity);
422422
fileContent += NEWLINE;
423423
fileContent += addNDepthChildDefinitions(entity.children, BASE_TAB_STOP, fileContent) + NEWLINE + NEWLINE
@@ -434,7 +434,7 @@ const addNDepthChildDefinitions = function(childCollection, tabStop, fileContent
434434
(childCollection || []).forEach(child => {
435435
myFileContent += "".padStart(tabStop * 4, ' ');
436436
myFileContent += `- @ ${getEntityType(child.features)} ${writeEntityName(child.name)}`;
437-
myFileContent += addRolesAndFeatures(child);
437+
myFileContent += addRolesAndFeatures(child, true);
438438
myFileContent += NEWLINE;
439439
if (child.children && child.children.length !== 0) {
440440
myFileContent += addNDepthChildDefinitions(child.children, tabStop + 1, myFileContent);
@@ -446,17 +446,18 @@ const getEntityType = function(features) {
446446
// find constraint
447447
let constraint = (features || []).find(feature => feature.isRequired == true);
448448
if (constraint !== undefined) {
449-
return constraint.modelName;
449+
return writeEntityName(constraint.modelName);
450450
} else {
451451
return EntityTypeEnum.ML;
452452
}
453453
}
454454
/**
455455
* Helper to construt role and features list for an entity
456456
* @param {Object} entity
457+
* @param {boolean} isInNDepth
457458
* @returns {String} file content to include.
458459
*/
459-
const addRolesAndFeatures = function(entity) {
460+
const addRolesAndFeatures = function(entity, isInNDepth = false) {
460461
let roleAndFeatureContent = ''
461462
if (entity.roles && entity.roles.length > 0) {
462463
roleAndFeatureContent += ` ${entity.roles.length > 1 ? `hasRoles` : `hasRole`} `;
@@ -474,8 +475,11 @@ const addRolesAndFeatures = function(entity) {
474475
if (item.featureName) featuresList.push(item.featureName);
475476
if (item.modelName) {
476477
if (item.isRequired !== undefined) {
477-
if (item.isRequired !== true)
478+
if (item.isRequired !== true) {
478479
featuresList.push(item.modelName);
480+
} else if (!isInNDepth) {
481+
featuresList.push(item.modelName + '*');
482+
}
479483
} else {
480484
featuresList.push(item.modelName);
481485
}

‎packages/lu/src/parser/utils/helpers.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,16 @@ const addIsRequiredProperty = function(item, phraseListInFinal = []) {
569569
delete feature.modelName;
570570
}
571571

572+
if (feature.featureName && feature.featureName.endsWith('*')) {
573+
feature.featureName = feature.featureName.slice(0, feature.featureName.length - 1);
574+
feature.isRequired = true;
575+
}
576+
577+
if (feature.modelName && feature.modelName.endsWith('*')) {
578+
feature.modelName = feature.modelName.slice(0, feature.modelName.length - 1);
579+
feature.isRequired = true;
580+
}
581+
572582
delete feature.featureType;
573583
delete feature.modelType;
574584
});

‎packages/lu/test/commands/luis/convert.test.js‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,22 @@ describe('luis:convert version 7 upgrade test', () => {
195195
it('Child entities names with spaces in them parse correctly to .lu format', async () => {
196196
await assertToLu('./../../fixtures/testcases/Child_Entity_With_Spaces.json', './../../fixtures/verified/Child_Entity_With_Spaces.lu')
197197
})
198+
199+
it('luis:convert successfully converts LUIS JSON model with required feature defined at top level to .lu format)', async () => {
200+
await assertToJSON('./../../fixtures/verified/requiredFeatureAtTopLevel.lu', './../../fixtures/verified/requiredFeatureAtTopLevel.json')
201+
})
202+
203+
it('luis:convert successfully converts .lu format with required feature defined at top level to LUIS JSON model', async () => {
204+
await assertToLu('./../../fixtures/verified/requiredFeatureAtTopLevel.json', './../../fixtures/verified/requiredFeatureAtTopLevel.lu')
205+
})
206+
207+
it('luis:convert successfully converts LUIS JSON model with space in child entity definition to .lu format', async () => {
208+
await assertToJSON('./../../fixtures/verified/childEntityDefinitionWithSpace.lu', './../../fixtures/verified/childEntityDefinitionWithSpace.json')
209+
})
210+
211+
it('luis:convert successfully converts .lu format with space in child entity definition to LUIS JSON model', async () => {
212+
await assertToLu('./../../fixtures/verified/childEntityDefinitionWithSpace.json', './../../fixtures/verified/childEntityDefinitionWithSpace.lu')
213+
})
198214
})
199215

200216
describe('luis:convert negative tests', () => {
@@ -256,8 +272,43 @@ describe('luis:convert negative tests', () => {
256272
LuisBuilder.fromLUAsync(res)
257273
.then(res => done(res))
258274
.catch(err => {
259-
assert.isTrue(err.text.includes('[ERROR] line 2:0 - line 2:26: Invalid utterance line, entity name @addto*Property cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]'))
260-
assert.isTrue(err.text.includes('[ERROR] line 4:0 - line 4:20: Invalid entity line, entity name delete$Property cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]'))
275+
assert.isTrue(err.text.includes('[ERROR] line 2:0 - line 2:26: Invalid utterance line, entity name @addto*Property in this utterance cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]'))
276+
done()
277+
})
278+
})
279+
})
280+
281+
it('luis:convert should show ERR message when entity name defined at top level contains invalid char', (done) => {
282+
loadLuFile('./../../fixtures/testcases/bad6.lu')
283+
.then(res => {
284+
LuisBuilder.fromLUAsync(res)
285+
.then(res => done(res))
286+
.catch(err => {
287+
assert.isTrue(err.text.includes('[ERROR] line 1:0 - line 1:1: Invalid entity line, entity name location* cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]'))
288+
done()
289+
})
290+
})
291+
})
292+
293+
it('luis:convert should show ERR message when intent name contains invalid char', (done) => {
294+
loadLuFile('./../../fixtures/testcases/bad7.lu')
295+
.then(res => {
296+
LuisBuilder.fromLUAsync(res)
297+
.then(res => done(res))
298+
.catch(err => {
299+
assert.isTrue(err.text.includes('[ERROR] line 1:0 - line 1:1: Invalid intent line, intent name greeting* cannot contain any of the following characters: [<, >, *, %, &, :, \\, $]'))
300+
done()
301+
})
302+
})
303+
})
304+
305+
it('luis:convert should show ERR message when phraselist name contains invalid char', (done) => {
306+
loadLuFile('./../../fixtures/testcases/bad8.lu')
307+
.then(res => {
308+
LuisBuilder.fromLUAsync(res)
309+
.then(res => done(res))
310+
.catch(err => {
311+
assert.isTrue(err.text.includes('[ERROR] line 1:0 - line 1:1: Invalid phraselist line, phraselist name pl* can only contain letters (a-z, A-Z), numbers (0-9) and symbols @ # _ . , ^ \\ [ ]'))
261312
done()
262313
})
263314
})

‎packages/lu/test/fixtures/testcases/bad5.lu‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,2 @@
11
# greeting
2-
- hi {@addto*Property=foo}
3-
4-
@ ml delete$Property
2+
- hi {@addto*Property=foo}

‎packages/lu/test/fixtures/testcases/bad6.lu‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
@ ml location*

‎packages/lu/test/fixtures/testcases/bad7.lu‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# greeting*
2+
- hi

0 commit comments

Comments
 (0)