chore: Migrate core/ to Typescript, actually (#6299)

* fix: convert files to typescript

* fix: add alias for AnyDuringMigration so that tsc will run

* chore: format

* chore: enable ts for the clang-format workflow (#6233)

* chore: Restore @fileoverview comment locations (#6237)

* chore: add declareModuleId (#6238)

* fix: Revert comment change to app_controller.js (#6241)

* fix: Add missing import goog statements (#6240)

I've added the import statement immediately before the
goog.declareModuleId calls that depend on it.

There is an argument to be made that we should put the import
statement in their normal place amongst any other imports, and
move the declareModuleId statement to below the double blank
line below the imports, but as these are so tightly coupled,
replace the previous goog.module calls, and will both be deleted
at the same time once the transition to TypeScript is fully complete
I think it's fine (and certainly much easier) to do it this way.

* chore: Fix whitespace (#6243)

* fix: Remove spurious blank lines

  Remove extraneous blank lines introduced by deletion of
  'use strict'; pragmas.

  Also fix the location of the goog.declareModuleId call in
  core/utils/array.ts.

* fix: Add missing double-blank-line before body of modules

  Our convention is to have two blank lines between the imports (or
  module ID, if there are no imports) and the beginning of the body
  of the module.  Enforce this.

* fix: one addition format error for PR #6243

* fix(build): Skip npm prepare when running in CI (#6244)

Have npm prepare do nothing when running in CI.

We don't need to do any building, because npm test will build
everything needed in the workflows in which it is run, and we
don't want to build anything in other workflows because a tsc
error would prevent those workflows from completing.

* fix: re-add `@package` annotations as `@internal` annotations (#6232)

* fix: add ~70% of internal attributes

* fix: work on manually adding more @internal annotations

* fix: add more manual internal annotations

* fix: rename package typos to internal

* fix: final manual fixes for internal annotations

* chore: format

* chore: make unnecessary multiline jsdoc a single line

* fix: fix internal tags in serialization exceptions

* fix: tsc errors picked up from develop (#6224)

* fix: relative path for deprecation utils

* fix: checking if properties exist in svg_math

* fix: set all timeout PIDs to AnyDuringMigration

* fix: make nullability errors explicity in block drag surface

* fix: make null check in events_block_change explicit

* fix: make getEventWorkspace_ internal so we can access it from CommentCreateDeleteHelper

* fix: rename DIV -> containerDiv in tooltip

* fix: ignore backwards compat check in category

* fix: set block styles to AnyDuringMigration

* fix: type typo in KeyboardShortcut

* fix: constants name in row measurables

* fix: typecast in mutator

* fix: populateProcedures type of flattened array

* fix: ignore errors related to workspace comment deserialization

* chore: format files

* fix: renaming imports missing file extensions

* fix: remove check for sound.play

* fix: temporarily remove bad requireType.

All `export type` statements are stripped when tsc is run. This means
that when we attempt to require BlockDefinition from the block files, we
get an error because it does not exist.

We decided to temporarily remove the require, because this will no
longer be a problem when we conver the blocks to typescript, and
everything gets compiled together.

* fix: bad jsdoc in array

* fix: silence missing property errors

Closure was complaining about inexistant properties, but they actually
do exist, they're just not being transpiled by tsc in a way that closure
understands.

I.E. if things are initialized in a function called by the constructor,
rather than in a class field or in the custructor itself, closure would
error.

It would also error on enums, because they are transpiled to a weird
IIFE.

* fix: context menu action handler not knowing the type of this.

this: TypeX information gets stripped when tsc is run, so closure could
not know that this was not global. Fixed this by reorganizing to use the
option object directly instead of passing it to onAction to be bound to
this.

* fix: readd getDeveloperVars checks (should not be part of migration)

This was found because ALL_DEVELOPER_VARS_WARNINGS_BY_BLOCK_TYPE was no
longer being accessed.

* fix: silence closure errors about overriding supertype props

We propertly define the overrides in typescript, but these get removed
from the compiled output, so closure doesn't know they exist.

* fix: silence globalThis errors

this: TypeX annotations get stripped from the compiled output, so
closure can't know that we're accessing the correct things. However,
typescript makes sure that this always has the correct properties, so
silencing this should be fine.

* fix: bad jsdoc name

* chore: attempt compiling with blockly.js

* fix: attempt moving the import statement above the namespace line

* chore: add todo comments to block def files

* chore: remove todo from context menu

* chore: add comments abotu disabled errors

* chore: move comments back to their correct positions (#6249)

* fix: work on fixing comments

* chore: finish moving all comments

* chore: format

* chore: move some other messed up comments

* chore: format

* fix: Correct enum formatting, use merged `namespace`s for types that are class static members (#6246)

* fix: formatting of enum KeyCodes

* fix: Use merged namespace for ContextMenuRegistry static types

  - Create a namespace to be merged with the ContextMenuRegistry
    class containing the types that were formerly declared as static
    properties on that class.

  - Use type aliases to export them individually as well, for
    compatibility with the changes made by MigranTS (and/or
    @gonfunko) to how other modules in core/ now import these
    types.

  - Update renamings.json5 to reflect the availability of the
    direct exports for modules that import this module directly
    (though they are not available to, and will not be used by,
    code that imports only via blockly.js/blockly.ts.)

* fix: Use merged namespace for Input.Align

  - Create a merged namespace for the Input.Align enum.

  - Use type/const aliases to export it as Input too.

  - Update renamings.json5 to reflect the availability of the
    direct export.

* fix: Use merged namespace for Names.NameType

  - Create a merged namespace for the Names.NameType enum.

  - Use type/const aliases to export it as NameType too.

  - Update renamings.json5 to reflect the availability of the
    direct export.  (This ought to have happened in an earlier
    version as it was already available by both routes.)

* chore: Fix minor issues for PR #6246

  - Use `Align` instead of `Input.Align` where possible.

* fix(build): Suppress irrelevant JSC_UNUSED_LOCAL_ASSIGNMENT errors

  tsc generates code for merged namespaces that looks like:

      (function (ClassName) {
          let EnumName;
          (function (EnumName) {
              EnumName[EnumNameAlign["v1"] = 0] = "v1";
              // etc.
          })(EnumName = ClassName.EnumName || (ClassName.EnumName = {}));
      })(ClassName || (ClassName = {}));

  and Closure Compiler complains about the fact that the EnumName let
  binding is initialised but never used.  (It exists so that any other
  code that was in the namespace could see the enum.)

  Suppress this message, since it is not actionable and lint and/or tsc
  should tell us if we have actual unused variables in our .ts files.

* chore(build): Suppress spurious warnings from closure-make-deps (#6253)

A little bit of an ugly hack, but it works: pipe stderr through
grep -v to suppress error output starting with "WARNING in".

* fix: remaining enums that weren't properly exported (#6251)

* fix: remaining enums that weren't properly exported

* chore: format

* fix: add enum value exports

* chore: format

* fix: properly export interfaces that were typedefs (#6250)

* fix: properly export interfaces that were typedefs

* fix: allowCollsion -> allowCollision

* fix: convert unconverted enums

* fix: enums that were/are instance properties

* fix: revert changes to property enums

* fix: renamed protected parameter properties (#6252)

* fix: bad protected parameter properties

* chore:format

* fix: gesture constructor

* fix: overridden properties that were renamed

* refactor: Migrate `blockly.js` to TypeScript (#6261)

* chore: Apply changes to blockly.js to blockly.ts

* fix: Build using core/blockly.ts instead of .js

  Compiles and runs in compressed mode correctly!

* fix(build): Don't depend on execSync running bash (#6262)

For some reason on Github CI servers execSync uses /bin/sh, which
is (on Ubuntu) dash rather than bash, and does not understand
the pipefail option.

So remove the grep pipe on stderr and just discard all error output
at all.

This is not ideal as errors in test deps will go unreported AND
not even cause test failure, but it's not clear that it's worth
investing more time to fix this at the moment.

* chore: use `import type` where possible (#6279)

* chore: automatically change imports to import types

* chore: revert changes that actually need to be imports

* chore: format

* chore: add more import type statements based on importsNotUsedAsValues

* chore: fix tsconfig

* chore: add link to compiler issue

* fix: add type information to blockly options (#6283)

* fix: add type information to blockly options

* chore: format

* chore: remove erroneous comment

* fix: bugs revealed by getting the built output working (#6282)

* fix: types of compose and decompose in block

* fix: workspace naming in toolbox

* chore: add jsdoc

* chore: restore registry comments to better positions

* chore: pr comments'

* fix(variables): Revert inadvertent change to allDeveloperVariables (#6290)

It appears that a function call got modified incorrectly (probably
in an effort to fix a typing issue).  This fix trivially reverts
the line in question to match the original JS version from develop.

This causes the generator tests to pass.

* fix: circular dependencies (#6281)

* chore: fix circular dependencies w/ static workspace funcs

* remove preserved imports that aren't currently necessary (probably)

* fix circular dependency with workspaces and block using stub

* fix dependency between variables and xml by moving function to utils

* add stub for trashcan as well

* fix line endings from rebase

* fix goog/base order

* add trashcan patch

* fix: types of compose and decompose in block

* fix: workspace naming in toolbox

* chore: add jsdoc

* chore: restore registry comments to better positions

* chore: remove implementations in goog.js

* chore: fix types of stubs

* chore: remove added AnyDuringMigration casts

* chore: remove modifications to xml and variables

* chore: format

* chore: remove event requirements in workspace comments

* chore: fix circular dependency with xml and workspace comments

* fixup remove ContextMenu import

* chore: fix dependency between mutator and workspace

* chore: break circular dependency between names and procedures

* chore: get tests to run?

* chore: pr comments'

* chore: fix stubbing field registry fromJson

* chore: fix spying on fire

* chore: fix stubbing parts of connection checker

* chore: fix stubbing dialog

* chore: fix stubbing style

* chore: fix spying on duplicate

* chore: fix stubbing variables

* chore: fix stubbing copy

* chore: fix stubbing in workspace

* chore: remove unnecessary stubs

* chore: fix formatting

* chore: fix other formatting

* chore: add backwards compatible static properties to workspace

* chore: move static type properties

* chore: move and comment stubs

* chore: add newlines at EOF

* chore: improve errors for monkey patched functions

* chore: update comment with a pointer to the doc

* chore: update comment with a pointer to the doc

* chore: format

* chore: revert changes to playground used for testing (#6292)

* chore: get mocha tests to pass. (#6291)

* chore: fix undo and empty code blocks

* chore: skip IE test

* chore: fix gesture test

* chore: fix replace message references test

* chore: fix string table interpolation

* chore: skip getById tests

* chore: fix field tests

* chore: fix console errors by making workspace nullable

* chore: format

* chore: fix definition overwrite warning

* chore: update metadata

* chore: temporarily modify the the advanced compile test

* chore: fix gestures by fixing test instead

Co-authored-by: Neil Fraser <fraser@google.com>
Co-authored-by: Christopher Allen <cpcallen+git@google.com>

core/field_variable.ts

@@ -7,135 +7,123 @@
 /**
  * @fileoverview Variable input field.
  */
-'use strict';
 
 /**
  * Variable input field.
  * @class
  */
-goog.module('Blockly.FieldVariable');
+import * as goog from '../closure/goog/goog.js';
+goog.declareModuleId('Blockly.FieldVariable');
 
-const Variables = goog.require('Blockly.Variables');
-const Xml = goog.require('Blockly.Xml');
-const fieldRegistry = goog.require('Blockly.fieldRegistry');
-const internalConstants = goog.require('Blockly.internalConstants');
-const parsing = goog.require('Blockly.utils.parsing');
-/* eslint-disable-next-line no-unused-vars */
-const {Block} = goog.requireType('Blockly.Block');
-const {Field} = goog.require('Blockly.Field');
-const {FieldDropdown} = goog.require('Blockly.FieldDropdown');
-/* eslint-disable-next-line no-unused-vars */
-const {MenuItem} = goog.requireType('Blockly.MenuItem');
-/* eslint-disable-next-line no-unused-vars */
-const {Menu} = goog.requireType('Blockly.Menu');
-const {Msg} = goog.require('Blockly.Msg');
-/* eslint-disable-next-line no-unused-vars */
-const {Sentinel} = goog.requireType('Blockly.utils.Sentinel');
-const {Size} = goog.require('Blockly.utils.Size');
-const {VariableModel} = goog.require('Blockly.VariableModel');
-/** @suppress {extraRequire} */
-goog.require('Blockly.Events.BlockChange');
+// Unused import preserved for side-effects. Remove if unneeded.
+import './events/events_block_change.js';
+
+import type {Block} from './block.js';
+import {Field} from './field.js';
+import {FieldDropdown} from './field_dropdown.js';
+import * as fieldRegistry from './field_registry.js';
+import * as internalConstants from './internal_constants.js';
+import type {Menu} from './menu.js';
+import type {MenuItem} from './menuitem.js';
+import {Msg} from './msg.js';
+import * as parsing from './utils/parsing.js';
+import type {Sentinel} from './utils/sentinel.js';
+import {Size} from './utils/size.js';
+import {VariableModel} from './variable_model.js';
+import * as Variables from './variables.js';
+import * as Xml from './xml.js';
 
 
 /**
  * Class for a variable's dropdown field.
- * @extends {FieldDropdown}
  * @alias Blockly.FieldVariable
  */
-class FieldVariable extends FieldDropdown {
+export class FieldVariable extends FieldDropdown {
+  protected override menuGenerator_: AnyDuringMigration[][]|
+      ((this: FieldDropdown) => AnyDuringMigration[][]);
+  defaultVariableName: string;
+
+  /** The type of the default variable for this field. */
+  private defaultType_ = '';
+
   /**
-   * @param {?string|!Sentinel} varName The default name for the variable.
+   * All of the types of variables that will be available in this field's
+   * dropdown.
+   */
+  variableTypes: string[]|null = [];
+  protected override size_: Size;
+
+  /** The variable model associated with this field. */
+  private variable_: VariableModel|null = null;
+
+  /**
+   * Serializable fields are saved by the serializer, non-serializable fields
+   * are not. Editable fields should also be serializable.
+   */
+  override SERIALIZABLE = true;
+
+  /**
+   * @param varName The default name for the variable.
    *     If null, a unique variable name will be generated.
    *     Also accepts Field.SKIP_SETUP if you wish to skip setup (only used by
-   *     subclasses that want to handle configuration and setting the field
-   *     value after their own constructors have run).
-   * @param {Function=} opt_validator A function that is called to validate
-   *    changes to the field's value. Takes in a variable ID  & returns a
-   *    validated variable ID, or null to abort the change.
-   * @param {Array<string>=} opt_variableTypes A list of the types of variables
-   *     to include in the dropdown. Will only be used if opt_config is not
-   *     provided.
-   * @param {string=} opt_defaultType The type of variable to create if this
-   *     field's value is not explicitly set.  Defaults to ''. Will only be used
-   *     if opt_config is not provided.
-   * @param {Object=} opt_config A map of options used to configure the field.
+   * subclasses that want to handle configuration and setting the field value
+   * after their own constructors have run).
+   * @param opt_validator A function that is called to validate changes to the
+   *     field's value. Takes in a variable ID  & returns a validated variable
+   *     ID, or null to abort the change.
+   * @param opt_variableTypes A list of the types of variables to include in the
+   *     dropdown. Will only be used if opt_config is not provided.
+   * @param opt_defaultType The type of variable to create if this field's value
+   *     is not explicitly set.  Defaults to ''. Will only be used if opt_config
+   *     is not provided.
+   * @param opt_config A map of options used to configure the field.
    *    See the [field creation documentation]{@link
-   *    https://developers.google.com/blockly/guides/create-custom-blocks/fields/built-in-fields/variable#creation}
-   *    for a list of properties this parameter supports.
+   * https://developers.google.com/blockly/guides/create-custom-blocks/fields/built-in-fields/variable#creation}
+   * for a list of properties this parameter supports.
    */
   constructor(
-      varName, opt_validator, opt_variableTypes, opt_defaultType, opt_config) {
+      varName: string|null|Sentinel, opt_validator?: Function,
+      opt_variableTypes?: string[], opt_defaultType?: string,
+      opt_config?: AnyDuringMigration) {
     super(Field.SKIP_SETUP);
 
     /**
      * An array of options for a dropdown list,
      * or a function which generates these options.
-     * @type {(!Array<!Array>|
-     *    !function(this:FieldDropdown): !Array<!Array>)}
-     * @protected
      */
-    this.menuGenerator_ = FieldVariable.dropdownCreate;
+    // AnyDuringMigration because:  Type '(this: FieldVariable) => any[][]' is
+    // not assignable to type 'any[][] | ((this: FieldDropdown) => any[][])'.
+    this.menuGenerator_ = FieldVariable.dropdownCreate as AnyDuringMigration;
 
     /**
      * The initial variable name passed to this field's constructor, or an
      * empty string if a name wasn't provided. Used to create the initial
      * variable.
-     * @type {string}
      */
     this.defaultVariableName = typeof varName === 'string' ? varName : '';
 
-    /**
-     * The type of the default variable for this field.
-     * @type {string}
-     * @private
-     */
-    this.defaultType_ = '';
-
-    /**
-     * All of the types of variables that will be available in this field's
-     * dropdown.
-     * @type {?Array<string>}
-     */
-    this.variableTypes = [];
-
-    /**
-     * The size of the area rendered by the field.
-     * @type {Size}
-     * @protected
-     * @override
-     */
+    /** The size of the area rendered by the field. */
     this.size_ = new Size(0, 0);
 
-    /**
-     * The variable model associated with this field.
-     * @type {?VariableModel}
-     * @private
-     */
-    this.variable_ = null;
-
-    /**
-     * Serializable fields are saved by the serializer, non-serializable fields
-     * are not. Editable fields should also be serializable.
-     * @type {boolean}
-     */
-    this.SERIALIZABLE = true;
-
-    if (varName === Field.SKIP_SETUP) return;
+    if (varName === Field.SKIP_SETUP) {
+      return;
+    }
 
     if (opt_config) {
       this.configure_(opt_config);
     } else {
       this.setTypes_(opt_variableTypes, opt_defaultType);
     }
-    if (opt_validator) this.setValidator(opt_validator);
+    if (opt_validator) {
+      this.setValidator(opt_validator);
+    }
   }
 
   /**
    * Configure the field based on the given map of options.
-   * @param {!Object} config A map of options to configure the field based on.
-   * @protected
+   * @param config A map of options to configure the field based on.
    */
-  configure_(config) {
+  protected override configure_(config: AnyDuringMigration) {
     super.configure_(config);
     this.setTypes_(config['variableTypes'], config['defaultType']);
   }
@@ -144,35 +132,31 @@ class FieldVariable extends FieldDropdown {
    * Initialize the model for this field if it has not already been initialized.
    * If the value has not been set to a variable by the first render, we make up
    * a variable rather than let the value be invalid.
-   * @package
+   * @internal
    */
-  initModel() {
+  override initModel() {
     if (this.variable_) {
       return;  // Initialization already happened.
     }
     const variable = Variables.getOrCreateVariablePackage(
-        this.sourceBlock_.workspace, null, this.defaultVariableName,
+        this.sourceBlock_.workspace!, null, this.defaultVariableName,
         this.defaultType_);
-
     // Don't call setValue because we don't want to cause a rerender.
     this.doValueUpdate_(variable.getId());
   }
 
-  /**
-   * @override
-   */
-  shouldAddBorderRect_() {
+  override shouldAddBorderRect_() {
     return super.shouldAddBorderRect_() &&
-        (!this.getConstants().FIELD_DROPDOWN_NO_BORDER_RECT_SHADOW ||
+        (!this.getConstants()!.FIELD_DROPDOWN_NO_BORDER_RECT_SHADOW ||
          this.sourceBlock_.type !== 'variables_get');
   }
 
   /**
    * Initialize this field based on the given XML.
-   * @param {!Element} fieldElement The element containing information about the
-   *    variable field's state.
+   * @param fieldElement The element containing information about the variable
+   *     field's state.
    */
-  fromXml(fieldElement) {
+  override fromXml(fieldElement: Element) {
     const id = fieldElement.getAttribute('id');
     const variableName = fieldElement.textContent;
     // 'variabletype' should be lowercase, but until July 2019 it was sometimes
@@ -180,8 +164,11 @@ class FieldVariable extends FieldDropdown {
     const variableType = fieldElement.getAttribute('variabletype') ||
         fieldElement.getAttribute('variableType') || '';
 
+    // AnyDuringMigration because:  Argument of type 'string | null' is not
+    // assignable to parameter of type 'string | undefined'.
     const variable = Variables.getOrCreateVariablePackage(
-        this.sourceBlock_.workspace, id, variableName, variableType);
+        this.sourceBlock_.workspace!, id, variableName as AnyDuringMigration,
+        variableType);
 
     // This should never happen :)
     if (variableType !== null && variableType !== variable.type) {
@@ -197,69 +184,66 @@ class FieldVariable extends FieldDropdown {
 
   /**
    * Serialize this field to XML.
-   * @param {!Element} fieldElement The element to populate with info about the
-   *    field's state.
-   * @return {!Element} The element containing info about the field's state.
+   * @param fieldElement The element to populate with info about the field's
+   *     state.
+   * @return The element containing info about the field's state.
    */
-  toXml(fieldElement) {
+  override toXml(fieldElement: Element): Element {
     // Make sure the variable is initialized.
     this.initModel();
 
-    fieldElement.id = this.variable_.getId();
-    fieldElement.textContent = this.variable_.name;
-    if (this.variable_.type) {
-      fieldElement.setAttribute('variabletype', this.variable_.type);
+    fieldElement.id = this.variable_!.getId();
+    fieldElement.textContent = this.variable_!.name;
+    if (this.variable_!.type) {
+      fieldElement.setAttribute('variabletype', this.variable_!.type);
     }
     return fieldElement;
   }
 
   /**
    * Saves this field's value.
-   * @param {boolean=} doFullSerialization If true, the variable field will
-   *     serialize the full state of the field being referenced (ie ID, name,
-   *     and type) rather than just a reference to it (ie ID).
-   * @return {*} The state of the variable field.
-   * @override
-   * @package
+   * @param doFullSerialization If true, the variable field will serialize the
+   *     full state of the field being referenced (ie ID, name, and type) rather
+   *     than just a reference to it (ie ID).
+   * @return The state of the variable field.
+   * @internal
    */
-  saveState(doFullSerialization) {
+  override saveState(doFullSerialization?: boolean): AnyDuringMigration {
     const legacyState = this.saveLegacyState(FieldVariable);
     if (legacyState !== null) {
       return legacyState;
     }
     // Make sure the variable is initialized.
     this.initModel();
-    const state = {'id': this.variable_.getId()};
+    const state = {'id': this.variable_!.getId()};
     if (doFullSerialization) {
-      state['name'] = this.variable_.name;
-      state['type'] = this.variable_.type;
+      (state as AnyDuringMigration)['name'] = this.variable_!.name;
+      (state as AnyDuringMigration)['type'] = this.variable_!.type;
     }
     return state;
   }
 
   /**
    * Sets the field's value based on the given state.
-   * @param {*} state The state of the variable to assign to this variable
-   *     field.
-   * @override
-   * @package
+   * @param state The state of the variable to assign to this variable field.
+   * @internal
    */
-  loadState(state) {
+  override loadState(state: AnyDuringMigration) {
     if (this.loadLegacyState(FieldVariable, state)) {
       return;
     }
     // This is necessary so that blocks in the flyout can have custom var names.
     const variable = Variables.getOrCreateVariablePackage(
-        this.sourceBlock_.workspace, state['id'] || null, state['name'],
+        this.sourceBlock_.workspace!, state['id'] || null, state['name'],
         state['type'] || '');
     this.setValue(variable.getId());
   }
 
   /**
    * Attach this field to a block.
-   * @param {!Block} block The block containing this field.
+   * @param block The block containing this field.
    */
-  setSourceBlock(block) {
+  override setSourceBlock(block: Block) {
     if (block.isShadow()) {
       throw Error('Variable fields are not allowed to exist on shadow blocks.');
     }
@@ -268,18 +252,18 @@ class FieldVariable extends FieldDropdown {
 
   /**
    * Get the variable's ID.
-   * @return {?string} Current variable's ID.
+   * @return Current variable's ID.
    */
-  getValue() {
+  override getValue(): string|null {
     return this.variable_ ? this.variable_.getId() : null;
   }
 
   /**
    * Get the text from this field, which is the selected variable's name.
-   * @return {string} The selected variable's name, or the empty string if no
-   *     variable is selected.
+   * @return The selected variable's name, or the empty string if no variable is
+   *     selected.
    */
-  getText() {
+  override getText(): string {
     return this.variable_ ? this.variable_.name : '';
   }
 
@@ -287,11 +271,10 @@ class FieldVariable extends FieldDropdown {
    * Get the variable model for the selected variable.
    * Not guaranteed to be in the variable map on the workspace (e.g. if accessed
    * after the variable has been deleted).
-   * @return {?VariableModel} The selected variable, or null if none was
-   *     selected.
-   * @package
+   * @return The selected variable, or null if none was selected.
+   * @internal
    */
-  getVariable() {
+  getVariable(): VariableModel|null {
     return this.variable_;
   }
 
@@ -300,9 +283,9 @@ class FieldVariable extends FieldDropdown {
    * Returns null if the variable is not set, because validators should not
    * run on the initial setValue call, because the field won't be attached to
    * a block and workspace at that point.
-   * @return {?Function} Validation function, or null.
+   * @return Validation function, or null.
    */
-  getValidator() {
+  override getValidator(): Function|null {
     // Validators shouldn't operate on the initial setValue call.
     // Normally this is achieved by calling setValidator after setValue, but
     // this is not a possibility with variable fields.
@@ -314,16 +297,16 @@ class FieldVariable extends FieldDropdown {
 
   /**
    * Ensure that the ID belongs to a valid variable of an allowed type.
-   * @param {*=} opt_newValue The ID of the new variable to set.
-   * @return {?string} The validated ID, or null if invalid.
-   * @protected
+   * @param opt_newValue The ID of the new variable to set.
+   * @return The validated ID, or null if invalid.
    */
-  doClassValidation_(opt_newValue) {
+  protected override doClassValidation_(opt_newValue?: AnyDuringMigration):
+      string|null {
     if (opt_newValue === null) {
       return null;
     }
-    const newId = /** @type {string} */ (opt_newValue);
-    const variable = Variables.getVariable(this.sourceBlock_.workspace, newId);
+    const newId = opt_newValue as string;
+    const variable = Variables.getVariable(this.sourceBlock_.workspace!, newId);
     if (!variable) {
       console.warn(
           'Variable id doesn\'t point to a real variable! ' +
@@ -345,22 +328,20 @@ class FieldVariable extends FieldDropdown {
    *
    * The variable ID should be valid at this point, but if a variable field
    * validator returns a bad ID, this could break.
-   * @param {*} newId The value to be saved.
-   * @protected
+   * @param newId The value to be saved.
    */
-  doValueUpdate_(newId) {
-    this.variable_ = Variables.getVariable(
-        this.sourceBlock_.workspace, /** @type {string} */ (newId));
+  protected override doValueUpdate_(newId: AnyDuringMigration) {
+    this.variable_ =
+        Variables.getVariable(this.sourceBlock_.workspace!, newId as string);
     super.doValueUpdate_(newId);
   }
 
   /**
    * Check whether the given variable type is allowed on this field.
-   * @param {string} type The type to check.
-   * @return {boolean} True if the type is in the list of allowed types.
-   * @private
+   * @param type The type to check.
+   * @return True if the type is in the list of allowed types.
    */
-  typeIsAllowed_(type) {
+  private typeIsAllowed_(type: string): boolean {
     const typeList = this.getVariableTypes_();
     if (!typeList) {
       return true;  // If it's null, all types are valid.
@@ -375,11 +356,10 @@ class FieldVariable extends FieldDropdown {
 
   /**
    * Return a list of variable types to include in the dropdown.
-   * @return {!Array<string>} Array of variable types.
+   * @return Array of variable types.
    * @throws {Error} if variableTypes is an empty array.
-   * @private
    */
-  getVariableTypes_() {
+  private getVariableTypes_(): string[] {
     // TODO (#1513): Try to avoid calling this every time the field is edited.
     let variableTypes = this.variableTypes;
     if (variableTypes === null) {
@@ -401,14 +381,13 @@ class FieldVariable extends FieldDropdown {
   /**
    * Parse the optional arguments representing the allowed variable types and
    * the default variable type.
-   * @param {Array<string>=} opt_variableTypes A list of the types of variables
-   *     to include in the dropdown.  If null or undefined, variables of all
-   * types will be displayed in the dropdown.
-   * @param {string=} opt_defaultType The type of the variable to create if this
-   *     field's value is not explicitly set.  Defaults to ''.
-   * @private
+   * @param opt_variableTypes A list of the types of variables to include in the
+   *     dropdown.  If null or undefined, variables of all types will be
+   *     displayed in the dropdown.
+   * @param opt_defaultType The type of the variable to create if this field's
+   *     value is not explicitly set.  Defaults to ''.
    */
-  setTypes_(opt_variableTypes, opt_defaultType) {
+  private setTypes_(opt_variableTypes?: string[], opt_defaultType?: string) {
     // If you expected that the default type would be the same as the only entry
     // in the variable types array, tell the Blockly team by commenting on
     // #1499.
@@ -445,10 +424,9 @@ class FieldVariable extends FieldDropdown {
    * Refreshes the name of the variable by grabbing the name of the model.
    * Used when a variable gets renamed, but the ID stays the same. Should only
    * be called by the block.
-   * @override
-   * @package
+   * @internal
    */
-  refreshVariableName() {
+  override refreshVariableName() {
     this.forceRerender();
   }
 
@@ -456,23 +434,21 @@ class FieldVariable extends FieldDropdown {
    * Handle the selection of an item in the variable dropdown menu.
    * Special case the 'Rename variable...' and 'Delete variable...' options.
    * In the rename case, prompt the user for a new name.
-   * @param {!Menu} menu The Menu component clicked.
-   * @param {!MenuItem} menuItem The MenuItem selected within menu.
-   * @protected
+   * @param menu The Menu component clicked.
+   * @param menuItem The MenuItem selected within menu.
    */
-  onItemSelected_(menu, menuItem) {
+  protected override onItemSelected_(menu: Menu, menuItem: MenuItem) {
     const id = menuItem.getValue();
     // Handle special cases.
     if (this.sourceBlock_ && this.sourceBlock_.workspace) {
       if (id === internalConstants.RENAME_VARIABLE_ID) {
         // Rename variable.
         Variables.renameVariable(
-            this.sourceBlock_.workspace,
-            /** @type {!VariableModel} */ (this.variable_));
+            this.sourceBlock_.workspace, this.variable_ as VariableModel);
         return;
       } else if (id === internalConstants.DELETE_VARIABLE_ID) {
         // Delete variable.
-        this.sourceBlock_.workspace.deleteVariableById(this.variable_.getId());
+        this.sourceBlock_.workspace.deleteVariableById(this.variable_!.getId());
         return;
       }
     }
@@ -483,25 +459,23 @@ class FieldVariable extends FieldDropdown {
   /**
    * Overrides referencesVariables(), indicating this field refers to a
    * variable.
-   * @return {boolean} True.
-   * @package
-   * @override
+   * @return True.
+   * @internal
    */
-  referencesVariables() {
+  override referencesVariables(): boolean {
     return true;
   }
 
   /**
    * Construct a FieldVariable from a JSON arg object,
    * dereferencing any string table references.
-   * @param {!Object} options A JSON object with options (variable,
-   *                          variableTypes, and defaultType).
-   * @return {!FieldVariable} The new field instance.
-   * @package
+   * @param options A JSON object with options (variable, variableTypes, and
+   *     defaultType).
+   * @return The new field instance.
    * @nocollapse
-   * @override
+   * @internal
    */
-  static fromJson(options) {
+  static override fromJson(options: AnyDuringMigration): FieldVariable {
     const varName = parsing.replaceMessageReferences(options['variable']);
     // `this` might be a subclass of FieldVariable if that class doesn't
     // override the static fromJson method.
@@ -511,17 +485,16 @@ class FieldVariable extends FieldDropdown {
   /**
    * Return a sorted list of variable names for variable dropdown menus.
    * Include a special option at the end for creating a new variable name.
-   * @return {!Array<!Array>} Array of variable names/id tuples.
-   * @this {FieldVariable}
+   * @return Array of variable names/id tuples.
    */
-  static dropdownCreate() {
+  static dropdownCreate(this: FieldVariable): AnyDuringMigration[][] {
     if (!this.variable_) {
       throw Error(
           'Tried to call dropdownCreate on a variable field with no' +
           ' variable selected.');
     }
     const name = this.getText();
-    let variableModelList = [];
+    let variableModelList: AnyDuringMigration[] = [];
     if (this.sourceBlock_ && this.sourceBlock_.workspace) {
       const variableTypes = this.getVariableTypes_();
       // Get a copy of the list, so that adding rename and new variable options
@@ -554,5 +527,3 @@ class FieldVariable extends FieldDropdown {
 }
 
 fieldRegistry.register('field_variable', FieldVariable);
-
-exports.FieldVariable = FieldVariable;