Merge pull request #554 from smartxworks/fix/validators-bugs-2

Fix/validators bugs 2

packages/editor/__tests__/model/fieldModel.spec.ts

@@ -35,6 +35,14 @@ describe('Field test', () => {
     ]);
   });
 
+  it('stop member expression when meeting other expression ast node', () => {
+    const field = new FieldModel('{{ Array.from([]).fill() }}');
+    expect(field.isDynamic).toEqual(true);
+    expect(field.refComponentInfos['Array' as ComponentId].refProperties).toEqual([
+      'from',
+    ]);
+  });
+
   it('parse inline variable in expression', () => {
     const field = new FieldModel('{{ [].length }}');
     expect(field.isDynamic).toEqual(true);
@@ -86,6 +94,24 @@ describe('Field test', () => {
     expect(field.refComponentInfos).toEqual({});
   });
 
+  it('should not count object keys in refs', () => {
+    const field = new FieldModel('{{ {foo: 1, bar: 2, baz: 3, } }}');
+    expect(field.isDynamic).toEqual(true);
+    expect(field.refComponentInfos).toEqual({});
+  });
+
+  it('should not count keys of object destructuring assignment in refs', () => {
+    const field = new FieldModel('{{ ({foo: bar}) => bar }}');
+    expect(field.isDynamic).toEqual(true);
+    expect(field.refComponentInfos).toEqual({});
+  });
+
+  it('should not count keys of array destructuring assignment in refs', () => {
+    const field = new FieldModel('{{ ([bar]) => bar }}');
+    expect(field.isDynamic).toEqual(true);
+    expect(field.refComponentInfos).toEqual({});
+  });
+
   it('get value by path', () => {
     const field = new FieldModel({ foo: [{}, { bar: { baz: 'Hello, world!' } }] });
     expect(field.getPropertyByPath('foo.1.bar.baz')?.getValue()).toEqual('Hello, world!');

packages/editor/src/AppModel/FieldModel.ts

@@ -21,7 +21,11 @@ import { JSONSchema7 } from 'json-schema';
 
 export type FunctionNode = ASTNode & { params: ASTNode[] };
 export type DeclaratorNode = ASTNode & { id: ASTNode };
+export type ObjectPatternNode = ASTNode & { properties: PropertyNode[] };
+export type ArrayPatternNode = ASTNode & { elements: ASTNode[] };
+export type PropertyNode = ASTNode & { value: ASTNode };
 export type LiteralNode = ASTNode & { raw: string };
+export type SequenceExpressionNode = ASTNode & { expressions: LiteralNode[] };
 export type ExpressionNode = ASTNode & {
   object: ExpressionNode;
   property: ExpressionNode | LiteralNode;
@@ -164,15 +168,14 @@ export class FieldModel implements IFieldModel {
     exps.forEach(exp => {
       let lastIdentifier: ComponentId = '' as ComponentId;
       const node = (acornLoose as typeof acorn).parse(exp, { ecmaVersion: 2020 });
-
       this.astNodes[exp] = node as ASTNode;
-
-      // These are variables of iife, they should be count in refs.
-      let localVariables: ASTNode[] = [];
+      // These are variables of iife or other identifiers, they can't be validated
+      // so they should not be added in refs
+      let whiteList: ASTNode[] = [];
 
       simpleWalk(node, {
         Function: functionNode => {
-          localVariables = [...localVariables, ...(functionNode as FunctionNode).params];
+          whiteList = [...whiteList, ...(functionNode as FunctionNode).params];
         },
         Expression: expressionNode => {
           switch (expressionNode.type) {
@@ -186,7 +189,7 @@ export class FieldModel implements IFieldModel {
                 this.refComponentInfos[key].componentIdASTNodes.push(
                   expressionNode as ASTNode
                 );
-              } else {
+              } else if (key) {
                 this.refComponentInfos[key] = {
                   componentIdASTNodes: [expressionNode as ASTNode],
                   refProperties: [],
@@ -196,20 +199,40 @@ export class FieldModel implements IFieldModel {
 
               break;
             case 'MemberExpression':
-              this.refComponentInfos[lastIdentifier]?.refProperties.push(
-                this.genPathFromMemberExpressionNode(expressionNode as ExpressionNode)
-              );
+              if (lastIdentifier) {
+                this.refComponentInfos[lastIdentifier]?.refProperties.push(
+                  this.genPathFromMemberExpressionNode(expressionNode as ExpressionNode)
+                );
+              }
+              break;
+            case 'SequenceExpression':
+              const sequenceExpression = expressionNode as SequenceExpressionNode;
+              whiteList.push(sequenceExpression.expressions[1]);
+              break;
+            case 'Literal':
+              // do nothing, just stop it from going to default
               break;
             default:
+              // clear lastIdentifier when meet other astNode to break the MemberExpression chain
+              lastIdentifier = '' as ComponentId;
           }
         },
+        ObjectPattern: objPatternNode => {
+          const propertyNodes = (objPatternNode as ObjectPatternNode).properties;
+          propertyNodes.forEach(property => {
+            whiteList.push(property.value);
+          });
+        },
+        ArrayPattern: arrayPatternNode => {
+          whiteList = [...whiteList, ...(arrayPatternNode as ArrayPatternNode).elements];
+        },
         VariableDeclarator: declarator => {
-          localVariables.push((declarator as DeclaratorNode).id);
+          whiteList.push((declarator as DeclaratorNode).id);
         },
       });
-      // remove localVariables from refs
+      // remove whiteList from refs
       for (const key in this.refComponentInfos) {
-        if (localVariables.some(({ name }) => key === name)) {
+        if (whiteList.some(({ name }) => key === name)) {
           delete this.refComponentInfos[key as any];
         }
       }

packages/editor/src/validator/rules/PropertiesRules.ts

@@ -103,6 +103,7 @@ class ExpressionValidatorRule implements PropertiesValidatorRule {
     // validate expression
     properties.traverse((fieldModel, key) => {
       Object.keys(fieldModel.refComponentInfos).forEach((id: string) => {
+        if (!id) return;
         const targetComponent = appModel.getComponentById(id as ComponentId);
         const paths = fieldModel.refComponentInfos[id as ComponentId].refProperties;
 

packages/editor/src/validator/rules/TraitRules.ts

@@ -8,11 +8,28 @@ import { GLOBAL_UTIL_METHOD_ID } from '@sunmao-ui/runtime';
 import { isExpression } from '../utils';
 import { ComponentId, EventName } from '../../AppModel/IAppModel';
 import { CORE_VERSION, CoreTraitName, EventHandlerSpec } from '@sunmao-ui/shared';
+import { get } from 'lodash';
+import { ErrorObject } from 'ajv';
 
 class EventHandlerValidatorRule implements TraitValidatorRule {
   kind: 'trait' = 'trait';
   traitMethods = ['setValue', 'resetValue', 'triggerFetch'];
 
+  private isErrorAnExpression(
+    error: ErrorObject,
+    parameters: Record<string, any> | undefined
+  ) {
+    let path = '';
+    const { instancePath, params } = error;
+    if (instancePath) {
+      path = instancePath.split('/').slice(1).join('.');
+    } else {
+      path = params.missingProperty;
+    }
+    const field = get(parameters, path);
+    return isExpression(field);
+  }
+
   validate({
     appModel,
     trait,
@@ -87,14 +104,16 @@ class EventHandlerValidatorRule implements TraitValidatorRule {
         const valid = validate(parameters);
         if (!valid) {
           validate.errors!.forEach(error => {
-            results.push({
-              message: error.message || '',
-              componentId: component.id,
-              property: error.instancePath,
-              traitType: trait?.type,
-              traitIndex,
-            });
-            return results;
+            if (!this.isErrorAnExpression(error, parameters)) {
+              results.push({
+                message: error.message || '',
+                componentId: component.id,
+                property: error.instancePath,
+                traitType: trait?.type,
+                traitIndex,
+              });
+              return results;
+            }
           });
         }
       } else {
@@ -126,22 +145,14 @@ class EventHandlerValidatorRule implements TraitValidatorRule {
         // check component method properties type
         if (method.parameters && !ajv.validate(method.parameters, parameters)) {
           ajv.errors!.forEach(error => {
-            if (error.keyword === 'type') {
-              const { instancePath } = error;
-              const path = instancePath.split('/')[1];
-              const value = trait.rawProperties[path];
-
-              // if value is an expression, skip it
-              if (isExpression(value)) {
-                return;
-              }
+            if (!this.isErrorAnExpression(error, parameters)) {
+              results.push({
+                message: error.message || '',
+                componentId: component.id,
+                traitType: trait.type,
+                property: `/handlers/${i}/method/parameters${error.instancePath}`,
+              });
             }
-            results.push({
-              message: error.message || '',
-              componentId: component.id,
-              traitType: trait.type,
-              property: `/handlers/${i}/method/parameters${error.instancePath}`,
-            });
           });
         }
       }