new warning - almost all plpgsql auto variables (without new and old)…

… should not be modified by user

README.md

@@ -144,7 +144,7 @@ You can set level of warnings via function's parameters:
 
 * `extra_warnings boolean DEFAULT true` - show warnings like missing `RETURN`,
   shadowed variables, dead code, never read (unused) function's parameter,
-  unmodified variables, ..
+  unmodified variables, modified auto variables, ..
 
 * `performance_warnings boolean DEFAULT false` - performance related warnings like
   declared type with type modificator, casting, implicit casts in where clause (can be

src/assign.c

@@ -42,7 +42,28 @@ plpgsql_check_record_variable_usage(PLpgSQL_checkstate *cstate, int dno, bool wr
 		if (!write)
 			cstate->used_variables = bms_add_member(cstate->used_variables, dno);
 		else
+		{
 			cstate->modif_variables = bms_add_member(cstate->modif_variables, dno);
+
+			/* raise extra warning when protected variable is modified */
+			if (bms_is_member(dno, cstate->protected_variables))
+			{
+				PLpgSQL_variable *var = (PLpgSQL_variable *) cstate->estate->datums[dno];
+				StringInfoData message;
+
+				initStringInfo(&message);
+
+				appendStringInfo(&message, "auto varible \"%s\" should not be modified by user", var->refname);
+				plpgsql_check_put_error(cstate,
+						  0, var->lineno,
+						  message.data,
+						  NULL,
+						  NULL,
+						  PLPGSQL_CHECK_WARNING_EXTRA,
+						  0, NULL, NULL);
+				pfree(message.data);
+			}
+		}
 	}
 }
 

src/check_function.c

@@ -53,7 +53,7 @@ static void function_check(PLpgSQL_function *func, PLpgSQL_checkstate *cstate);
 static void trigger_check(PLpgSQL_function *func, Node *tdata, PLpgSQL_checkstate *cstate);
 static void release_exprs(List *exprs);
 static int load_configuration(HeapTuple procTuple, bool *reload_config);
