[typer] error on invalid identifier names from macros, closes #5002 (#8708)

* [typer] error on invalid identifier names from macros, closes #5002

* Capitalize kind at the start of the error

* Some cleanup

* capture variables ->pattern variables

* Make sure `new` isn't allowed in variable names

* Use quotes instead of backticks + s_escape the name

* Minor

* Don't allow fields to be named `this`

src/context/typecore.ml

@@ -222,7 +222,40 @@ let add_local ctx k n t p =
 	ctx.locals <- PMap.add n v ctx.locals;
 	v
 
+let check_identifier_name ctx name kind p =
+	if starts_with name '$' then
+		display_error ctx ((StringHelper.capitalize kind) ^ " names starting with a dollar are not allowed") p
+	else if not (Lexer.is_valid_identifier name) then
+		display_error ctx ("\"" ^ (StringHelper.s_escape name) ^ "\" is not a valid " ^ kind ^ " name") p
+
+let check_field_name ctx name p =
+	match name with
+	| "new" -> () (* the only keyword allowed in field names *)
+	| _ -> check_identifier_name ctx name "field" p
+
+let check_type_name ctx name p =
+	if Ast.is_lower_ident name then
+		display_error ctx "Type name should start with an uppercase letter" p
+	else
+		check_identifier_name ctx name "type" p
+
+let check_local_variable_name ctx name origin p =
+	match name with
+	| "this" -> () (* TODO: vars named `this` should technically be VGenerated, not VUser *)
+	| _ ->
+		let s_var_origin origin =
+			match origin with
+			| TVOLocalVariable -> "variable"
+			| TVOArgument -> "function argument"
+			| TVOForVariable -> "for variable"
+			| TVOPatternVariable -> "pattern variable"
+			| TVOCatchVariable -> "catch variable"
+			| TVOLocalFunction -> "function"
+		in
+		check_identifier_name ctx name (s_var_origin origin) p
+
 let add_local_with_origin ctx origin n t p =
+	check_local_variable_name ctx n origin p;
 	add_local ctx (VUser origin) n t p
 
 let gen_local_prefix = "`"

src/typing/matcher.ml

@@ -190,7 +190,7 @@ module Pattern = struct
 		in
 		let add_local final name p =
 			let is_wildcard_local = name = "_" in
-			if not is_wildcard_local && pctx.is_postfix_match then error "Capture variables are not allowed in .match patterns" p;
+			if not is_wildcard_local && pctx.is_postfix_match then error "Pattern variables are not allowed in .match patterns" p;
 			if not is_wildcard_local && PMap.mem name pctx.current_locals then error (Printf.sprintf "Variable %s is bound multiple times" name) p;
 			match pctx.or_locals with
 			| Some map when not is_wildcard_local ->
@@ -276,7 +276,7 @@ module Pattern = struct
 				with _ ->
 					restore();
 					if not (is_lower_ident s) && (match s.[0] with '`' | '_' -> false | _ -> true) then begin
-						display_error ctx "Capture variables must be lower-case" p;
+						display_error ctx "Pattern variables must be lower-case" p;
 					end;
 					let sl = match follow t with
 						| TEnum(en,_) ->

src/typing/typeloadFields.ml

@@ -1322,7 +1322,7 @@ let init_field (ctx,cctx,fctx) f =
 	let name = fst f.cff_name in
 	TypeloadCheck.check_global_metadata ctx f.cff_meta (fun m -> f.cff_meta <- m :: f.cff_meta) c.cl_module.m_path c.cl_path (Some name);
 	let p = f.cff_pos in
-	if starts_with name '$' then display_error ctx "Field names starting with a dollar are not allowed" p;
+	Typecore.check_field_name ctx name p;
 	List.iter (fun acc ->
 		match (fst acc, f.cff_kind) with
 		| APublic, _ | APrivate, _ | AStatic, _ | AFinal, _ | AExtern, _ -> ()

src/typing/typeloadModule.ml

@@ -203,7 +203,7 @@ let module_pass_1 ctx m tdecls loadp =
 			| Some _ -> error "import and using may not appear after a type declaration" p)
 		| EClass d ->
 			let name = fst d.d_name in
