Merge pull request #88 from lizlooney/pr_method_name_python_reserved_word_2

Handle method names that are python reserved words.

Author: lizlooney

src/blocks/mrc_call_python_function.ts

@@ -48,7 +48,7 @@ export type FunctionArg = {
   type: string,
 };
 
-type CallPythonFunctionBlock = Blockly.Block & CallPythonFunctionMixin;
+export type CallPythonFunctionBlock = Blockly.Block & CallPythonFunctionMixin;
 interface CallPythonFunctionMixin extends CallPythonFunctionMixinType {
   mrcFunctionKind: FunctionKind,
   mrcReturnType: string,
@@ -57,6 +57,7 @@ interface CallPythonFunctionMixin extends CallPythonFunctionMixinType {
   mrcImportModule: string,
   mrcActualFunctionName: string,
   mrcExportedFunction: boolean,
+  maybeRenameProcedure(this: CallPythonFunctionBlock, oldName: string, legalName: string): void;
 }
 type CallPythonFunctionMixinType = typeof CALL_PYTHON_FUNCTION;
 
@@ -263,6 +264,13 @@ const CALL_PYTHON_FUNCTION = {
         input.setCheck(getAllowedTypesForSetCheck(this.mrcArgs[i].type));
       }
     }
+  },
+  maybeRenameProcedure: function(this: CallPythonFunctionBlock, oldName: string, newName: string): void {
+    if (this.mrcFunctionKind === FunctionKind.INSTANCE_WITHIN) {
+      if (this.getFieldValue(pythonUtils.FIELD_FUNCTION_NAME) == oldName) {
+        this.setFieldValue(newName, pythonUtils.FIELD_FUNCTION_NAME);
+      }
+    }
   }
 };
 
@@ -315,10 +323,8 @@ export const pythonFromBlock = function(
       break;
     }
     case FunctionKind.INSTANCE_WITHIN: {
-      const callPythonFunctionBlock = block as CallPythonFunctionBlock;
-      const functionName = (callPythonFunctionBlock.mrcActualFunctionName)
-          ? callPythonFunctionBlock.mrcActualFunctionName
-          : block.getFieldValue(pythonUtils.FIELD_FUNCTION_NAME);
+      const blocklyName = block.getFieldValue(pythonUtils.FIELD_FUNCTION_NAME);
+      const functionName = generator.getProcedureName(blocklyName);
       code = 'self.' + functionName;
       break;
     }

src/blocks/mrc_class_method_def.ts

@@ -26,6 +26,7 @@ import * as ChangeFramework from './utils/change_framework'
 import { getLegalName } from './utils/python';
 import { Order } from 'blockly/python';
 import { ExtendedPythonGenerator } from '../editor/extended_python_generator';
+import { CallPythonFunctionBlock } from './mrc_call_python_function'
 
 export const BLOCK_NAME = 'mrc_class_method_def';
 
@@ -158,8 +159,10 @@ const CLASS_METHOD_DEF = {
         input.removeField('NAME');
 
         if (this.mrcCanChangeSignature) {
-            input.insertFieldAt(0, new Blockly.FieldTextInput(name), 'NAME');
+            const nameField = new Blockly.FieldTextInput(name);
+            input.insertFieldAt(0, nameField, 'NAME');
             this.setMutator(new Blockly.icons.MutatorIcon([MUTATOR_BLOCK_NAME], this));
+            nameField.setValidator(this.mrcNameFieldValidator.bind(this, nameField));
         }
         else {
             input.insertFieldAt(0, createFieldNonEditableText(name), 'NAME');
@@ -224,8 +227,79 @@ const CLASS_METHOD_DEF = {
             Blockly.Events.enable();
         }
     },
+    mrcNameFieldValidator(this: ClassMethodDefBlock, nameField: Blockly.FieldTextInput, name: string): string {
+      // When the user changes the method name on the block, clear the mrcPythonMethodName field.
+      this.mrcPythonMethodName = '';
+
+      // Strip leading and trailing whitespace.
+      name = name.trim();
+
+      const legalName = findLegalMethodName(name, this);
+      const oldName = nameField.getValue();
+      if (oldName !== name && oldName !== legalName) {
+        // Rename any callers.
+        for (const block of this.workspace.getBlocksByType('mrc_call_python_function')) {
+          (block as CallPythonFunctionBlock).maybeRenameProcedure(oldName, legalName);
+        }
+      }
+      return legalName;
+    },
 };
 
