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.

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

fix: Check all expressions in dialog #1798

Merged
merged 31 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
129807d
check all expressions in dialog
lei9444 Dec 19, 2019
3e535fc
update the schema"
lei9444 Dec 23, 2019
716d4ec
update schema
lei9444 Dec 23, 2019
0ee6ae5
Merge branch 'master' into fixcheck
lei9444 Dec 23, 2019
944b273
check items in schema
lei9444 Dec 24, 2019
b262a69
navigation
lei9444 Dec 24, 2019
3bf8dae
update the fragment function
lei9444 Dec 24, 2019
9ff6938
move convert function to client
lei9444 Dec 24, 2019
0dd2282
fix unit test
lei9444 Dec 25, 2019
7a4ef0c
update the diagnostic path
lei9444 Dec 25, 2019
58fdb72
commit -m change the checkfun structure
lei9444 Dec 27, 2019
bc17a5a
update the schema
lei9444 Dec 27, 2019
b1d0344
expression regx
alanlong9278 Dec 27, 2019
954bc4c
Merge branch 'fixcheck' of https://github.com/lei9444/BotFramework-Co…
alanlong9278 Dec 27, 2019
478be0b
ui adjust for assignment property in setProperties
alanlong9278 Dec 28, 2019
7ca6362
Merge branch 'master' into fixcheck
lei9444 Dec 28, 2019
b8ecd88
fix e2e bug
alanlong9278 Dec 29, 2019
f022292
fix the array copy error
lei9444 Dec 30, 2019
0ba0f19
Merge branch 'master' into fixcheck
lei9444 Dec 30, 2019
24d83dc
sync expression error in formEditor
alanlong9278 Dec 30, 2019
686c152
Merge branch 'fixcheck' of https://github.com/lei9444/BotFramework-Co…
alanlong9278 Dec 30, 2019
b3a73b8
fix some comments
lei9444 Dec 31, 2019
a0fd9c3
Merge branch 'fixcheck' of https://github.com/lei9444/BotFramework-Co…
lei9444 Dec 31, 2019
8b6395f
Merge branch 'master' into fixcheck
lei9444 Dec 31, 2019
89998e0
move diagnostic path to shell
lei9444 Dec 31, 2019
9d7b8f9
Merge branch 'fixcheck' of https://github.com/lei9444/BotFramework-Co…
lei9444 Dec 31, 2019
512c567
fix unit test
lei9444 Dec 31, 2019
0ccc90d
update shared and indexers
lei9444 Jan 2, 2020
26a967d
fix spell error
lei9444 Jan 2, 2020
011e3ae
Merge branch 'master' into fixcheck
lei9444 Jan 6, 2020
c898ae0
Merge branch 'master' into fixcheck
cwhitten Jan 6, 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import { parsePathToFocused } from '../../../src/utils/convertUtils';

describe('parsePathToFocused', () => {
it('should return focusedPath', () => {
expect(parsePathToFocused('')).toBe('');
expect(parsePathToFocused('main.triggers[0].actions[0]')).toBe('triggers[0].actions[0]');
expect(parsePathToFocused('main.triggers[0].actions[0].actions[1]')).toBe('triggers[0].actions[0].actions[1]');
expect(parsePathToFocused('main.triggers[0].actions[0].elseActions[1]')).toBe(
'triggers[0].actions[0].elseActions[1]'
);
});
});
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License

import { parsePathToSelected } from '../../src';
import { parsePathToSelected } from '../../../src/utils/convertUtils';