-			if starts_with name '$' then error "Type names starting with a dollar are not allowed" p;
+			Typecore.check_type_name ctx name p;
 			pt := Some p;
 			let priv = List.mem HPrivate d.d_flags in
 			let path = make_path name priv in
@@ -224,7 +224,7 @@ let module_pass_1 ctx m tdecls loadp =
 			acc
 		| EEnum d ->
 			let name = fst d.d_name in
-			if starts_with name '$' then error "Type names starting with a dollar are not allowed" p;
+			Typecore.check_type_name ctx name p;
 			pt := Some p;
 			let priv = List.mem EPrivate d.d_flags in
 			let path = make_path name priv in
@@ -248,7 +248,7 @@ let module_pass_1 ctx m tdecls loadp =
 			acc
 		| ETypedef d ->
 			let name = fst d.d_name in
-			if starts_with name '$' then error "Type names starting with a dollar are not allowed" p;
+			Typecore.check_type_name ctx name p;
 			if has_meta Meta.Using d.d_meta then error "@:using on typedef is not allowed" p;
 			pt := Some p;
 			let priv = List.mem EPrivate d.d_flags in
@@ -275,7 +275,7 @@ let module_pass_1 ctx m tdecls loadp =
 			acc
 		 | EAbstract d ->
 		 	let name = fst d.d_name in
-			if starts_with name '$' then error "Type names starting with a dollar are not allowed" p;
+			Typecore.check_type_name ctx name p;
 			let priv = List.mem AbPrivate d.d_flags in
 			let path = make_path name priv in
 			let a = {

src/typing/typer.ml

@@ -1467,7 +1467,6 @@ and type_vars ctx vl p =
 					let e = AbstractCast.cast_or_unify ctx t e p in
 					Some e
 			) in
-			if starts_with v '$' then display_error ctx "Variables names starting with a dollar are not allowed" p;
 			let v = add_local_with_origin ctx TVOLocalVariable v t pv in
 			if final then v.v_final <- true;
 			if ctx.in_display && DisplayPosition.display_position#enclosed_in pv then
@@ -1894,7 +1893,6 @@ and type_try ctx e1 catches with_type p =
 			| _ -> error "Catch type must be a class, an enum or Dynamic" (pos e_ast)
 		in
 		let name,t2 = loop t in
-		if starts_with v '$' then display_error ctx "Catch variable names starting with a dollar are not allowed" p;
 		check_unreachable acc1 t2 (pos e_ast);
 		let locals = save_locals ctx in
 		let v = add_local_with_origin ctx TVOCatchVariable v t pv in
