diff --git a/validator/README.md b/validator/README.md index c378497d9c..db6dc8b10e 100644 --- a/validator/README.md +++ b/validator/README.md @@ -5,19 +5,19 @@ It is configured [in the specification directory](../specification/eslint.config ## Rules -| Name | Description | -|---------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `single-key-dictionary-key-is-string` | `SingleKeyDictionary` keys must be strings. | -| `dictionary-key-is-string` | `Dictionary` keys must be strings. | -| `no-native-types` | TypeScript native utility types (`Record`, `Partial`, etc.) and collection types (`Map`, `Set`, etc.) are not allowed. Use spec-defined aliases like `Dictionary` instead. | -| `invalid-node-types` | The spec uses a subset of TypeScript, so some types, clauses and expressions are not allowed. | -| `no-generic-number` | Generic `number` type is not allowed outside of `_types/Numeric.ts`. Use concrete numeric types like `integer`, `long`, `float`, `double`, etc. | -| `request-must-have-urls` | All Request interfaces extending `RequestBase` must have a `urls` property defining their endpoint paths and HTTP methods. | -| `no-variants-on-responses` | `@variants` is only supported on Interface types, not on Request or Response classes. Use value_body pattern with `@codegen_name` instead. | -| `no-inline-unions` | Inline union types (e.g., `field: A \| B`) are not allowed in properties/fields. Define a named type alias instead to improve code generation for statically-typed languages. | -| `prefer-tagged-variants` | Union of class types should use tagged variants (`@variants internal` or `@variants container`) instead of inline unions for better deserialization support in statically-typed languages. | -| `no-duplicate-type-names` | All types must be unique across class and enum definitions. | -| `no-all-string-literal-unions` | Unions consisting entirely of string literals (e.g., `"green" \| "yellow" \| "red"`) are not allowed, use enums instead. | | +| Name | Description | +|---------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `single-key-dictionary-key-is-string` | `SingleKeyDictionary` keys must be strings. | +| `dictionary-key-is-string` | `Dictionary` keys must be strings. | +| `no-native-types` | TypeScript native utility types (`Record`, `Partial`, etc.) and collection types (`Map`, `Set`, etc.) are not allowed. Use spec-defined aliases like `Dictionary` instead. | +| `invalid-node-types` | The spec uses a subset of TypeScript, so some types, clauses and expressions are not allowed. | +| `no-generic-number` | Generic `number` type is not allowed outside of `_types/Numeric.ts`. Use concrete numeric types like `integer`, `long`, `float`, `double`, etc. | +| `request-must-have-urls` | All Request interfaces extending `RequestBase` must have a `urls` property defining their endpoint paths and HTTP methods. | +| `no-variants-on-responses` | `@variants` is only supported on Interface types, not on Request or Response classes. Use value_body pattern with `@codegen_name` instead. Includes additional checks on variant tag use. | +| `no-inline-unions` | Inline union types (e.g., `field: A \| B`) are not allowed in properties/fields. Define a named type alias instead to improve code generation for statically-typed languages. | +| `prefer-tagged-variants` | Union of class types should use tagged variants (`@variants internal` or `@variants container`) instead of inline unions for better deserialization support in statically-typed languages. | +| `no-duplicate-type-names` | All types must be unique across class and enum definitions. | +| `no-all-string-literal-unions | Unions consisting entirely of string literals (e.g., `"green" \| "yellow" \| "red"`) are not allowed, use enums instead. | | `jsdoc-endpoint-check` | Validates JSDoc on endpoints in the specification. Ensuring consistent formatting. Some errors can be fixed with `--fix`. | ## Usage diff --git a/validator/rules/no-variants-on-responses.js b/validator/rules/no-variants-on-responses.js index ff6207b4e9..e834eda7f9 100644 --- a/validator/rules/no-variants-on-responses.js +++ b/validator/rules/no-variants-on-responses.js @@ -16,51 +16,124 @@ * specific language governing permissions and limitations * under the License. */ -import { ESLintUtils } from '@typescript-eslint/utils'; +import { ESLintUtils } from '@typescript-eslint/utils' -const createRule = ESLintUtils.RuleCreator(name => `https://example.com/rule/${name}`) +const createRule = ESLintUtils.RuleCreator( + (name) => `https://example.com/rule/${name}` +) export default createRule({ name: 'no-variants-on-responses', create(context) { + const sourceCode = context.sourceCode || context.getSourceCode() + + const getJsDocTags = (node) => { + const targetNode = + node.parent?.type === 'ExportNamedDeclaration' ? node.parent : node + const comments = sourceCode.getCommentsBefore(targetNode) + + const jsDocComment = comments + ?.filter( + (comment) => comment.type === 'Block' && comment.value.startsWith('*') + ) + .pop() + + if (!jsDocComment) return [] + + return jsDocComment.value + .split('\n') + .map((line) => line.trim().match(/^\*?\s*@(\w+)(?:\s+(.*))?$/)) + .filter(Boolean) + .map(([, tag, value]) => ({ tag, value: value?.trim() || '' })) + } + return { - ClassDeclaration(node) { - const className = node.id?.name; - if (className !== 'Response' && className !== 'Request') { - return; + 'TSInterfaceDeclaration, ClassDeclaration'(node) { + const className = node.id?.name + if (className === 'Response' || className === 'Request') { + const fullText = sourceCode.text + + const nodeStart = node.range[0] + const textBefore = fullText.substring( + Math.max(0, nodeStart - 200), + nodeStart + ) + + const hasVariantsTag = + /@variants\s+(container|internal|external|untagged)/.test( + textBefore + ) + + if (hasVariantsTag) { + context.report({ + node, + messageId: 'noVariantsOnResponses', + data: { + className, + suggestion: + 'Move @variants to a separate body class and use value_body pattern with @codegen_name. See SearchResponse for an example.' + } + }) + } + return } - - const sourceCode = context.sourceCode || context.getSourceCode(); - const fullText = sourceCode.text; - - const nodeStart = node.range[0]; - const textBefore = fullText.substring(Math.max(0, nodeStart - 200), nodeStart); - - const hasVariantsTag = /@variants\s+(container|internal|external|untagged)/.test(textBefore); - - if (hasVariantsTag) { + + const jsDocTags = getJsDocTags(node) + + const nonContainerVariant = jsDocTags.find( + ({ tag, value }) => tag === 'variants' && value !== 'container' + ) + if (nonContainerVariant) { context.report({ node, - messageId: 'noVariantsOnResponses', + messageId: 'interfaceWithNonContainerVariants', data: { - className, - suggestion: 'Move @variants to a separate body class and use value_body pattern with @codegen_name. See SearchResponse for an example.' + interfaceName: node.id.name, + variantValue: nonContainerVariant.value } - }); + }) + return } }, + TSTypeAliasDeclaration(node) { + const jsDocTags = getJsDocTags(node) + const allowedVariants = ['internal', 'typed_keys_quirk', 'untagged'] + + const invalidVariant = jsDocTags.find( + ({ tag, value }) => + tag === 'variants' && + !allowedVariants.some((allowed) => value.startsWith(allowed)) + ) + + if (invalidVariant) { + context.report({ + node, + messageId: 'invalidVariantsTag', + data: { + typeName: node.id.name, + variantValue: invalidVariant.value, + allowedValues: allowedVariants.join(', ') + } + }) + } + } } }, meta: { docs: { - description: '@variants is only supported on Interface types, not on Request or Response classes. Use value_body pattern instead.', + description: + '@variants is only supported on Interface types, not on Request or Response classes. Use value_body pattern instead.' }, messages: { - noVariantsOnResponses: '@variants on {{className}} is not supported in metamodel. {{suggestion}}' + noVariantsOnResponses: + '@variants on {{className}} is not supported in metamodel. {{suggestion}}', + interfaceWithNonContainerVariants: + "Interface '{{ interfaceName }}' has '@variants {{ variantValue }}' but only 'container' is allowed for interfaces.", + invalidVariantsTag: + "Type alias '{{ typeName }}' has invalid '@variants {{ variantValue }}'. Must start with: {{ allowedValues }}." }, type: 'problem', schema: [] }, defaultOptions: [] }) - diff --git a/validator/test/no-variants-on-responses.test.js b/validator/test/no-variants-on-responses.test.js index a1bbc0cae2..ddd99c5123 100644 --- a/validator/test/no-variants-on-responses.test.js +++ b/validator/test/no-variants-on-responses.test.js @@ -23,11 +23,11 @@ const ruleTester = new RuleTester({ languageOptions: { parserOptions: { projectService: { - allowDefaultProject: ['*.ts*'], + allowDefaultProject: ['*.ts*'] }, - tsconfigRootDir: import.meta.dirname, - }, - }, + tsconfigRootDir: import.meta.dirname + } + } }) ruleTester.run('no-variants-on-responses', rule, { @@ -42,7 +42,7 @@ ruleTester.run('no-variants-on-responses', rule, { classification?: ClassificationSummary regression?: RegressionSummary }`, - + `export class Request { path_parts: {} query_parameters: {} @@ -51,19 +51,31 @@ ruleTester.run('no-variants-on-responses', rule, { /** @variants internal tag='type' */ export type RequestBody = TypeA | TypeB`, - + `/** @variants container */ export interface MyContainer { option_a?: OptionA option_b?: OptionB }`, - + `export class Response { body: { count: integer results: string[] } }`, + { + name: 'not Request or Response', + code: `/** @variants container */ +export class MyClass { + body: MyContainer +}` + }, + { + name: 'internal tag on type alias', + code: `/** @variants internal tag='type' */ +export type MyType = string | number` + } ], invalid: [ { @@ -92,6 +104,29 @@ ruleTester.run('no-variants-on-responses', rule, { }`, errors: [{ messageId: 'noVariantsOnResponses' }] }, - ], + { + name: 'Request has variants tag', + code: `/** @variants container */ +export class Request {}`, + errors: [{ messageId: 'noVariantsOnResponses' }] + }, + { + name: 'Response has variants tag', + code: `/** @variants container */ +export class Response {}`, + errors: [{ messageId: 'noVariantsOnResponses' }] + }, + { + name: 'Interface has non-container variants tag', + code: `/** @variants internal */ +export class RankContainer {}`, + errors: [{ messageId: 'interfaceWithNonContainerVariants' }] + }, + { + name: 'Type alias has invalid variants tag', + code: `/** @variants invalid */ +export type MyType = string | number`, + errors: [{ messageId: 'invalidVariantsTag' }] + } + ] }) -