describe('parsePathToSelected', () => {
it('should return selected path', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License
import { PromptTab } from '@bfc/shared';

import { parseTypeToFragment, PromptTab } from './../../src';
import { parseTypeToFragment } from '../../../src/utils/convertUtils';

describe('parseTypeToFragment', () => {
it('should return corrent tab name', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { parsePathToSelected } from './parsePathToSelected';

export function parsePathToFocused(path: string): string {
//path is like main.trigers[0].actions[0]
lei9444 marked this conversation as resolved.
Show resolved Hide resolved

const trigger = parsePathToSelected(path);

const list = path.split('.');

const matchActions = list.filter(x => x.startsWith('actions') || x.startsWith('elseActions'));
lei9444 marked this conversation as resolved.
Show resolved Hide resolved

if (matchActions.length > 0) {
return trigger + '.' + matchActions.join('.');
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
}
return '';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { dialogGroups, DialogGroup, PromptTab } from '@bfc/shared';

export function parseTypeToFragment(type: string, property: string): string {
const inputTypes = dialogGroups[DialogGroup.INPUT].types;
const index = inputTypes.findIndex(t => t === type);
if (index >= 0) {
switch (property) {
case 'prompt':
return PromptTab.BOT_ASKS;
case 'property':
case 'choices':
case 'outputFormat':
return PromptTab.USER_INPUT;
default:
return PromptTab.OTHER;
}
}
return '';
}
2 changes: 1 addition & 1 deletion Composer/packages/client/src/utils/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
import cloneDeep from 'lodash/cloneDeep';
import { navigate, NavigateOptions } from '@reach/router';
import { Diagnostic } from '@bfc/indexers';
import { parsePathToFocused, parsePathToSelected, parseTypeToFragment } from '@bfc/shared';

import { BreadcrumbItem, DesignPageLocation } from '../store/types';

import { parsePathToFocused, parsePathToSelected, parseTypeToFragment } from './convertUtils';
import { BASEPATH } from './../constants/index';
import { resolveToBasePath } from './fileUtil';

Expand Down

This file was deleted.

24 changes: 12 additions & 12 deletions Composer/packages/lib/indexers/src/dialogIndexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
import has from 'lodash/has';
import uniq from 'lodash/uniq';

import { checkerFuncs } from './dialogUtils/dialogChecker';
import { ITrigger, DialogInfo, FileInfo } from './type';
import { JsonWalk, VisitorFunc } from './utils/jsonWalk';
import { getBaseName } from './utils/help';
import { Diagnostic } from './diagnostic';
import { extractLgTemplateRefs } from './lgUtils/parsers/parseLgTemplateRef';
import { getExpressionProperties } from './dialogUtils/extractExpressionDefinitions';
import { IsExpression } from './dialogUtils';

// find out all lg templates given dialog
function ExtractLgTemplates(dialog): string[] {
const templates: string[] = [];
Expand Down Expand Up @@ -127,27 +127,26 @@ function ExtractReferredDialogs(dialog): string[] {

// check all fields
function CheckFields(dialog, id: string, schema: any): Diagnostic[] {
const errors: Diagnostic[] = [];
const expressionProperties = getExpressionProperties(schema);
const diagnostics: Diagnostic[] = [];
/**
*
* @param path , jsonPath string
* @param value , current node value *
* @return boolean, true to stop walk
* */
const visitor: VisitorFunc = (path: string, value: any): boolean => {
// it's a valid schema dialog node.
if (has(value, '$type') && has(expressionProperties, value.$type)) {
const diagnostics = IsExpression(path, value, { ...expressionProperties[value.$type] });

if (diagnostics) {
errors.push(...diagnostics);
}
if (has(value, '$type')) {
checkerFuncs.forEach(checkerFunc => {
const result = checkerFunc(path, value, value.$type, schema.definitions[value.$type]);
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
if (result) {
diagnostics.splice(0, 0, ...result);
}
});
}
return false;
};
JsonWalk(id, dialog, visitor);
return errors.map(e => {
return diagnostics.map(e => {
e.source = id;
return e;
});
Expand All @@ -164,6 +163,7 @@ function validate(id: string, content, schema: any): Diagnostic[] {
function parse(id: string, content: any, schema: any) {
const luFile = typeof content.recognizer === 'string' ? content.recognizer : '';
const lgFile = typeof content.generator === 'string' ? content.generator : '';

return {
content,
diagnostics: validate(id, content, schema),
Expand Down
105 changes: 83 additions & 22 deletions Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,91 @@ import { CheckerFunc } from './types';

const ExpressionParser = new ExpressionEngine();

export const IsExpression: CheckerFunc = (
path,
value,
optional: { properties: string[]; requiredTypes: { [key: string]: boolean } }
) => {
const { properties, requiredTypes } = optional;
return properties.reduce((result: Diagnostic[], property) => {
let message = '';
const exp = get(value, property);
if (!exp && requiredTypes[property]) {
message = formatMessage(`is missing or empty`);
} else {
try {
ExpressionParser.parse(exp);
} catch (error) {
message = `${formatMessage('must be an expression:')} ${error})`;
const createPath = (path: string, type: string): string => {
const steps = ['triggers', 'actions', 'elseActions'];
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
let list = path.split('.');
const matchs = list.filter(x => !steps.every(step => !x.startsWith(step)));
lei9444 marked this conversation as resolved.
Show resolved Hide resolved

const focused = matchs.join('.');
list = path.split(`${focused}.`);
if (list.length !== 2) return path;

return `${list[0]}${focused}#${type}#${list[1]}`;
};

export const checkExpression = (exp: string, required: boolean, path: string, type: string): Diagnostic | null => {
let message = '';
if (!exp && required) {
message = formatMessage(`is missing or empty`);
} else {
try {
ExpressionParser.parse(exp);
} catch (error) {
message = `${formatMessage('must be an expression:')} ${error})`;
}
}
if (message) {
const diagnostic = new Diagnostic(message, '');
diagnostic.path = createPath(path, type);
return diagnostic;
}

return null;
};

function findAllRequiredType(schema: any): { [key: string]: boolean } {
const types = schema.anyOf?.filter(x => x.title === 'Type');
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
const required = {};
if (types && types.length) {
types[0].required.forEach((element: string) => {
required[element] = true;
});
}

if (schema.required) {
schema.required.forEach((element: string) => {
required[element] = true;
});
}
return required;
}

export const IsExpression: CheckerFunc = (path, value, type, schema) => {
const diagnostics: Diagnostic[] = [];
const requiredTypes = findAllRequiredType(schema);
Object.keys(value).forEach(key => {
const property = value[key];
if (Array.isArray(property)) {
const itemsSchema = schema.properties[key].items;
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
if (itemsSchema?.$role === 'expression') {
lei9444 marked this conversation as resolved.
Show resolved Hide resolved
property.forEach((child, index) => {
const diagnostic = checkExpression(child, !!requiredTypes[key], `${path}.${key}[${index}]`, type);
if (diagnostic) diagnostics.push(diagnostic);
});
} else if (itemsSchema?.type === 'object') {
property.forEach((child, index) => {
const result = IsExpression(`${path}.${key}[${index}]`, child, type, itemsSchema);
if (result) diagnostics.splice(0, 0, ...result);
});
}
} else if (get(schema.properties[key], '$role') === 'expression') {
const diagnostic = checkExpression(property, !!requiredTypes[key], `${path}.${key}`, type);
if (diagnostic) diagnostics.push(diagnostic);
}
if (message) {
const diagnostic = new Diagnostic(message, '');
diagnostic.path = `${path}#${value.$type}#${property}`;
result.push(diagnostic);
});
return diagnostics;
};

//the type of 'Microsoft.ChoiceInput' has anyof schema in choices
export const checkChoices: CheckerFunc = (path, value, type, schema) => {
if (type === 'Microsoft.ChoiceInput') {
const choices = value.choices;
if (typeof choices === 'string') {
const diagnostic = checkExpression(choices, false, `${path}.choices`, type);
if (diagnostic) return [diagnostic];
}
return result;
}, []);
}
return null;
};

export const checkerFuncs: CheckerFunc[] = [IsExpression, checkChoices];

This file was deleted.

1 change: 0 additions & 1 deletion Composer/packages/lib/indexers/src/dialogUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@
// Licensed under the MIT License.

export * from './dialogChecker';
export * from './extractExpressionDefinitions';
export * from './types';
2 changes: 1 addition & 1 deletion Composer/packages/lib/indexers/src/dialogUtils/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ export interface IExpressionProperties {
};
}

export type CheckerFunc = (path: string, value: any, optional?: any) => Diagnostic[] | null; // error msg
export type CheckerFunc = (path: string, value: any, type: string, schema: any) => Diagnostic[] | null; // error msg
Loading