@@ -2042,7 +2040,6 @@ and type_local_function ctx kind f with_type p =
 	let v = (match v with
 		| None -> None
 		| Some v ->
-			if starts_with v '$' then display_error ctx "Variable names starting with a dollar are not allowed" p;
 			let v = (add_local_with_origin ctx TVOLocalFunction v ft pname) in
 			if params <> [] then v.v_extra <- Some (params,None);
 			Some v

tests/misc/projects/Issue4907/compile-fail.hxml.stderr

@@ -1 +1 @@
-Main.hx:7: characters 18-23 : Capture variables must be lower-case
+Main.hx:7: characters 18-23 : Pattern variables must be lower-case

tests/misc/projects/Issue5843/Macro.hx

@@ -17,7 +17,7 @@ class Macro {
 	
 	static function buildAnon(a:AnonType):Type {
 		var sig = Context.signature(a);
-		var name = sig;
+		var name = "Type" + sig;
 		try return Context.getType(name) catch(e:Dynamic) {}
 		var pos = Context.currentPos();
 		var fields = [];

tests/misc/projects/Issue7921/compile-fail.hxml.stderr

@@ -1 +1 @@
-Main.hx:9: characters 18-19 : Capture variables are not allowed in .match patterns
+Main.hx:9: characters 18-19 : Pattern variables are not allowed in .match patterns

tests/misc/projects/issue5002/Macro.hx

@@ -0,0 +1,18 @@
+import haxe.macro.Context;
+import haxe.macro.Expr;
+
+class Macro {
+	public static function invalidField():Array<Field> {
+		var fields = Context.getBuildFields();
+		function addField(name:String) {
+			fields.push({
+				name: name,
+				kind: FVar(macro:String, null),
+				pos: Context.currentPos()
+			});
+		}
+		addField("0");
+		addField("this");
+		return fields;
+	}
+}

tests/misc/projects/issue5002/Main.hx

@@ -0,0 +1,4 @@
+@:build(Macro.invalidField())
+class Main {
+	static function main() {}
+}

tests/misc/projects/issue5002/Main2.hx

@@ -0,0 +1,107 @@
+import haxe.macro.Context;
+import haxe.macro.Expr;
+
+class Main2 {
+	static function main() {
+		invalidVariable("0_variable");
+		invalidVariable("var");
+		invalidVariable("new");
+		invalidVariable("foo \"\t\n");
+		invalidCatchVariable();
+		invalidFunctionName();
+		invalidFunctionArgumentName();
+		invalidPatternVariable();
+		invalidForVariable();
+		invalidForVariableKeyValue();
+	}
+
+	static macro function invalidVariable(name:String) {
+		return {
+			expr: EVars([{name: name, type: null, expr: null}]),
+			pos: Context.currentPos()
+		};
+	}
+
+	static macro function invalidCatchVariable() {
+		return {
+			expr: ETry(macro {}, [{name: "0_catchVariable", type: macro:Dynamic, expr: macro {}}]),
+			pos: Context.currentPos()
+		};
+	}
+
+	static macro function invalidFunctionName() {
+		return {
+			expr: EFunction(FNamed("0_function", false), {
+				args: [],
+				ret: macro:Void,
+				expr: macro {}
+			}),
+			pos: Context.currentPos()
+		};
+	}
+
+	static macro function invalidFunctionArgumentName() {
+		return {
+			expr: EFunction(FNamed("foo", false), {
+				args: [
+					{
+						name: "0_argument",
+						type: macro:Int
+					}
+				],
+				ret: macro:Void,
+				expr: macro {}
+			}),
+			pos: Context.currentPos()
+		};
+	}
+
+	static macro function invalidPatternVariable() {
+		return {
+			expr: ESwitch(macro "", [
+				{
+					values: [
+						{
+							expr: EConst(CIdent("0_patternVariable")),
+							pos: Context.currentPos()
+						}
+					],
+					expr: macro {}
+				}
+			], null),
+			pos: Context.currentPos()
+		};
+	}
+
+	static macro function invalidForVariable() {
+		return {
+			expr: EFor({
+				expr: EBinop(OpIn, {
+					expr: EConst(CIdent("0_forVariable")),
+					pos: Context.currentPos()
+				}, macro []),
+				pos: Context.currentPos()
+			}, macro {}),
+			pos: Context.currentPos()
+		};
+	}
+
+	static macro function invalidForVariableKeyValue() {
+		return {
+			expr: EFor({
+				expr: EBinop(OpIn, {
+					expr: EBinop(OpArrow, {
+						expr: EConst(CIdent("0_forVariableKey")),
+						pos: Context.currentPos()
+					}, {
+						expr: EConst(CIdent("0_forVariableValue")),
+						pos: Context.currentPos()
+					}),
+					pos: Context.currentPos()
+				}, macro new Map()),
+				pos: Context.currentPos()
+			}, macro {}),
+			pos: Context.currentPos()
+		};
+	}
+}

tests/misc/projects/issue5002/Main3.hx

@@ -0,0 +1,22 @@
+import haxe.macro.Context;
+
+class Main3 {
+	static function init() {
+		function defineType(name, kind) {
+			Context.defineType({
+				pack: [],
+				name: name,
+				pos: (macro 0).pos,
+				kind: kind,
+				fields: []
+			});
+		}
+
+		defineType("lowercase", TDClass());
+
+		defineType("0_class", TDClass());
+		defineType("0_enum", TDEnum);
+		defineType("0_struct", TDStructure);
+		defineType("0_abstract", TDAbstract(TPath({pack: [], name: "Int"})));
+	}
+}

tests/misc/projects/issue5002/Main4.hx

@@ -0,0 +1,13 @@
+class Main4 {
+	static function main() {
+		var $i:Int;
+
+		try {} catch ($i:Int) {}
+
+		for ($i in 0...10) {}
+
+		switch "" {
+			case $i:
+		}
+	}
+}

tests/misc/projects/issue5002/compile-fail.hxml

@@ -0,0 +1 @@
+-main Main

tests/misc/projects/issue5002/compile-fail.hxml.stderr

@@ -0,0 +1,4 @@
+Main.hx:1: characters 1-8 : "0" is not a valid field name
+Main.hx:2: lines 2-4 : Defined in this class
+Main.hx:1: characters 1-8 : "this" is not a valid field name
+Main.hx:2: lines 2-4 : Defined in this class

tests/misc/projects/issue5002/compile2-fail.hxml

@@ -0,0 +1 @@
+-main Main2

tests/misc/projects/issue5002/compile2-fail.hxml.stderr

@@ -0,0 +1,18 @@
+"0_variable" is not a valid variable name
+Main2.hx:6: characters 3-32 : Called from macro here
+"var" is not a valid variable name
+Main2.hx:7: characters 3-25 : Called from macro here
+"new" is not a valid variable name
+Main2.hx:8: characters 3-25 : Called from macro here
+"foo \"\t\n" is not a valid variable name
+Main2.hx:9: characters 3-32 : Called from macro here
+"0_catchVariable" is not a valid catch variable name
+Main2.hx:10: characters 3-25 : Called from macro here
+"0_function" is not a valid function name
+Main2.hx:11: characters 3-24 : Called from macro here
+"0_argument" is not a valid function argument name
+Main2.hx:12: characters 3-32 : Called from macro here
+Main2.hx:13: characters 3-27 : Pattern variables must be lower-case
+Main2.hx:14: characters 3-23 : "0_forVariable" is not a valid for variable name
+Main2.hx:15: characters 3-31 : "0_forVariableKey" is not a valid for variable name
+Main2.hx:15: characters 3-31 : "0_forVariableValue" is not a valid for variable name

tests/misc/projects/issue5002/compile3-fail.hxml

@@ -0,0 +1 @@
+--macro Main3.init()

tests/misc/projects/issue5002/compile3-fail.hxml.stderr

@@ -0,0 +1,6 @@
+Main3.hx:9: characters 17-18 : Type name should start with an uppercase letter
+Main3.hx:9: characters 17-18 : "0_class" is not a valid type name
+Main3.hx:9: characters 17-18 : "0_enum" is not a valid type name
+Main3.hx:9: characters 17-18 : "0_struct" is not a valid type name
+Main3.hx:9: characters 17-18 : "0_abstract" is not a valid type name
+Main3.hx:9: characters 17-18 : "0_abstract_Impl_" is not a valid type name

tests/misc/projects/issue5002/compile4-fail.hxml

@@ -0,0 +1 @@
+-main Main4

tests/misc/projects/issue5002/compile4-fail.hxml.stderr

@@ -0,0 +1,4 @@
+Main4.hx:3: characters 7-9 : Variable names starting with a dollar are not allowed
+Main4.hx:5: characters 17-19 : Catch variable names starting with a dollar are not allowed
+Main4.hx:7: characters 8-10 : For variable names starting with a dollar are not allowed
+Main4.hx:10: characters 9-11 : Pattern variables must be lower-case