[eslint] limit access to this in property initializers (#119227)

packages/elastic-eslint-config-kibana/.eslintrc.js

@@ -105,5 +105,6 @@ module.exports = {
     '@kbn/eslint/no_async_foreach': 'error',
     '@kbn/eslint/no_trailing_import_slash': 'error',
     '@kbn/eslint/no_constructor_args_in_property_initializers': 'error',
+    '@kbn/eslint/no_this_in_property_initializers': 'error',
   },
 };

packages/kbn-eslint-plugin-eslint/index.js

@@ -17,5 +17,6 @@ module.exports = {
     no_async_foreach: require('./rules/no_async_foreach'),
     no_trailing_import_slash: require('./rules/no_trailing_import_slash'),
     no_constructor_args_in_property_initializers: require('./rules/no_constructor_args_in_property_initializers'),
+    no_this_in_property_initializers: require('./rules/no_this_in_property_initializers'),
   },
 };

packages/kbn-eslint-plugin-eslint/rules/no_constructor_args_in_property_initializers.test.js

@@ -53,7 +53,6 @@ ruleTester.run('@kbn/eslint/no_constructor_args_in_property_initializers', rule,
   ],
 
   invalid: [
-    // no catch
     {
       code: dedent`
         class Foo {

packages/kbn-eslint-plugin-eslint/rules/no_this_in_property_initializers.js

@@ -0,0 +1,62 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+const tsEstree = require('@typescript-eslint/typescript-estree');
+const traverse = require('eslint-traverse');
+const esTypes = tsEstree.AST_NODE_TYPES;
+
+/** @typedef {import("eslint").Rule.RuleModule} Rule */
+/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.Node} Node */
+/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.ClassBody} ClassBody */
+
+/**
+ * @param {string} arg
+ */
+const ERROR_MSG = `"this" is not fully initialized in class property intializers, define the value for this property in the constructor instead`;
+
+/** @type {Rule} */
+module.exports = {
+  meta: {
+    schema: [],
+  },
+  create: (context) => ({
+    ClassBody(_) {
+      const node = /** @type {ClassBody} */ (_);
+
+      for (const prop of node.body) {
+        if (prop.type !== esTypes.PropertyDefinition) {
+          continue;
+        }
+
+        const visitor = (path) => {
+          /** @type {Node} node */
+          const node = path.node;
+
+          if (
+            node.type === esTypes.FunctionExpression ||
+            node.type === esTypes.ArrowFunctionExpression
+          ) {
+            return traverse.STOP;
+          }
+
+          if (
+            node.type === esTypes.ThisExpression &&
+            node.parent?.type !== esTypes.MemberExpression
+          ) {
+            context.report({
+              message: ERROR_MSG,
+              loc: node.loc,
+            });
+          }
+        };
+
+        traverse(context, prop, visitor);
+      }
+    },
+  }),
+};

packages/kbn-eslint-plugin-eslint/rules/no_this_in_property_initializers.test.js

@@ -0,0 +1,79 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+const { RuleTester } = require('eslint');
+const rule = require('./no_this_in_property_initializers');
+const dedent = require('dedent');
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('@typescript-eslint/parser'),
+  parserOptions: {
+    sourceType: 'module',
+    ecmaVersion: 2018,
+    ecmaFeatures: {
+      jsx: true,
+    },
+  },
+});
+
+ruleTester.run('@kbn/eslint/no_this_in_property_initializers', rule, {
+  valid: [
+    {
+      code: dedent`
+        class Foo {
+          bar = 'baz'
+        }
+      `,
+    },
+    {
+      code: dedent`
+        class Foo {
+          bar = 'baz'
+          constructor(private readonly foo: Box) {}
+        }
+      `,
+    },
+    {
+      code: dedent`
+        class Foo {
+          bar = 'baz'
+          constructor(private readonly foo: () => void) {}
+
+          get = () => {
+            return this.foo()
+          }
+        }
+      `,
+    },
+    {
+      code: dedent`
+        class Foo {
+          bar = this.foo.split().reverse()
+          constructor(private readonly foo: string) {}
+        }
+      `,
+    },
+  ],
+
+  invalid: [
+    {
+      code: dedent`
+        class Foo {
+          bar = new Bar(this)
+          constructor(private readonly foo: (() => void) = defaultValue) {}
+        }
+      `,
+      errors: [
+        {
+          line: 2,
+          message: `"this" is not fully initialized in class property intializers, define the value for this property in the constructor instead`,
+        },
+      ],
+    },
+  ],
+});

src/plugins/advanced_settings/public/component_registry/component_registry.ts

@@ -34,47 +34,43 @@ export class ComponentRegistry {
 
   registry: { [key in Id]?: RegistryComponent } = {};
 
-  /**
-   * Attempts to register the provided component, with the ability to optionally allow
-   * the component to override an existing one.
-   *
-   * If the intent is to override, then `allowOverride` must be set to true, otherwise an exception is thrown.
-   *
-   * @param {*} id the id of the component to register
-   * @param {*} component the component
-   * @param {*} allowOverride (default: false) - optional flag to allow this component to override a previously registered component
-   */
-  private register(id: Id, component: RegistryComponent, allowOverride = false) {
-    if (!allowOverride && id in this.registry) {
-      throw new Error(`Component with id ${id} is already registered.`);
-    }
-
-    // Setting a display name if one does not already exist.
-    // This enhances the snapshots, as well as the debugging experience.
-    if (!component.displayName) {
-      component.displayName = id;
-    }
-
-    this.registry[id] = component;
-  }
-
-  /**
-   * Retrieve a registered component by its ID.
-   * If the component does not exist, then an exception is thrown.
-   *
-   * @param {*} id the ID of the component to retrieve
-   */
-  private get(id: Id): RegistryComponent {
-    return this.registry[id] || ComponentRegistry.defaultRegistry[id];
-  }
-
   setup = {
     componentType: ComponentRegistry.componentType,
-    register: this.register.bind(this),
+    /**
+     * Attempts to register the provided component, with the ability to optionally allow
+     * the component to override an existing one.
+     *
+     * If the intent is to override, then `allowOverride` must be set to true, otherwise an exception is thrown.
+     *
+     * @param id the id of the component to register
+     * @param component the component
+     * @param allowOverride (default: false) - optional flag to allow this component to override a previously registered component
+     */
+    register: (id: Id, component: RegistryComponent, allowOverride = false) => {
+      if (!allowOverride && id in this.registry) {
+        throw new Error(`Component with id ${id} is already registered.`);
+      }
+
+      // Setting a display name if one does not already exist.
+      // This enhances the snapshots, as well as the debugging experience.
+      if (!component.displayName) {
+        component.displayName = id;
+      }
+
+      this.registry[id] = component;
+    },
   };
 
   start = {
     componentType: ComponentRegistry.componentType,
-    get: this.get.bind(this),
+    /**
+     * Retrieve a registered component by its ID.
+     * If the component does not exist, then an exception is thrown.
+     *
+     * @param id the ID of the component to retrieve
+     */
+    get: (id: Id): RegistryComponent => {
+      return this.registry[id] || ComponentRegistry.defaultRegistry[id];
+    },
   };
 }

src/plugins/expressions/common/execution/execution.ts

@@ -196,8 +196,7 @@ export class Execution<
    * Contract is a public representation of `Execution` instances. Contract we
    * can return to other plugins for their consumption.
    */
-  public readonly contract: ExecutionContract<Input, Output, InspectorAdapters> =
-    new ExecutionContract<Input, Output, InspectorAdapters>(this);
+  public readonly contract: ExecutionContract<Input, Output, InspectorAdapters>;
 
   public readonly expression: string;
 
@@ -208,6 +207,8 @@ export class Execution<
   constructor(public readonly execution: ExecutionParams) {
     const { executor } = execution;
 
+    this.contract = new ExecutionContract<Input, Output, InspectorAdapters>(this);
+
     if (!execution.ast && !execution.expression) {
       throw new TypeError('Execution params should contain at least .ast or .expression key.');
     } else if (execution.ast && execution.expression) {

src/plugins/expressions/common/executor/executor.ts

@@ -100,16 +100,18 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
   /**
    * @deprecated
    */
-  public readonly functions = new FunctionsRegistry(this as Executor);
+  public readonly functions: FunctionsRegistry;
 
   /**
    * @deprecated
    */
-  public readonly types = new TypesRegistry(this as Executor);
+  public readonly types: TypesRegistry;
 
   protected parent?: Executor<Context>;
 
   constructor(state?: ExecutorState<Context>) {
+    this.functions = new FunctionsRegistry(this as Executor);
+    this.types = new TypesRegistry(this as Executor);
     this.container = createExecutorContainer<Context>(state);
   }
 

src/plugins/ui_actions/public/triggers/trigger_internal.ts

@@ -15,9 +15,11 @@ import { UiActionsService } from '../service';
  * within `ui_actions` plugin.
  */
 export class TriggerInternal<Context extends object = object> {
-  public readonly contract: TriggerContract<Context> = new TriggerContract<Context>(this);
+  public readonly contract: TriggerContract<Context>;
 
-  constructor(public readonly service: UiActionsService, public readonly trigger: Trigger) {}
+  constructor(public readonly service: UiActionsService, public readonly trigger: Trigger) {
+    this.contract = new TriggerContract<Context>(this);
+  }
 
   public async execute(context: Context, alwaysShowPopup?: boolean) {
     const triggerId = this.trigger.id;

x-pack/test/functional/page_objects/tag_management_page.ts

@@ -245,8 +245,15 @@ export class TagManagementPageObject extends FtrService {
   private readonly header = this.ctx.getPageObject('header');
   private readonly settings = this.ctx.getPageObject('settings');
 
-  public readonly tagModal = new TagModal(this.ctx, this);
-  public readonly assignFlyout = new TagAssignmentFlyout(this.ctx, this);
+  public readonly tagModal: TagModal;
+  public readonly assignFlyout: TagAssignmentFlyout;
+
+  constructor(ctx: FtrProviderContext) {
+    super(ctx);
+
+    this.tagModal = new TagModal(this.ctx, this);
+    this.assignFlyout = new TagAssignmentFlyout(this.ctx, this);
+  }
 
   /**
    * Navigate to the tag management section, by accessing the management app, then clicking