Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 2 additions & 1 deletion specification/eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export default defineConfig({
}
}
],
'es-spec-validator/no-all-string-literal-unions': 'error'
'es-spec-validator/no-all-string-literal-unions': 'error',
'es-spec-validator/check-variants': 'error'
}
})
27 changes: 14 additions & 13 deletions validator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,20 @@ 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. |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! Although I think it would be nice to extend off of the existing variant rule...at least thats what I had in mind

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make a lot of sense. I will refactor it into the existing rule Monday morning! 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@margaretjgu refactored in e414575 :)

| `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. |
| `check-variants` | Checks that `@variants` are valid for specific types. `Request` and `Response`, no variants allowed; Type Aliases, `internal`, `typed_keys_quirk` or `untagged`; Interface classes, `container` |

## Usage

Expand Down
4 changes: 3 additions & 1 deletion validator/eslint-plugin-es-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import noInlineUnions from './rules/no-inline-unions.js'
import preferTaggedVariants from './rules/prefer-tagged-variants.js'
import noDuplicateTypeNames from './rules/no-duplicate-type-names.js'
import noAllStringLiteralUnions from './rules/no-all-string-literal-unions.js'
import checkVariants from './rules/check-variants.js'

export default {
rules: {
Expand All @@ -40,6 +41,7 @@ export default {
'no-inline-unions': noInlineUnions,
'prefer-tagged-variants': preferTaggedVariants,
'no-duplicate-type-names': noDuplicateTypeNames,
'no-all-string-literal-unions': noAllStringLiteralUnions
'no-all-string-literal-unions': noAllStringLiteralUnions,
'check-variants': checkVariants
}
}
122 changes: 122 additions & 0 deletions validator/rules/check-variants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { ESLintUtils } from '@typescript-eslint/utils'

export const checkVariants = ESLintUtils.RuleCreator.withoutDocs({
name: 'check-variants',
meta: {
type: 'problem',
docs: {
description: 'Checks that variants are allowed on a given type.',
recommended: 'error'
},
messages: {
variantsOnRequestOrResponse:
'Variants are not allowed on {{ className }} classes.',
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 }}."
}
},
defaultOptions: [],
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() || '' }))
}

const isRequestOrResponse = (name) =>
name === 'Request' || name === 'Response'

const hasVariantsTag = (tags) => tags.some(({ tag }) => tag === 'variants')

return {
'TSInterfaceDeclaration, ClassDeclaration'(node) {
const jsDocTags = getJsDocTags(node)
if (isRequestOrResponse(node.id.name)) {
if (hasVariantsTag(jsDocTags)) {
context.report({
node,
messageId: 'variantsOnRequestOrResponse',
data: { className: node.id.name }
})
}
return
}

const nonContainerVariant = jsDocTags.find(
({ tag, value }) => tag === 'variants' && value !== 'container'
)
if (nonContainerVariant) {
context.report({
node,
messageId: 'interfaceWithNonContainerVariants',
data: {
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(', ')
}
})
}
}
}
}
})

export default checkVariants
74 changes: 74 additions & 0 deletions validator/test/check-variants.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { RuleTester } from '@typescript-eslint/rule-tester'
import rule from '../rules/check-variants.js'

const ruleTester = new RuleTester({
languageOptions: {
parserOptions: {
projectService: {
allowDefaultProject: ['*.ts*']
},
tsconfigRootDir: import.meta.dirname
}
}
})

ruleTester.run('check-variants', rule, {
valid: [
{
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: [
{
name: 'Request has variants tag',
code: `/** @variants container */
export class Request {}`,
errors: [{ messageId: 'variantsOnRequestOrResponse' }]
},
{
name: 'Response has variants tag',
code: `/** @variants container */
export class Response {}`,
errors: [{ messageId: 'variantsOnRequestOrResponse' }]
},
{
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' }]
}
]
})
Loading