fix: refactors concrete implementations of the procedure data models (#6575)

* fix: expand the IParameterModel interface

* fix: remove support for return types from the concrete procedure model

* feat: add an interface for the procedure map, and add getting procedures

* fix: add procedure map to workspace

* chore: format

* fix: add name parameter to procedure model to match parameter model

* chore: format

* chore: fix comments

core/interfaces/i_parameter_model.ts

@@ -25,6 +25,16 @@ export interface IParameterModel {
    */
   setTypes(types: string[]): this;
 
+  /**
+   * Returns the name of this parameter.
+   */
+  getName(): string;
+
+  /**
+   * Return the types of this parameter.
+   */
+  getTypes(): string[];
+
   /**
    * Returns the unique language-neutral ID for the parameter.
    *

core/interfaces/i_procedure_map.ts

@@ -0,0 +1,20 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import {IProcedureModel} from './i_procedure_model.js';
+
+
+export interface IProcedureMap extends Map<string, IProcedureModel> {
+  /**
+   * Adds the given ProcedureModel to the map of procedure models, so that
+   * blocks can find it.
+   */
+  add(proc: IProcedureModel): this;
+
+
+  /** Returns all of the procedures stored in this map. */
+  getProcedures(): IProcedureModel[];
+}

core/procedures/observable_parameter_model.ts

@@ -48,6 +48,20 @@ export class ObservableParameterModel implements IParameterModel {
         'implement your own custom ParameterModel.');
   }
 
+  /**
+   * Returns the name of this parameter.
+   */
+  getName(): string {
+    return this.variable.name;
+  }
+
+  /**
+   * Returns the types of this parameter.
+   */
+  getTypes(): string[] {
+    return [];
+  }
+
   /**
    * Returns the unique language-neutral ID for the parameter.
    *

core/procedures/observable_procedure_map.ts

@@ -7,9 +7,11 @@
 import type {IProcedureModel} from '../interfaces/i_procedure_model.js';
 import {triggerProceduresUpdate} from './update_procedures.js';
 import type {Workspace} from '../workspace.js';
+import {IProcedureMap} from '../interfaces/i_procedure_map.js';
 
 
-export class ObservableProcedureMap extends Map<string, IProcedureModel> {
+export class ObservableProcedureMap extends
+    Map<string, IProcedureModel> implements IProcedureMap {
   constructor(private readonly workspace: Workspace) {
     super();
   }
@@ -52,4 +54,11 @@ export class ObservableProcedureMap extends Map<string, IProcedureModel> {
     // TODO(#6526): See if this method is actually useful.
     return this.set(proc.getId(), proc);
   }
+
+  /**
+   * Returns all of the procedures stored in this map.
+   */
+  getProcedures(): IProcedureModel[] {
+    return [...this.values()];
+  }
 }

core/procedures/observable_procedure_model.ts