-static void init_datum_dno(PLpgSQL_checkstate *cstate, int dno);
+static void init_datum_dno(PLpgSQL_checkstate *cstate, int dno, bool is_auto, bool is_protected);
 static PLpgSQL_datum * copy_plpgsql_datum(PLpgSQL_checkstate *cstate, PLpgSQL_datum *datum);
 static void plpgsql_check_setup_estate(PLpgSQL_execstate *estate, PLpgSQL_function *func, ReturnSetInfo *rsi, plpgsql_check_info *cinfo);
 static void plpgsql_check_setup_cstate(PLpgSQL_checkstate *cstate, plpgsql_check_result_info *result_info,
@@ -72,7 +72,7 @@ collect_out_variables(PLpgSQL_function *func, PLpgSQL_checkstate *cstate)
 		int		varno = func->out_param_varno;
 		PLpgSQL_variable *var = (PLpgSQL_variable *) func->datums[varno];
 
-		if (var->dtype == PLPGSQL_DTYPE_ROW && is_internal_variable(var))
+		if (var->dtype == PLPGSQL_DTYPE_ROW && is_internal_variable(cstate, var))
 		{
 			/* this function has more OUT parameters */
 			PLpgSQL_row *row = (PLpgSQL_row*) var;
@@ -521,6 +521,8 @@ function_check(PLpgSQL_function *func, PLpgSQL_checkstate *cstate)
 	for (i = 0; i < cstate->estate->ndatums; i++)
 		cstate->estate->datums[i] = copy_plpgsql_datum(cstate, func->datums[i]);
 
+	init_datum_dno(cstate, cstate->estate->found_varno, true, true);
+
 	/*
 	 * check function's parameters to not be reserved keywords
 	 */
@@ -553,7 +555,7 @@ function_check(PLpgSQL_function *func, PLpgSQL_checkstate *cstate)
 	 */
 	for (i = 0; i < func->fn_nargs; i++)
 	{
-		init_datum_dno(cstate, func->fn_argvarnos[i]);
+		init_datum_dno(cstate, func->fn_argvarnos[i], false, false);
 	}
 
 	/*
@@ -600,6 +602,8 @@ trigger_check(PLpgSQL_function *func, Node *tdata, PLpgSQL_checkstate *cstate)
 	for (i = 0; i < cstate->estate->ndatums; i++)
 		cstate->estate->datums[i] = copy_plpgsql_datum(cstate, func->datums[i]);
 
+	init_datum_dno(cstate, cstate->estate->found_varno, true, true);
+
 	if (IsA(tdata, TriggerData))
 	{
 		TriggerData *trigdata = (TriggerData *) tdata;
@@ -624,7 +628,7 @@ trigger_check(PLpgSQL_function *func, Node *tdata, PLpgSQL_checkstate *cstate)
 			PLpgSQL_datum *datum = func->datums[i];
 
 			if (datum->dtype == PLPGSQL_DTYPE_PROMISE)
-				init_datum_dno(cstate, datum->dno);
+				init_datum_dno(cstate, datum->dno, true, datum->dno != func->new_varno && datum->dno != func->old_varno);
 		}
 
 		rec_new = (PLpgSQL_rec *) (cstate->estate->datums[func->new_varno]);
@@ -647,16 +651,16 @@ trigger_check(PLpgSQL_function *func, Node *tdata, PLpgSQL_checkstate *cstate)
 		/*
 		 * Assign the special tg_ variables
 		 */
-		init_datum_dno(cstate, func->tg_op_varno);
-		init_datum_dno(cstate, func->tg_name_varno);
-		init_datum_dno(cstate, func->tg_when_varno);
-		init_datum_dno(cstate, func->tg_level_varno);
-		init_datum_dno(cstate, func->tg_relid_varno);
-		init_datum_dno(cstate, func->tg_relname_varno);
-		init_datum_dno(cstate, func->tg_table_name_varno);
-		init_datum_dno(cstate, func->tg_table_schema_varno);
-		init_datum_dno(cstate, func->tg_nargs_varno);
-		init_datum_dno(cstate, func->tg_argv_varno);
+		init_datum_dno(cstate, func->tg_op_varno, true, true);
+		init_datum_dno(cstate, func->tg_name_varno, true, true);
+		init_datum_dno(cstate, func->tg_when_varno, true, true);
+		init_datum_dno(cstate, func->tg_level_varno, true, true);
+		init_datum_dno(cstate, func->tg_relid_varno, true, true);
+		init_datum_dno(cstate, func->tg_relname_varno, true, true);
+		init_datum_dno(cstate, func->tg_table_name_varno, true, true);
+		init_datum_dno(cstate, func->tg_table_schema_varno, true, true);
+		init_datum_dno(cstate, func->tg_nargs_varno, true, true);
+		init_datum_dno(cstate, func->tg_argv_varno, true, true);
 
 #endif
 
@@ -666,8 +670,8 @@ trigger_check(PLpgSQL_function *func, Node *tdata, PLpgSQL_checkstate *cstate)
 
 #if PG_VERSION_NUM < 110000
 
-		init_datum_dno(cstate, func->tg_event_varno);
-		init_datum_dno(cstate, func->tg_tag_varno);
+		init_datum_dno(cstate, func->tg_event_varno, true, true);
+		init_datum_dno(cstate, func->tg_tag_varno, true, true);
 
 #endif
 
@@ -1042,6 +1046,8 @@ plpgsql_check_setup_cstate(PLpgSQL_checkstate *cstate,
 	cstate->fake_rtd = fake_rtd;
 
 	cstate->safe_variables = NULL;
+	cstate->protected_variables = NULL;
+	cstate->auto_variables = NULL;
 
 	cstate->stop_check = false;
 	cstate->allow_mp = false;
@@ -1109,7 +1115,7 @@ release_exprs(List *exprs)
  *
  */
 static void
-init_datum_dno(PLpgSQL_checkstate *cstate, int dno)
+init_datum_dno(PLpgSQL_checkstate *cstate, int dno, bool is_auto, bool is_protected)
 {
 	switch (cstate->estate->datums[dno]->dtype)
 	{
@@ -1153,14 +1159,19 @@ init_datum_dno(PLpgSQL_checkstate *cstate, int dno)
 					if (row->varnos[fnum] < 0)
 						continue;		/* skip dropped column in row struct */
 
-					init_datum_dno(cstate, row->varnos[fnum]);
+					init_datum_dno(cstate, row->varnos[fnum], is_auto, is_protected);
 				}
 			}
 			break;
 
 		default:
 			elog(ERROR, "unexpected dtype: %d", cstate->estate->datums[dno]->dtype);
 	}
+
+	if (is_protected)
+		cstate->protected_variables = bms_add_member(cstate->protected_variables, dno);
+	if (is_auto)
+		cstate->auto_variables = bms_add_member(cstate->auto_variables, dno);
 }
 
 /*

src/plpgsql_check.h

@@ -103,6 +103,8 @@ typedef struct PLpgSQL_checkstate
 	plpgsql_check_info *cinfo;
 	Bitmapset	   *safe_variables;			/* track which variables are safe against sql injection */
 	Bitmapset	   *out_variables;			/* what variables are used as OUT variables */
+	Bitmapset	   *protected_variables;	/* what variables should be assigned internal only */
+	Bitmapset	   *auto_variables;			/* variables initialized, used by runtime */
 	bool			stop_check;				/* true after error when fatal_errors option is active */
 	bool			allow_mp;				/* true, when multiple plans in plancache are allowed */
 	bool			has_mp;					/* true, when multiple plan was used */
@@ -229,7 +231,7 @@ extern void plpgsql_check_assignment_to_variable(PLpgSQL_checkstate *cstate, PLp
 extern char * plpgsql_check_datum_get_refname(PLpgSQL_datum *d);
 extern void plpgsql_check_report_unused_variables(PLpgSQL_checkstate *cstate);
 extern void plpgsql_check_report_too_high_volatility(PLpgSQL_checkstate *cstate);
-extern bool is_internal_variable(PLpgSQL_variable *var);
+extern bool is_internal_variable(PLpgSQL_checkstate *cstate, PLpgSQL_variable *var);
 
 /*
  * functions from stmtwalk.c

src/report.c

@@ -35,8 +35,11 @@ is_internal(char *refname, int lineno)
 }
 
 bool
-is_internal_variable(PLpgSQL_variable *var)
+is_internal_variable(PLpgSQL_checkstate *cstate, PLpgSQL_variable *var)
 {
+	if (bms_is_member(var->dno, cstate->auto_variables))
+		return true;
+
 	return is_internal(var->refname, var->lineno);
 }
 
@@ -91,6 +94,9 @@ datum_is_explicit(PLpgSQL_checkstate *cstate, int dno)
 {
 	PLpgSQL_execstate *estate = cstate->estate;
 
+	if (bms_is_member(dno, cstate->auto_variables))
+		return false;
+
 	switch (estate->datums[dno]->dtype)
 	{
 		case PLPGSQL_DTYPE_VAR:
@@ -229,7 +235,6 @@ plpgsql_check_report_unused_variables(PLpgSQL_checkstate *cstate)
 		PLpgSQL_function *func = estate->func;
 
 		/* check never read variables */
-
 		for (i = 0; i < estate->ndatums; i++)
 		{
 			if (datum_is_explicit(cstate, i)
@@ -320,7 +325,7 @@ plpgsql_check_report_unused_variables(PLpgSQL_checkstate *cstate)
 			int		varno = func->out_param_varno;
 			PLpgSQL_variable *var = (PLpgSQL_variable *) estate->datums[varno];
 
-			if (var->dtype == PLPGSQL_DTYPE_ROW && is_internal_variable(var))
+			if (var->dtype == PLPGSQL_DTYPE_ROW && is_internal_variable(cstate, var))
 			{
 				/* this function has more OUT parameters */
 				PLpgSQL_row *row = (PLpgSQL_row*) var;

src/stmtwalk.c

@@ -623,6 +623,10 @@ plpgsql_check_stmt(PLpgSQL_checkstate *cstate, PLpgSQL_stmt *stmt, int *closing,
 					if (stmt_fori->step)
 						plpgsql_check_assignment(cstate, stmt_fori->step, NULL, NULL, dno);
 
+					/* this variable should not be updated */
+					cstate->protected_variables = bms_add_member(cstate->protected_variables, dno);
+					cstate->auto_variables = bms_add_member(cstate->auto_variables, dno);
+
 					check_stmts(cstate, stmt_fori->body, &closing_local, &exceptions_local);
 					*closing = possibly_closed(closing_local);
 				}

src/typdesc.c

@@ -84,6 +84,8 @@ plpgsql_check_CallExprGetRowTarget(PLpgSQL_checkstate *cstate, PLpgSQL_expr *Cal
 
 		row = palloc0(sizeof(PLpgSQL_row));
 		row->dtype = PLPGSQL_DTYPE_ROW;
+		row->dno = -1;
+		row->refname = NULL;
 		row->lineno = 0;
 		row->varnos = palloc(sizeof(int) * list_length(funcargs));