Backport 5622 to 9.2 (#5812)

* Fix variants on Response class - use value_body pattern (#5622)

Co-authored-by: Quentin Pradet <[email protected]>

* fix merge

---------

Co-authored-by: Quentin Pradet <[email protected]>

Author: l-trotta

specification/eslint.config.js

@@ -37,6 +37,7 @@ export default defineConfig({
     'es-spec-validator/invalid-node-types': 'error',
     'es-spec-validator/no-generic-number': 'error',
     'es-spec-validator/request-must-have-urls': 'error',
+    'es-spec-validator/no-variants-on-responses': 'error',
     'es-spec-validator/jsdoc-endpoint-check': [
       'error',
       {

specification/ml/evaluate_data_frame/MlEvaluateDataFrameResponse.ts

@@ -23,22 +23,25 @@ import {
   DataframeRegressionSummary
 } from './types'
 
-/** @variants container */
 export class Response {
-  body: {
-    /**
-     * Evaluation results for a classification analysis.
-     * It outputs a prediction that identifies to which of the classes each document belongs.
-     */
-    classification?: DataframeClassificationSummary
-    /**
-     * Evaluation results for an outlier detection analysis.
-     * It outputs the probability that each document is an outlier.
-     */
-    outlier_detection?: DataframeOutlierDetectionSummary
-    /**
-     * Evaluation results for a regression analysis which outputs a prediction of values.
-     */
-    regression?: DataframeRegressionSummary
-  }
+  /** @codegen_name result */
+  body: ResponseBody
+}
+
+/** @variants container */
+export class ResponseBody {
+  /**
+   * Evaluation results for a classification analysis.
+   * It outputs a prediction that identifies to which of the classes each document belongs.
+   */
+  classification?: DataframeClassificationSummary
+  /**
+   * Evaluation results for an outlier detection analysis.
+   * It outputs the probability that each document is an outlier.
+   */
+  outlier_detection?: DataframeOutlierDetectionSummary
+  /**
+   * Evaluation results for a regression analysis which outputs a prediction of values.
+   */
+  regression?: DataframeRegressionSummary
 }

validator/README.md

@@ -13,6 +13,7 @@ It is configured [in the specification directory](../specification/eslint.config
 | `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. |
 | `jsdoc-endpoint-check`                | Validates JSDoc on endpoints in the specification. Ensuring consistent formatting. Some errors can be fixed with `--fix`.                                                                  |
 
 ## Usage

validator/eslint-plugin-es-spec.js

@@ -22,6 +22,7 @@ import noNativeTypes from './rules/no-native-types.js'
 import invalidNodeTypes from './rules/invalid-node-types.js'
 import noGenericNumber from './rules/no-generic-number.js'
 import requestMustHaveUrls from './rules/request-must-have-urls.js'
+import noVariantsOnResponses from './rules/no-variants-on-responses.js'
 import jsdocEndpointCheck from './rules/jsdoc-endpoint-check.js'
 
 export default {
@@ -32,6 +33,7 @@ export default {
     'invalid-node-types': invalidNodeTypes,
     'no-generic-number': noGenericNumber,
     'request-must-have-urls': requestMustHaveUrls,
+    'no-variants-on-responses': noVariantsOnResponses,
     'jsdoc-endpoint-check': jsdocEndpointCheck
   }
 }

validator/rules/no-variants-on-responses.js

@@ -0,0 +1,66 @@
+/*
+ * 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';
+
+const createRule = ESLintUtils.RuleCreator(name => `https://example.com/rule/${name}`)
+
+export default createRule({
+  name: 'no-variants-on-responses',
+  create(context) {
+    return {
+      ClassDeclaration(node) {
+        const className = node.id?.name;
+        if (className !== 'Response' && className !== 'Request') {
+          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) {
+          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.'
+            }
+          });
+        }
+      },
+    }
+  },
+  meta: {
+    docs: {
+      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}}'
+    },
+    type: 'problem',
+    schema: []
+  },
+  defaultOptions: []
+})
+

validator/test/no-variants-on-responses.test.js

@@ -0,0 +1,97 @@
+/*
+ * 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/no-variants-on-responses.js'
+
+const ruleTester = new RuleTester({
+  languageOptions: {
+    parserOptions: {
+      projectService: {
+        allowDefaultProject: ['*.ts*'],
+      },
+      tsconfigRootDir: import.meta.dirname,
+    },
+  },
+})
+
+ruleTester.run('no-variants-on-responses', rule, {
+  valid: [
+    `export class Response {
+      /** @codegen_name result */
+      body: ResponseBody
+    }
+    
+    /** @variants container */
+    export class ResponseBody {
+      classification?: ClassificationSummary
+      regression?: RegressionSummary
+    }`,
+    
+    `export class Request {
+      path_parts: {}
+      query_parameters: {}
+      body: RequestBody
+    }
+    
+    /** @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[]
+      }
+    }`,
+  ],
+  invalid: [
+    {
+      code: `/** @variants container */
+      export class Response {
+        body: {
+          classification?: ClassificationSummary
+          regression?: RegressionSummary
+        }
+      }`,
+      errors: [{ messageId: 'noVariantsOnResponses' }]
+    },
+    {
+      code: `/** @variants internal tag='type' */
+      export class Request {
+        path_parts: {}
+        query_parameters: {}
+        body: SomeType
+      }`,
+      errors: [{ messageId: 'noVariantsOnResponses' }]
+    },
+    {
+      code: `/** @variants container */
+      export class Response {
+        body: ResponseData
+      }`,
+      errors: [{ messageId: 'noVariantsOnResponses' }]
+    },
+  ],
+})
+