@@ -13,13 +13,15 @@ import type {Workspace} from '../workspace.js';
 
 export class ObservableProcedureModel implements IProcedureModel {
   private id: string;
-  private name = '';
+  private name: string;
   private parameters: IParameterModel[] = [];
   private returnTypes: string[]|null = null;
   private enabled = true;
 
-  constructor(private readonly workspace: Workspace, id?: string) {
+  constructor(
+      private readonly workspace: Workspace, name: string, id?: string) {
     this.id = id ?? genUid();
+    this.name = name;
   }
 
   /** Sets the human-readable name of the procedure. */
@@ -51,13 +53,22 @@ export class ObservableProcedureModel implements IProcedureModel {
   }
 
   /**
-   * Sets the return type(s) of the procedure.
+   * Sets whether the procedure has a return value (empty array) or no return
+   * value (null).
    *
-   * Pass null to represent a procedure that does not return.
+   * The built-in procedure model does not support procedures that have actual
+   * return types (i.e. non-empty arrays, e.g. ['number']). If you want your
+   * procedure block to have return types, you need to implement your own
+   * procedure model.
    */
   setReturnTypes(types: string[]|null): this {
-    // TODO(#6516): Fire events.
+    if (types && types.length) {
+      throw new Error(
+          'The built-in ProcedureModel does not support typing. You need to ' +
+          'implement your own custom ProcedureModel.');
+    }
     this.returnTypes = types;
+    // TODO(#6516): Fire events.
     triggerProceduresUpdate(this.workspace);
     return this;
   }

core/workspace.ts

@@ -32,6 +32,8 @@ import type * as toolbox from './utils/toolbox.js';
 import {VariableMap} from './variable_map.js';
 import type {VariableModel} from './variable_model.js';
 import type {WorkspaceComment} from './workspace_comment.js';
+import {IProcedureMap} from './interfaces/i_procedure_map.js';
+import {ObservableProcedureMap} from './procedures.js';
 
 
 /**
@@ -110,6 +112,7 @@ export class Workspace implements IASTNodeLocation {
   private readonly blockDB = new Map<string, Block>();
   private readonly typedBlocksDB = new Map<string, Block[]>();
   private variableMap: VariableMap;
+  private procedureMap: IProcedureMap = new ObservableProcedureMap(this);
 
   /**
    * Blocks in the flyout can refer to variables that don't exist in the main
@@ -829,6 +832,11 @@ export class Workspace implements IASTNodeLocation {
     this.variableMap = variableMap;
   }
 
+  /** Returns the map of all procedures on the workpace. */
+  getProcedureMap(): IProcedureMap {
+    return this.procedureMap;
+  }
+
   /**
    * Find the workspace with the specified ID.
    *

tests/mocha/procedure_map_test.js

@@ -12,9 +12,7 @@ suite('Procedure Map', function() {
   setup(function() {
     sharedTestSetup.call(this);
     this.workspace = new Blockly.Workspace();
-    // this.procedureMap = this.workspace.getProcedureMap();
-    this.procedureMap =
-        new Blockly.procedures.ObservableProcedureMap(this.workspace);
+    this.procedureMap = this.workspace.getProcedureMap();
   });
 
   teardown(function() {
@@ -40,7 +38,8 @@ suite('Procedure Map', function() {
     suite('procedure map updates', function() {
       test('inserting a procedure does not trigger an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name');
         this.procedureMap.set(procedureModel.getId(), procedureModel);
 
         chai.assert.isFalse(
@@ -49,7 +48,8 @@ suite('Procedure Map', function() {
 
       test('adding a procedure does not trigger an update', function() {
         this.procedureMap.add(
-            new Blockly.procedures.ObservableProcedureModel(this.workspace));
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name'));
 
         chai.assert.isFalse(
             this.updateSpy.called, 'Expected no update to be triggered');
@@ -57,7 +57,8 @@ suite('Procedure Map', function() {
 
       test('deleting a procedure triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name');
         this.procedureMap.add(procedureModel);
 
         this.procedureMap.delete(procedureModel.getId());
@@ -70,7 +71,8 @@ suite('Procedure Map', function() {
     suite('procedure model updates', function() {
       test('setting the name triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name');
         this.procedureMap.add(procedureModel);
 
         procedureModel.setName('new name');
@@ -81,10 +83,11 @@ suite('Procedure Map', function() {
   
       test('setting the return type triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name');
         this.procedureMap.add(procedureModel);
 
-        procedureModel.setReturnTypes(['return type 1', 'return type 2']);
+        procedureModel.setReturnTypes([]);
 
         chai.assert.isTrue(
             this.updateSpy.calledOnce, 'Expected an update to be triggered');
@@ -92,8 +95,9 @@ suite('Procedure Map', function() {
   
       test('removing the return type triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace)
-                .setReturnTypes(['return type']);
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name')
+                .setReturnTypes([]);
         this.procedureMap.add(procedureModel);
         this.updateSpy.resetHistory();
 
@@ -105,7 +109,8 @@ suite('Procedure Map', function() {
   
       test('disabling the procedure triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name');
         this.procedureMap.add(procedureModel);
 
         procedureModel.setEnabled(false);
@@ -116,7 +121,8 @@ suite('Procedure Map', function() {
   
       test('enabling the procedure triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace)
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name')
                 .setEnabled(false);
         this.procedureMap.add(procedureModel);
         this.updateSpy.resetHistory();
@@ -129,7 +135,8 @@ suite('Procedure Map', function() {
 
       test('inserting a parameter triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name');
         this.procedureMap.add(procedureModel);
 
         procedureModel.insertParameter(
@@ -141,7 +148,8 @@ suite('Procedure Map', function() {
   
       test('deleting a parameter triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace)
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name')
                 .insertParameter(
                     new Blockly.procedures.ObservableParameterModel(
                         this.workspace));
@@ -161,7 +169,8 @@ suite('Procedure Map', function() {
             new Blockly.procedures.ObservableParameterModel(
                 this.workspace, 'test1');
         this.procedureMap.add(
-            new Blockly.procedures.ObservableProcedureModel(this.workspace)
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name')
                 .insertParameter(parameterModel));
         this.updateSpy.resetHistory();
 
@@ -176,7 +185,8 @@ suite('Procedure Map', function() {
             new Blockly.procedures.ObservableParameterModel(
                 this.workspace, 'test1');
         this.procedureMap.add(
-            new Blockly.procedures.ObservableProcedureModel(this.workspace)
+            new Blockly.procedures.ObservableProcedureModel(
+                this.workspace, 'test name')
                 .insertParameter(parameterModel));
         this.updateSpy.resetHistory();