Drivers UI: Added name validation/linting for Driver Variables

When attempting to change a driver variable name to an "invalid" name,
an indicator will now be shown beside the offending variable name.
Clicking on this icon will show a popup which provides more information
about why the variable name cannot be used.

Reasons that it knows about are:
1) Starts with number
2) Has a dot
3) Has a space
4) Starts with or contains a special character
5) Starts with an underscore (Python does allow this, but it's bad practice,
   and makes checking security of drivers harder)
6) Is a reserved Python keyword

5 files changed, 162 insertions, 7 deletions

diff --git a/source/blender/blenkernel/BKE_fcurve.h b/source/blender/blenkernel/BKE_fcurve.h
index 443a03a475a..a1176b91312 100644
--- a/source/blender/blenkernel/BKE_fcurve.h
+++ b/source/blender/blenkernel/BKE_fcurve.h
@@ -95,6 +95,7 @@ struct ChannelDriver *fcurve_copy_driver(struct ChannelDriver *driver);
 
 void driver_free_variable(struct ChannelDriver *driver, struct DriverVar *dvar);
 void driver_change_variable_type(struct DriverVar *dvar, int type);
+void driver_variable_name_validate(struct DriverVar *dvar);
 struct DriverVar *driver_add_new_variable(struct ChannelDriver *driver);
 
 float driver_get_variable_value(struct ChannelDriver *driver, struct DriverVar *dvar);
diff --git a/source/blender/blenkernel/intern/fcurve.c b/source/blender/blenkernel/intern/fcurve.c
index abf847274c4..e2b1acddc9c 100644
--- a/source/blender/blenkernel/intern/fcurve.c
+++ b/source/blender/blenkernel/intern/fcurve.c
@@ -1593,6 +1593,69 @@ void driver_change_variable_type(DriverVar *dvar, int type)
 	DRIVER_TARGETS_LOOPER_END
 }
 
+/* Validate driver name (after being renamed) */
+void driver_variable_name_validate(DriverVar *dvar)
+{
+	/* Special character blacklist */
+	const char special_char_blacklist[] = {
+		'~', '`', '!', '@', '#', '$', '%', '^', '&', '*', '+', '=', '-',
+	    '/', '\\', '?', ':', ';',  '<', '>', '{', '}', '[', ']', '|',
+		' ', '.', '\t', '\n', '\r'
+	};
+	
+	/* sanity checks */
+	if (dvar == NULL)
+		return;
+	
+	/* clear all invalid-name flags */
+	dvar->flag &= ~DVAR_ALL_INVALID_FLAGS;
+	
+	/* 1) Must start with a letter */
+	/* XXX: We assume that valid unicode letters in other languages are ok too, hence the blacklisting */
+	if (ELEM(dvar->name[0], '0', '1', '2', '3', '4', '5', '6', '7', '8', '9')) {
+		dvar->flag |= DVAR_FLAG_INVALID_START_NUM;
+	}
+	else if (dvar->name[0] == '_') {
+		/* NOTE: We don't allow names to start with underscores (i.e. it helps when ruling out security risks) */
+		dvar->flag |= DVAR_FLAG_INVALID_START_CHAR;
+	}
+	
+	/* 2) Must not contain invalid stuff in the middle of the string */
+	if (strchr(dvar->name, ' ')) {
+		dvar->flag |= DVAR_FLAG_INVALID_HAS_SPACE;
+	}
+	if (strchr(dvar->name, '.')) {
+		dvar->flag |= DVAR_FLAG_INVALID_HAS_DOT;
+	}
+	
+	/* 3) Check for special characters - Either at start, or in the middle */
+	for (int i = 0; i < sizeof(special_char_blacklist); i++) {
+		char *match = strchr(dvar->name, special_char_blacklist[i]);
+		
+		if (match == dvar->name)
+			dvar->flag |= DVAR_FLAG_INVALID_START_CHAR;
+		else if (match != NULL)
+			dvar->flag |= DVAR_FLAG_INVALID_HAS_SPECIAL;
+	}
+	
+	/* 4) Check if the name is a reserved keyword
+	 * NOTE: These won't confuse Python, but it will be impossible to use the variable
+	 *       in an expression without Python misinterpreting what these are for
+	 */
+	if (STREQ(dvar->name, "if") || STREQ(dvar->name, "elif") || STREQ(dvar->name, "else") ||
+	    STREQ(dvar->name, "for") || STREQ(dvar->name, "while") || STREQ(dvar->name, "def") ||
+	    STREQ(dvar->name, "True") || STREQ(dvar->name, "False") || STREQ(dvar->name, "import") ||
+	    STREQ(dvar->name, "pass")  || STREQ(dvar->name, "with"))
+	{
+		dvar->flag |= DVAR_FLAG_INVALID_PY_KEYWORD;
+	}
+	
+	
+	/* If any these conditions match, the name is invalid */
+	if (dvar->flag & DVAR_ALL_INVALID_FLAGS)
+		dvar->flag |= DVAR_FLAG_INVALID_NAME;
+}
+
 /* Add a new driver variable */
 DriverVar *driver_add_new_variable(ChannelDriver *driver)
 {
@@ -1619,7 +1682,7 @@ DriverVar *driver_add_new_variable(ChannelDriver *driver)
 	if (driver->type == DRIVER_TYPE_PYTHON)
 		driver->flag |= DRIVER_FLAG_RENAMEVAR;
 #endif
-
+	
 	/* return the target */
 	return dvar;
 }
