Azure / autorest.powershell

AutoRest PowerShell Generator
MIT License
112 stars 81 forks source link

[Discussion] directive has ambiguous syntax #1236

Open VeryEarly opened 1 year ago

VeryEarly commented 1 year ago

for where statement, following have ambiguous syntax

      const subjectRegex = getPatternToMatch(directive.where.subject);
      const subjectPrefixRegex = getPatternToMatch(directive.where['subject-prefix']);
      const verbRegex = getPatternToMatch(directive.where.verb);
      const variantRegex = getPatternToMatch(directive.where.variant);
      const parameterRegex = getPatternToMatch(directive.where['parameter-name']);
      const enumNameRegex = getPatternToMatch(directive.where['enum-name']);
      const enumValueNameRegex = getPatternToMatch(directive.where['enum-value-name']);
      const modelNameRegex = getPatternToMatch(directive.where['model-name']);
      const modelFullNameRegex = getPatternToMatch(directive.where['model-fullname']);
      const modelNamespaceRegex = getPatternToMatch(directive.where['model-namespace']);
      const propertyNameRegex = getPatternToMatch(directive.where['property-name']);

the method "getPatternToMatch" treat "pattern" as /^pattern$/ and "pattern." as /pattern./ for a regex, /pattern/ has different meaning from /^pattern$/, autorest.powershell should not make such assumption.

We should treat all input as regex.

dolauli commented 1 year ago

Let use https://github.com/Azure/autorest.powershell/blob/main/docs/directives.md#cmdlet-hiding-exportation-suppression for example. Current design: Filters without special characters, e.g. verb: update, it will be implemented /^update$/ instead of /update/ for exactly word match. Filters with special characters, e.g. subject: PetService.*, it will be implemented as /PetService./ Limitation of the design, if someone want to filter all the verb that contains update, he will need to use something like ```verb: update\d```

@VeryEarly's suggestion is in both cases, we just use regex. And for this solution, for an exact match, developers will need to change the directive from verb: update to verb: ^update$

dolauli commented 1 year ago

Considering it is a breaking change and the impact is low, it is not worth to make change. Will need to update the docs to make the current design clear to all the users.