fix: project cereal cleanup (google#5398)

* fix: make exception constructors package

* fix: rename blocks.load to blocks.append

* fix: inline docs

* fix: consistency in block serialization

* fix: remove unexported functions

* fix: interface requires

* fix: tag TODO with issue number

core/connection.js

@@ -757,7 +757,7 @@ Connection.prototype.createShadowBlock_ = function(attemptToConnect) {
 
   let blockShadow;
   if (shadowState) {
-    blockShadow = blocks.loadInternal(
+    blockShadow = blocks.appendInternal(
         shadowState,
         parentBlock.workspace,
         {

core/events/events_block_create.js

@@ -94,7 +94,7 @@ BlockCreate.prototype.fromJson = function(json) {
 BlockCreate.prototype.run = function(forward) {
   const workspace = this.getEventWorkspace_();
   if (forward) {
-    blocks.load(this.json, workspace);
+    blocks.append(this.json, workspace);
   } else {
     for (let i = 0; i < this.ids.length; i++) {
       const id = this.ids[i];

core/events/events_block_delete.js

@@ -117,7 +117,7 @@ BlockDelete.prototype.run = function(forward) {
       }
     }
   } else {
-    blocks.load(this.oldJson, workspace);
+    blocks.append(this.oldJson, workspace);
   }
 };
 

core/flyout_base.js

@@ -674,7 +674,7 @@ Flyout.prototype.createButton_ = function(btnInfo, isLabel) {
  * Create a block from the xml and permanently disable any blocks that were
  * defined as disabled.
  * @param {!toolbox.BlockInfo} blockInfo The info of the block.
- * @return {!BlockSvg} The block created from the blockXml.
+ * @return {!BlockSvg} The block created from the blockInfo.
  * @private
  */
 Flyout.prototype.createFlyoutBlock_ = function(blockInfo) {
@@ -694,7 +694,7 @@ Flyout.prototype.createFlyoutBlock_ = function(blockInfo) {
         blockInfo['enabled'] =
             blockInfo['disabled'] !== 'true' && blockInfo['disabled'] !== true;
       }
-      block = blocks.load(
+      block = blocks.append(
           /** @type {blocks.State} */ (blockInfo),this.workspace_);
     }
   }
@@ -1077,7 +1077,7 @@ Flyout.prototype.placeNewBlock_ = function(oldBlock) {
   const json = /** @type {!blocks.State} */ (blocks.save(oldBlock));
   // Normallly this resizes leading to weird jumps. Save it for terminateDrag.
   targetWorkspace.setResizesEnabled(false);
-  const block = /** @type {!BlockSvg} */ (blocks.load(json, targetWorkspace));
+  const block = /** @type {!BlockSvg} */ (blocks.append(json, targetWorkspace));
 
   this.positionNewBlock_(oldBlock, block);
 

core/interfaces/i_serializer.js

@@ -19,17 +19,17 @@ const Workspace = goog.requireType('Blockly.Workspace');
 
 
 /**
- * Serializes and deserializes a plugin.
+ * Serializes and deserializes a plugin or system.
  * @interface
  */
 class ISerializer {
   constructor() {
     /**
-     * A priority value used to determine the order of deserializing plugins.
+     * A priority value used to determine the order of deserializing state.
      * More positive priorities are deserialized before less positive
      * priorities. Eg if you have priorities (0, -10, 10, 100) the order of
      * deserialiation will be (100, 10, 0, -10).
-     * If two plugins have the same priority, they are deserialized in an
+     * If two serializers have the same priority, they are deserialized in an
      * arbitrary order relative to each other.
      * @type {number}
      */
@@ -39,28 +39,28 @@ class ISerializer {
   /* eslint-disable no-unused-vars, valid-jsdoc */
 
   /**
-   * Saves the state of the plugin.
-   * @param {!Workspace} workspace The workspace the plugin to serialize is
+   * Saves the state of the plugin or system.
+   * @param {!Workspace} workspace The workspace the system to serialize is
    *     associated with.
-   * @return {?} A JS object containing the plugin's state, or null if
+   * @return {?} A JS object containing the system's state, or null if
    *     there is no state to record.
    */
   save(workspace) {}
 
   /* eslint-enable valid-jsdoc */
 
   /**
-   * Loads the state of the plugin.
-   * @param {?} state The state of the plugin to deserialize. This will always
+   * Loads the state of the plugin or system.
+   * @param {?} state The state of the system to deserialize. This will always
    *     be non-null.
-   * @param {!Workspace} workspace The workspace the plugin to deserialize is
+   * @param {!Workspace} workspace The workspace the system to deserialize is
    *     associated with.
    */
   load(state, workspace) {}
 
   /**
-   * Clears the state of the plugin.
-   * @param {!Workspace} workspace The workspace the plugin to clear the state
+   * Clears the state of the plugin or system.
+   * @param {!Workspace} workspace The workspace the system to clear the state
    *     of is associated with.
    */
   clear(workspace) {}

core/registry.js

@@ -145,7 +145,7 @@ Type.SERIALIZER = new Type('serializer');
  *     an already registered item.
  * @throws {Error} if the type or name is empty, a name with the given type has
  *     already been registered, or if the given class or object is not valid for
- *     it's type.
+ *     its type.
  * @template T
  */
 const register = function(type, name, registryItem, opt_allowOverrides) {

core/serialization/blocks.js

@@ -20,7 +20,7 @@ const Block = goog.requireType('Blockly.Block');
 const Connection = goog.requireType('Blockly.Connection');
 const Events = goog.require('Blockly.Events');
 // eslint-disable-next-line no-unused-vars
-const {ISerializer} = goog.requireType('Blockly.serialization.ISerializer');
+const {ISerializer} = goog.require('Blockly.serialization.ISerializer');
 const Size = goog.require('Blockly.utils.Size');
 // eslint-disable-next-line no-unused-vars
 const Workspace = goog.requireType('Blockly.Workspace');
@@ -30,7 +30,7 @@ const priorities = goog.require('Blockly.serialization.priorities');
 const serializationRegistry = goog.require('Blockly.serialization.registry');
 
 
-// TODO: Remove this once lint is fixed.
+// TODO(#5160): Remove this once lint is fixed.
 /* eslint-disable no-use-before-define */
 
 /**
@@ -52,9 +52,6 @@ exports.ConnectionState = ConnectionState;
  *     y: (number|undefined),
  *     collapsed: (boolean|undefined),
  *     enabled: (boolean|undefined),
- *     editable: (boolean|undefined),
- *     deletable: (boolean|undefined),
- *     movable: (boolean|undefined),
  *     inline: (boolean|undefined),
  *     data: (string|undefined),
  *     extra-state: (*|undefined),
@@ -148,8 +145,8 @@ const saveAttributes = function(block, state) {
 
 /**
  * Adds the coordinates of the given block to the given state object.
- * @param {!Block} block The block to base the coordinates on
- * @param {!State} state The state object to append to
+ * @param {!Block} block The block to base the coordinates on.
+ * @param {!State} state The state object to append to.
  */
 const saveCoords = function(block, state) {
   const workspace = block.workspace;
@@ -206,19 +203,17 @@ const saveIcons = function(block, state) {
  *     state).
  */
 const saveFields = function(block, state, doFullSerialization) {
-  let hasFieldState = false;
   const fields = Object.create(null);
   for (let i = 0; i < block.inputList.length; i++) {
     const input = block.inputList[i];
     for (let j = 0; j < input.fieldRow.length; j++) {
       const field = input.fieldRow[j];
       if (field.isSerializable()) {
-        hasFieldState = true;
         fields[field.name] = field.saveState(doFullSerialization);
       }
     }
   }
-  if (hasFieldState) {
+  if (Object.keys(fields).length) {
     state['fields'] = fields;
   }
 };
@@ -301,10 +296,10 @@ const saveConnection = function(connection, doFullSerialization) {
  *       by the user. False by default.
  * @return {!Block} The block that was just loaded.
  */
-const load = function(state, workspace, {recordUndo = false} = {}) {
-  return loadInternal(state, workspace, {recordUndo});
+const append = function(state, workspace, {recordUndo = false} = {}) {
+  return appendInternal(state, workspace, {recordUndo});
 };
-exports.load = load;
+exports.append = append;
 
 /**
  * Loads the block represented by the given state into the given workspace.
@@ -318,13 +313,13 @@ exports.load = load;
  *     (boolean|undefined), recordUndo: (boolean|undefined)}=} param1
  *     parentConnection: If provided, the system will attempt to connect the
  *       block to this connection after it is created. Undefined by default.
- *     isShadow: The block will be set to a shadow block after it is created.
- *       False by default.
+ *     isShadow: If true, the block will be set to a shadow block after it is
+ *       created. False by default.
  *     recordUndo: If true, events triggered by this function will be undo-able
  *       by the user. False by default.
- * @return {!Block} The block that was just loaded.
+ * @return {!Block} The block that was just appended.
  */
-const loadInternal = function(
+const appendInternal = function(
     state,
     workspace,
     {
@@ -341,7 +336,7 @@ const loadInternal = function(
   }
   Events.disable();
 
-  const block = loadPrivate(state, workspace, {parentConnection, isShadow});
+  const block = appendPrivate(state, workspace, {parentConnection, isShadow});
 
   Events.enable();
   Events.fire(new (Events.get(Events.BLOCK_CREATE))(block));
@@ -361,24 +356,24 @@ const loadInternal = function(
   return block;
 };
 /** @package */
-exports.loadInternal = loadInternal;
+exports.appendInternal = appendInternal;
 
 /**
  * Loads the block represented by the given state into the given workspace.
  * This is defined privately so that it can be called recursively without firing
  * eroneous events. Events (and other things we only want to occur on the top
- * block) are handled by loadInternal.
+ * block) are handled by appendInternal.
  * @param {!State} state The state of a block to deserialize into the workspace.
  * @param {!Workspace} workspace The workspace to add the block to.
- * @param {{parentConnection: (!Connection|undefined), isShadow:
- *     (boolean|undefined), recordUndo: (boolean|undefined)}=} param1
+ * @param {{parentConnection: (!Connection|undefined),
+ *     isShadow: (boolean|undefined)}=} param1
  *     parentConnection: If provided, the system will attempt to connect the
  *       block to this connection after it is created. Undefined by default.
  *     isShadow: The block will be set to a shadow block after it is created.
  *       False by default.
- * @return {!Block} The block that was just loaded.
+ * @return {!Block} The block that was just appended.
  */
-const loadPrivate = function(
+const appendPrivate = function(
     state,
     workspace,
     {
@@ -460,9 +455,9 @@ const loadExtraState = function(block, state) {
 
 /**
  * Attempts to connect the block to the parent connection, if it exists.
- * @param {(!Connection|undefined)} parentConnection The parent connnection to
+ * @param {(!Connection|undefined)} parentConnection The parent connection to
  *     try to connect the block to.
- * @param {!Block} child The block to try to conecnt to the parent.
+ * @param {!Block} child The block to try to connect to the parent.
  * @param {!State} state The state which defines the given block
  */
 const tryToConnectParent = function(parentConnection, child, state) {
@@ -591,7 +586,7 @@ const loadNextBlocks = function(block, state) {
 /**
  * Applies the state defined by connectionState to the given connection, ie
  * assigns shadows and attaches child blocks.
- * @param {!Connection} connection The connection to serialize the
+ * @param {!Connection} connection The connection to deserialize the
  *     connected blocks of.
  * @param {!ConnectionState} connectionState The object containing the state of
  *     any connected shadow block, or any connected real block.
@@ -601,7 +596,7 @@ const loadConnection = function(connection, connectionState) {
     connection.setShadowState(connectionState['shadow']);
   }
   if (connectionState['block']) {
-    loadPrivate(
+    appendPrivate(
         connectionState['block'],
         connection.getSourceBlock().workspace,
         {parentConnection: connection});
@@ -627,9 +622,8 @@ const initBlock = function(block, rendered) {
   }
 };
 
-// Aliases to disambiguate saving/loading within the serializer.
+// Alias to disambiguate saving within the serializer.
 const saveBlock = save;
-const loadBlock = load;
 
 /**
  * Serializer for saving and loading block state.
@@ -651,18 +645,18 @@ class BlockSerializer {
    *     the workspace's blocks, or null if there are no blocks.
    */
   save(workspace) {
-    const blockState = [];
+    const blockStates = [];
     for (const block of workspace.getTopBlocks(false)) {
       const state = saveBlock(
           block, {addCoordinates: true, doFullSerialization: false});
       if (state) {
-        blockState.push(state);
+        blockStates.push(state);
       }
     }
-    if (blockState.length) {
+    if (blockStates.length) {
       return {
         'languageVersion': 0, // Currently unused.
-        'blocks': blockState
+        'blocks': blockStates
       };
     }
     return null;
@@ -678,7 +672,7 @@ class BlockSerializer {
   load(state, workspace) {
     const blockStates = state['blocks'];
     for (const state of blockStates) {
-      loadBlock(state, workspace, {recordUndo: Events.getRecordUndo()});
+      append(state, workspace, {recordUndo: Events.getRecordUndo()});
     }
   }
 

core/serialization/exceptions.js

@@ -27,6 +27,7 @@ exports.DeserializationError = DeserializationError;
 class MissingBlockType extends DeserializationError {
   /**
    * @param {!State} state The state object which is missing the block type.
+   * @package
    */
   constructor(state) {
     super(`Expected to find a 'type' property, defining the block type`);
@@ -50,6 +51,7 @@ class MissingConnection extends DeserializationError {
    *     'IF0', or 'next'.
    * @param {!Blockly.Block} block The block missing the connection.
    * @param {!State} state The state object containing the bad connection.
+   * @package
    */
   constructor(connection, block, state) {
     super(`The block ${block.toDevString()} is missing a(n) ${connection}
@@ -82,6 +84,7 @@ class BadConnectionCheck extends DeserializationError {
    * @param {!Blockly.Block} childBlock The child block that could not connect
    *     to its parent.
    * @param {!State} childState The state object representing the child block.
+   * @package
    */
   constructor(reason, childConnection, childBlock, childState) {
     super(`The block ${childBlock.toDevString()} could not connect its
@@ -112,6 +115,7 @@ exports.BadConnectionCheck = BadConnectionCheck;
 class RealChildOfShadow extends DeserializationError {
   /**
    * @param {!State} state The state object representing the real block.
+   * @package
    */
   constructor(state) {
     super(`Encountered a real block which is defined as a child of a shadow