diff --git a/source/blender/editors/space_graph/graph_buttons.c b/source/blender/editors/space_graph/graph_buttons.c
index 1bab7bd349c..858baf61576 100644
--- a/source/blender/editors/space_graph/graph_buttons.c
+++ b/source/blender/editors/space_graph/graph_buttons.c
@@ -519,6 +519,39 @@ static void driver_delete_var_cb(bContext *UNUSED(C), void *driver_v, void *dvar
 	driver_free_variable(driver, dvar);
 }
 
+/* callback to report why a driver variable is invalid */
+static void driver_dvar_invalid_name_query_cb(bContext *C, void *dvar_v, void *UNUSED(arg))
+{
+	uiPopupMenu *pup = UI_popup_menu_begin(C, CTX_IFACE_(BLT_I18NCONTEXT_OPERATOR_DEFAULT, "Invalid Variable Name"), ICON_NONE);
+	uiLayout *layout = UI_popup_menu_layout(pup);
+	
+	DriverVar *dvar = (DriverVar *)dvar_v;
+	
+	if (dvar->flag & DVAR_FLAG_INVALID_START_NUM) {
+		uiItemL(layout, "It cannot start with a number", ICON_ERROR);
+	}
+	if (dvar->flag & DVAR_FLAG_INVALID_START_CHAR) {
+		uiItemL(layout, 
+		        "It cannot start with a special character,"
+		        " including '$', '@', '!', '~', '+', '-', '_', '.', or ' '",
+				ICON_NONE);
+	}
+	if (dvar->flag & DVAR_FLAG_INVALID_HAS_SPACE) {
+		uiItemL(layout, "It cannot contain spaces (e.g. 'a space')", ICON_ERROR);
+	}
+	if (dvar->flag & DVAR_FLAG_INVALID_HAS_DOT) {
+		uiItemL(layout, "It cannot contain dots (e.g. 'a.dot')", ICON_ERROR);
+	}
+	if (dvar->flag & DVAR_FLAG_INVALID_HAS_SPECIAL) {
+		uiItemL(layout, "It cannot contain special (non-alphabetical/numeric) characters", ICON_ERROR);
+	}
+	if (dvar->flag & DVAR_FLAG_INVALID_PY_KEYWORD) {
+		uiItemL(layout, "It cannot be a reserved keyword in Python", ICON_INFO);
+	}
+	
+	UI_popup_menu_end(C, pup);
+}
+
 /* callback to reset the driver's flags */
 static void driver_update_flags_cb(bContext *UNUSED(C), void *fcu_v, void *UNUSED(arg))
 {
@@ -816,8 +849,16 @@ static void graph_panel_drivers(const bContext *C, Panel *pa)
 		/* variable name */
 		uiItemR(row, &dvar_ptr, "name", 0, "", ICON_NONE);
 		
-		/* remove button */
+		/* invalid name? */
 		UI_block_emboss_set(block, UI_EMBOSS_NONE);
+		
+		if (dvar->flag & DVAR_FLAG_INVALID_NAME) {
+			but = uiDefIconBut(block, UI_BTYPE_BUT, B_IPO_DEPCHANGE, ICON_ERROR, 290, 0, UI_UNIT_X, UI_UNIT_Y,
+			                   NULL, 0.0, 0.0, 0.0, 0.0, IFACE_("Invalid variable name. Click here for details"));
+			UI_but_func_set(but, driver_dvar_invalid_name_query_cb, dvar, NULL); // XXX: reports?
+		}
+		
+		/* remove button */
 		but = uiDefIconBut(block, UI_BTYPE_BUT, B_IPO_DEPCHANGE, ICON_X, 290, 0, UI_UNIT_X, UI_UNIT_Y,
 		                   NULL, 0.0, 0.0, 0.0, 0.0, IFACE_("Delete target variable"));
 		UI_but_func_set(but, driver_delete_var_cb, driver, dvar);
diff --git a/source/blender/makesdna/DNA_anim_types.h b/source/blender/makesdna/DNA_anim_types.h
index d80b50945df..fc32bbd1e99 100644
--- a/source/blender/makesdna/DNA_anim_types.h
+++ b/source/blender/makesdna/DNA_anim_types.h
@@ -327,13 +327,15 @@ typedef enum eDriverTarget_TransformChannels {
  */
 typedef struct DriverVar {
 	struct DriverVar *next, *prev;
-
+	
 	char name[64];              /* name of the variable to use in py-expression (must be valid python identifier) - MAX_ID_NAME-2 */
-
+	
 	DriverTarget targets[8];    /* MAX_DRIVER_TARGETS, target slots */
-	short num_targets;          /* number of targets actually used by this variable */
-
-	short type;                 /* type of driver target (eDriverTarget_Types) */
+	
+	char num_targets;           /* number of targets actually used by this variable */
+	char type;                  /* type of driver variable (eDriverVar_Types) */
+	
+	short flag;                 /* validation tags, etc. (eDriverVar_Flags) */
 	float curval;               /* result of previous evaluation */
 } DriverVar;
 
@@ -355,6 +357,38 @@ typedef enum eDriverVar_Types {
 	MAX_DVAR_TYPES
 } eDriverVar_Types;
 
+/* Driver Variable Flags */
+typedef enum eDriverVar_Flags {
+	/* variable is not set up correctly */
+	DVAR_FLAG_ERROR               = (1 << 0),
+	
+	/* variable name doesn't pass the validation tests */
+	DVAR_FLAG_INVALID_NAME        = (1 << 1),
+	/* name starts with a number */
+	DVAR_FLAG_INVALID_START_NUM   = (1 << 2),
+	/* name starts with a special character (!, $, @, #, _, etc.) */
+	DVAR_FLAG_INVALID_START_CHAR  = (1 << 3),
+	/* name contains a space */
+	DVAR_FLAG_INVALID_HAS_SPACE   = (1 << 4),
+	/* name contains a dot */
+	DVAR_FLAG_INVALID_HAS_DOT     = (1 << 5),
+	/* name contains invalid chars */
+	DVAR_FLAG_INVALID_HAS_SPECIAL = (1 << 6),
+	/* name is a reserved keyword */
+	DVAR_FLAG_INVALID_PY_KEYWORD  = (1 << 7),
+} eDriverVar_Flags;
+
+/* All invalid dvar name flags */
+#define DVAR_ALL_INVALID_FLAGS (   \
+	DVAR_FLAG_INVALID_NAME |       \
+	DVAR_FLAG_INVALID_START_NUM | \
+	DVAR_FLAG_INVALID_START_CHAR | \
+	DVAR_FLAG_INVALID_HAS_SPACE |  \
+	DVAR_FLAG_INVALID_HAS_DOT |    \
+	DVAR_FLAG_INVALID_HAS_SPECIAL |  \
+	DVAR_FLAG_INVALID_PY_KEYWORD  \
+)
+
 /* --- */
 
 /* Channel Driver (i.e. Drivers / Expressions) (driver)
diff --git a/source/blender/makesrna/intern/rna_fcurve.c b/source/blender/makesrna/intern/rna_fcurve.c
index fbc332f5658..c34d8125367 100644
--- a/source/blender/makesrna/intern/rna_fcurve.c
+++ b/source/blender/makesrna/intern/rna_fcurve.c
@@ -275,6 +275,15 @@ static void rna_DriverVariable_type_set(PointerRNA *ptr, int value)
 	driver_change_variable_type(dvar, value);
 }
 
+void rna_DriverVariable_name_set(PointerRNA *ptr, const char *value)
+{
+	DriverVar *data = (DriverVar *)(ptr->data);
+	
+	BLI_strncpy_utf8(data->name, value, 64);
+	driver_variable_name_validate(data);
+}
+
+
 /* ----------- */
 
 static DriverVar *rna_Driver_new_variable(ChannelDriver *driver)
@@ -1474,6 +1483,7 @@ static void rna_def_drivervar(BlenderRNA *brna)
 	/* Variable Name */
 	prop = RNA_def_property(srna, "name", PROP_STRING, PROP_NONE);
 	RNA_def_struct_name_property(srna, prop);
+	RNA_def_property_string_funcs(prop, NULL, NULL, "rna_DriverVariable_name_set");
 	RNA_def_property_ui_text(prop, "Name",
 	                         "Name to use in scripted expressions/functions (no spaces or dots are allowed, "
 	                         "and must start with a letter)");
@@ -1493,6 +1503,12 @@ static void rna_def_drivervar(BlenderRNA *brna)
 	RNA_def_property_collection_sdna(prop, NULL, "targets", "num_targets");
 	RNA_def_property_struct_type(prop, "DriverTarget");
 	RNA_def_property_ui_text(prop, "Targets", "Sources of input data for evaluating this variable");
+	
+	/* Name Validity Flags */
+	prop = RNA_def_property(srna, "is_name_valid", PROP_BOOLEAN, PROP_NONE);
+	RNA_def_property_boolean_negative_sdna(prop, NULL, "flag", DVAR_FLAG_INVALID_NAME);
+	RNA_def_property_clear_flag(prop, PROP_EDITABLE);
+	RNA_def_property_ui_text(prop, "Is Name Valid", "Is this a valid name for a driver variable");	
 }