+/**
+ * Ensure two identically-named methods don't exist.
+ * Take the proposed method name, and return a legal name i.e. one that
+ * is not empty and doesn't collide with other methods.
+ *
+ * @param name Proposed method name.
+ * @param block Block to disambiguate.
+ * @returns Non-colliding name.
+ */
+function findLegalMethodName(name: string, block: ClassMethodDefBlock): string {
+  if (block.isInFlyout) {
+    // Flyouts can have multiple methods called 'my_method'.
+    return name;
+  }
+  name = name || 'unnamed';
+  while (isMethodNameUsed(name, block.workspace, block)) {
+    // Collision with another method.
+    const r = name.match(/^(.*?)(\d+)$/);
+    if (!r) {
+      name += '2';
+    } else {
+      name = r[1] + (parseInt(r[2]) + 1);
+    }
+  }
+  return name;
+}
+
+/**
+ * Return if the given name is already a method name.
+ *
+ * @param name The questionable name.
+ * @param workspace The workspace to scan for collisions.
+ * @param opt_exclude Optional block to exclude from comparisons (one doesn't
+ *     want to collide with oneself).
+ * @returns True if the name is used, otherwise return false.
+ */
+function isMethodNameUsed(
+    name: string, workspace: Workspace, opt_exclude?: Block): boolean {
+  const nameLowerCase = name.toLowerCase();
+  for (const block of workspace.getBlocksByType('mrc_class_method_def')) {
+    if (block === opt_exclude) {
+      continue;
+    }
+    if (nameLowerCase === block.getFieldValue('NAME').toLowerCase()) {
+      return true;
+    }
+    if (block.mrcPythonMethodName &&
+        nameLowerCase === block.mrcPythonMethodName.toLowerCase()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 const METHOD_PARAM_CONTAINER = {
     init: function (this : Blockly.Block) {
         this.appendDummyInput("TITLE").appendField('Parameters');
@@ -421,7 +495,7 @@ export const pythonFromBlock = function (
         xfix2 +
         returnValue;
     code = generator.scrub_(block, code);
-    generator.addClassMethodDefinition(block.getFieldValue('NAME'), funcName, code);
+    generator.addClassMethodDefinition(funcName, code);
 
     return '';
-}
+}

src/editor/extended_python_generator.ts

@@ -97,8 +97,7 @@ export class ExtendedPythonGenerator extends PythonGenerator {
   /**
    * Add a class method definition.
    */
-  addClassMethodDefinition(nameFieldValue: string, methodName: string, code: string): void {
-    this.context.addClassMethodName(nameFieldValue, methodName);
+  addClassMethodDefinition(methodName: string, code: string): void {
     this.classMethods[methodName] = code;
   }
 

src/editor/generator_context.ts

@@ -33,9 +33,6 @@ export class GeneratorContext {
   // The exported blocks for the current module.
   private exportedBlocks: Block[] = [];
 
-  // Key is the mrc_class_method_def block's NAME field, value is the python method name.
-  private classMethodNames: {[key: string]: string} = Object.create(null);
-
   // Has mechanisms (ie, needs in init)
   private hasMechanisms = false;
 
@@ -46,12 +43,13 @@ export class GeneratorContext {
 
   clear(): void {
     this.clearExportedBlocks();
-    this.clearClassMethodNames();
     this.hasMechanisms = false;
   }
+
   setHasMechanism():void{
     this.hasMechanisms = true;
   }
+
   getHasMechanisms():boolean{
     return this.hasMechanisms;
   }
@@ -101,21 +99,4 @@ export class GeneratorContext {
   getExportedBlocks(): Block[] {
     return this.exportedBlocks;
   }
-
-  clearClassMethodNames() {
-    this.classMethodNames = Object.create(null);
-  }
-
-  addClassMethodName(nameFieldValue: string, methodName: string) {
-    if (nameFieldValue !== methodName) {
-      this.classMethodNames[nameFieldValue] = methodName;
-    }
-  }
-
-  getClassMethodName(nameFieldValue: string): string | null {
-    if (this.classMethodNames[nameFieldValue]) {
-      return this.classMethodNames[nameFieldValue];
-    }
-    return nameFieldValue;
-  }
 }

src/toolbox/methods_category.ts

@@ -118,7 +118,6 @@ export class MethodsCategory {
             functionKind: 'instance_within',
             returnType: classMethodDefBlock.mrcReturnType,
             args: [],
-            actualFunctionName: classMethodDefBlock.mrcPythonMethodName,
           },
           fields: {
             FUNC: classMethodDefBlock.getFieldValue('NAME'),