Move private accessor check to field_access (#12273)

* Move private accessor check to field_access

* maybe like this

* Error on typedef private accessors

* Test unops and fix wrong field positions

pass only `obj.foo` instead of `obj.foo += 1` to `type_field`

* okay, this is good too

* Dont hang on me

* right...

src/context/typecore.ml

@@ -518,11 +518,11 @@ let needs_inline ctx c cf =
 
 (** checks if we can access to a given class field using current context *)
 let can_access ctx c cf ?(check_prop=false) ?(is_setter=false) stat =
-	let is_not_private_prop = not check_prop || match cf.cf_kind with
-		| Var { v_read = AccPrivateCall } when not is_setter -> false
-		| Var { v_write = AccPrivateCall } when is_setter -> false
-		| _ -> true in
-	if (is_not_private_prop && has_class_field_flag cf CfPublic) then
+	let is_private_prop = check_prop && match cf.cf_kind with
+		| Var { v_read = AccPrivateCall } when not is_setter -> true
+		| Var { v_write = AccPrivateCall } when is_setter -> true
+		| _ -> false in
+	if (not is_private_prop && has_class_field_flag cf CfPublic) then
 		true
 	else if c == ctx.c.curclass then
 		true

src/typing/calls.ml

@@ -231,25 +231,6 @@ let rec acc_get ctx g =
 				FieldAccess.get_field_expr fa FRead
 		end
 	| AKAccessor fa ->
-		let c,stat = match fa.fa_host with
-			| FHInstance(c,tl) ->
-				(* c refers to the host of the field, let's find the class we're actually accessing on *)
-				let c = match follow fa.fa_on.etype with
-					| TInst(c,_) -> c
-					| _ -> c
-				in
-				Some c,false
-			| FHStatic c -> Some c,true
-			| FHAbstract(a,tl,c) -> Some c,true
-			| FHAnon -> None,false
-		in
-		begin match c with
-			| Some c ->
-				let can = can_access ctx c fa.fa_field ~check_prop:true ~is_setter:false stat in
-				if not can then
-					raise_typing_error "This property cannot be accessed for reading" fa.fa_pos
-			| _ -> ()
-		end;
 		(dispatcher fa.fa_pos)#field_call fa [] []
 	| AKUsingAccessor sea ->
 		(dispatcher sea.se_access.fa_pos)#field_call sea.se_access [sea.se_this] []

src/typing/fields.ml

@@ -163,13 +163,31 @@ let field_access ctx mode f fh e pfield =
 			AKUsingField sea
 		end;
 	| Var v ->
+		let is_prop_access access =
+			match access with
+				| AccCall | AccPrivateCall -> ctx.e.bypass_accessor = 0
+				| _ -> false
+		in
+		let check_field_with_mode c stat =
+			begin match mode with
+				| MGet when is_prop_access v.v_read ->
+					let can = can_access ctx c f ~check_prop:true ~is_setter:false stat in
+					if not can then
+						raise_typing_error "This property cannot be accessed for reading" pfield
+				| MSet _ when is_prop_access v.v_write ->
+					let can = can_access ctx c f ~check_prop:true ~is_setter:true stat in
+					if not can then
+						raise_typing_error "This property cannot be accessed for writing" pfield
+				| _ -> check_field_access ctx c f stat pfield
+			end
+		in
 		begin match fh with
 		| FHStatic c | FHAbstract(_,_,c) ->
-			check_field_access ctx c f true pfield
+			check_field_with_mode c true;
 		| FHInstance _ ->
 			begin match follow e.etype with
 			| TInst(c,_) ->
-				check_field_access ctx c f false pfield
+				check_field_with_mode c false;
 			| _ ->
 				()
 			end;
@@ -182,7 +200,16 @@ let field_access ctx mode f fh e pfield =
 					display_error ctx.com "Normal variables cannot be accessed with 'super', use 'this' instead" pfield;
 			end;
 		| FHAnon ->
-			()
+			let is_private_access access =
+				access = AccPrivateCall && not (Meta.has Meta.PrivateAccess ctx.f.meta) && ctx.e.bypass_accessor = 0
+			in
+			begin match mode with
+				| MGet when is_private_access v.v_read ->
+					raise_typing_error "This property cannot be accessed for reading" pfield
+				| MSet _ when is_private_access v.v_write ->
+					raise_typing_error "This property cannot be accessed for writing" pfield
+				| _ -> ()
+			end;
 		end;
 		let normal inline =
 			AKField (make_access inline)

src/typing/operators.ml

@@ -598,25 +598,6 @@ let type_assign ctx e1 e2 with_type p =
 		| AKExpr e1  ->
 			assign_to e1
 		| AKAccessor fa ->
-			let c,stat = match fa.fa_host with
-				| FHInstance(c,tl) ->
-					(* c refers to the host of the field, let's find the class we're actually accessing on *)
-					let c = match follow fa.fa_on.etype with
-						| TInst(c,_) -> c
-						| _ -> c
-					in
-					Some c,false
-				| FHStatic c -> Some c,true
-				| FHAbstract(a,tl,c) -> Some c,true
-				| _ -> None,false
-			in
-			begin match c with
-				| Some c ->
-					let can = can_access ctx c fa.fa_field ~check_prop:true ~is_setter:true stat in
-					if not can then
-						raise_typing_error "This property cannot be accessed for writing" fa.fa_pos
-				| _ -> ()
-			end;
 			let dispatcher = new call_dispatcher ctx (MSet (Some e2)) with_type p in
 			dispatcher#accessor_call fa [] [e2]
 		| AKAccess(a,tl,c,ebase,ekey) ->
@@ -696,7 +677,8 @@ type 'a assign_op_api = {
 
 let handle_assign_op ctx api e1 e2 with_type p =
 	let field_rhs_by_name name ev with_type =
-		let access_get = type_field_default_cfg ctx ev name p MGet with_type in
+		let field_pos = snd e1 in
+		let access_get = type_field_default_cfg ctx ev name field_pos MGet with_type in
 		let e_get = acc_get ctx access_get in
 		e_get,api.type_rhs e_get e2
 	in
@@ -963,7 +945,8 @@ let type_unop ctx op flag e with_type p =
 				e_lhs,None
 		in
 		let read_on vr ef fa =
-			let access_get = type_field_default_cfg ctx ef fa.fa_field.cf_name p MGet WithType.value in
+			let field_pos = snd e in
+			let access_get = type_field_default_cfg ctx ef fa.fa_field.cf_name field_pos MGet WithType.value in
 			let e_lhs = acc_get ctx access_get in
 			let e_lhs,e_out = maybe_tempvar_postfix vr e_lhs in
 			e_lhs,e_out
@@ -988,7 +971,8 @@ let type_unop ctx op flag e with_type p =
 			| AKField fa ->
 				let vr = new value_reference ctx in
 				let ef = vr#get_expr_part "fh" fa.fa_on in
-				let access_get = type_field_default_cfg ctx ef fa.fa_field.cf_name p MGet WithType.value in
+				let field_pos = snd e in
+				let access_get = type_field_default_cfg ctx ef fa.fa_field.cf_name field_pos MGet WithType.value in
 				let e,e_out = match access_get with
 				| AKField _ ->
 					let e = FieldAccess.get_field_expr {fa with fa_on = ef} FGet in

tests/misc/projects/Issue3053/Main.hx

@@ -35,14 +35,8 @@ class Main {
 	}
 
 	public var notAllowed(private get, private set):Int; // err
-
-	function set_notAllowed(value:Int):Int {
-		throw new haxe.exceptions.NotImplementedException();
-	}
-
-	function get_notAllowed():Int {
-		throw new haxe.exceptions.NotImplementedException();
-	}
+	function set_notAllowed(v):Int return 0;
+	function get_notAllowed():Int return 0;
 
 	public function new() {
 		foo = 1;
@@ -110,14 +104,8 @@ class Rect implements Shape {
 
 	public function new() {}
 	public var width(get, private set):Int; // err
-
-	function set_width(value:Int):Int {
-		return 0;
-	}
-
-	function get_width():Int {
-		return 0;
-	}
+	function set_width(v):Int return 0;
+	function get_width():Int return 0;
 }
 
 @:build(PropertyMacro.addIntProperty("age"))
@@ -129,9 +117,7 @@ class Bar {
 	public var defaultNull(default, null):Int;
 
 	public var defaultPrivateSet(default, private set):Int;
-	function set_defaultPrivateSet(value:Int):Int {
-		return value;
-	}
+	function set_defaultPrivateSet(v):Int return v;
 
 	var width(private get, private set):Int;
 
@@ -146,16 +132,38 @@ class Bar {
 
 class Parent {
 	var width(private get, private set):Int;
-	function set_width(value:Int):Int {
-		return 0;
-	}
-	function get_width():Int {
-		return 0;
-	}
+	function set_width(v):Int return 0;
+	function get_width():Int return 0;
 }
 
 class Child extends Parent {
 	public function new() {
 		width = 0;
+		super.width;
+		super.width = 0;
+	}
+}
+
+@:access(Element)
+class MainElement {
+	public static function main() {
+		new Element().foo;
+		new Element().fooSet = false;
 	}
 }
+
+class Element extends Entity {}
+
+class Entity {
+	function new() {}
+
+	var foo(private get, never):Bool;
+
+	function get_foo():Bool
+		return true;
+
+	var fooSet(default, private set):Bool;
+
+	function set_fooSet(v):Bool
+		return fooSet = v;
+}

tests/misc/projects/Issue3053/Main2.hx

@@ -0,0 +1,129 @@
+typedef PrivateVariationsType = {
+	var privateBoth(private get, private set):Int;
+	var privateGetSet(private get, set):Int;
+	var getPrivateSet(get, private set):Int;
+	var defaultPrivateSet(default, private set):Int;
+	var getNever(get, never):Int;
+}
+
+class PrivateVariations {
+	public function new() {}
+	var privateBoth(private get, private set):Int;
+	public var privateGetSet(private get, set):Int;
+	public var getPrivateSet(get, private set):Int;
+	public var defaultPrivateSet(default, private set):Int;
+
+	static var sprivateBoth(private get, private set):Int;
+	static public var sprivateGetSet(private get, set):Int;
+	static public var sgetPrivateSet(get, private set):Int;
+	static public var sdefaultPrivateSet(default, private set):Int;
+
+	function set_privateBoth(v):Int return 0;
+	function get_privateBoth():Int return 0;
+	function set_privateGetSet(v):Int return 0;
+	function get_privateGetSet():Int return 0;
+	function set_getPrivateSet(v):Int return 0;
+	function get_getPrivateSet():Int return 0;
+	function set_defaultPrivateSet(v):Int return 0;
+
+	static function set_sprivateBoth(v):Int return 0;
+	static function get_sprivateBoth():Int return 0;
+	static function set_sprivateGetSet(v):Int return 0;
+	static function get_sprivateGetSet():Int return 0;
+	static function set_sgetPrivateSet(v):Int return 0;
+	static function get_sgetPrivateSet():Int return 0;
+	static function set_sdefaultPrivateSet(v):Int return 0;
+}
+
+class CheckVariations {
+	static function main() {
+		final type:PrivateVariationsType = null;
+		type.getNever = 1; // err
+		type.getPrivateSet;
+		type.getPrivateSet = 1; // err
+		type.getPrivateSet += 1; // err
+		type.getPrivateSet++; // err
+		type.privateGetSet; // err
+		type.privateGetSet = 1;
+		type.privateGetSet += 1; // err (reading)
+		type.privateGetSet++; // err (reading)
+		type.privateBoth; // err
+		type.privateBoth = 1; // err
+		type.privateBoth += 1; // err
+		type.privateBoth++; // err
+		@:privateAccess type.privateBoth;
+		@:privateAccess type.privateBoth = 1;
+		@:privateAccess type.privateBoth += 1;
+		@:privateAccess type.privateBoth++;
+		@:bypassAccessor type.privateBoth; // err
+		@:bypassAccessor type.privateBoth = 1; // err
+		@:bypassAccessor type.privateBoth += 1; // err
+		@:bypassAccessor type.privateBoth++; // err
+
+		type.defaultPrivateSet;
+		type.defaultPrivateSet = 1; // err
+		type.defaultPrivateSet += 1; // err
+		type.defaultPrivateSet++; // err
+		@:privateAccess type.defaultPrivateSet = 1;
+		@:privateAccess type.defaultPrivateSet += 1;
+		@:privateAccess type.defaultPrivateSet++;
+		@:bypassAccessor type.defaultPrivateSet = 1;
+		@:bypassAccessor type.defaultPrivateSet += 1;
+		@:bypassAccessor type.defaultPrivateSet++;
+
+		final vars = new PrivateVariations();
+		vars.getPrivateSet;
+		vars.getPrivateSet = 1; // err
+		vars.getPrivateSet += 1; // err
+		vars.getPrivateSet++; // err
+		vars.privateGetSet; // err
+		vars.privateGetSet = 1;
+		vars.privateGetSet += 1; // err (reading)
+		vars.privateGetSet++; // err (reading)
+		vars.privateBoth; // err
+		vars.privateBoth = 1; // err
+		vars.privateBoth += 1; // err
+		vars.privateBoth++; // err
+		@:privateAccess vars.privateBoth;
+		@:privateAccess vars.privateBoth = 1;
+		@:privateAccess vars.privateBoth += 1;
+		@:privateAccess vars.privateBoth++;
+		vars.defaultPrivateSet;
+		vars.defaultPrivateSet = 1; // err
+		vars.defaultPrivateSet += 1; // err
+		vars.defaultPrivateSet++; // err
+		@:privateAccess vars.defaultPrivateSet = 1;
+		@:privateAccess vars.defaultPrivateSet += 1;
+		@:privateAccess vars.defaultPrivateSet++;
+		@:bypassAccessor vars.defaultPrivateSet = 1;
+		@:bypassAccessor vars.defaultPrivateSet += 1;
+		@:bypassAccessor vars.defaultPrivateSet++;
+
+		PrivateVariations.sgetPrivateSet;
+		PrivateVariations.sgetPrivateSet = 1; // err
+		PrivateVariations.sgetPrivateSet += 1; // err
+		PrivateVariations.sgetPrivateSet++; // err
+		PrivateVariations.sprivateGetSet; // err
+		PrivateVariations.sprivateGetSet = 1;
+		PrivateVariations.sprivateGetSet += 1; // err (reading)
+		PrivateVariations.sprivateGetSet++; // err (reading)
+		PrivateVariations.sprivateBoth; // err
+		PrivateVariations.sprivateBoth = 1; // err
+		PrivateVariations.sprivateBoth += 1; // err
+		PrivateVariations.sprivateBoth++; // err
+		@:privateAccess PrivateVariations.sprivateBoth;
+		@:privateAccess PrivateVariations.sprivateBoth = 1;
+		@:privateAccess PrivateVariations.sprivateBoth += 1;
+		@:privateAccess PrivateVariations.sprivateBoth++;
+		PrivateVariations.sdefaultPrivateSet;
+		PrivateVariations.sdefaultPrivateSet = 1; // err
+		PrivateVariations.sdefaultPrivateSet += 1; // err
+		PrivateVariations.sdefaultPrivateSet++; // err
+		@:privateAccess PrivateVariations.sdefaultPrivateSet = 1;
+		@:privateAccess PrivateVariations.sdefaultPrivateSet += 1;
+		@:privateAccess PrivateVariations.sdefaultPrivateSet++;
+		@:bypassAccessor PrivateVariations.sdefaultPrivateSet = 1;
+		@:bypassAccessor PrivateVariations.sdefaultPrivateSet += 1;
+		@:bypassAccessor PrivateVariations.sdefaultPrivateSet++;
+	}
+}

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

@@ -1,17 +1,15 @@
 Main.hx:37: characters 2-54 : notAllowed: (private get, private set) property cannot be public
-Main.hx:53: characters 3-17 : This property cannot be accessed for writing
-Main.hx:56: characters 3-13 : This property cannot be accessed for writing
-Main.hx:61: characters 3-14 : This property cannot be accessed for reading
-Main.hx:65: characters 3-24 : This property cannot be accessed for writing
-Main.hx:70: characters 7-12 : Cannot access private field width
-Main.hx:70: characters 3-12 : This property cannot be accessed for writing
-Main.hx:71: characters 7-12 : Cannot access private field width
-Main.hx:71: characters 3-12 : This property cannot be accessed for reading
-Main.hx:73: characters 7-18 : This expression cannot be accessed for writing
-Main.hx:78: characters 3-10 : This property cannot be accessed for writing
+Main.hx:47: characters 8-17 : This property cannot be accessed for writing
+Main.hx:50: characters 8-13 : This property cannot be accessed for writing
+Main.hx:55: characters 9-14 : This property cannot be accessed for reading
+Main.hx:59: characters 7-24 : This property cannot be accessed for writing
+Main.hx:64: characters 7-12 : This property cannot be accessed for writing
+Main.hx:65: characters 7-12 : This property cannot be accessed for reading
+Main.hx:67: characters 7-18 : This expression cannot be accessed for writing
+Main.hx:72: characters 7-10 : This property cannot be accessed for writing
 Main.hx:21: characters 3-26 : Main should be FooType
 Main.hx:21: characters 3-26 : ... Inconsistent getter for field foo : private get should be get
 Main.hx:24: characters 3-19 : FooPrivateGetType should be FooType
 Main.hx:24: characters 3-19 : ... Inconsistent getter for field foo : private get should be get
-Main.hx:112: characters 13-18 : Field width has different property access than in Shape: (get,private set) should be (private get,set)
-Main.hx:94: characters 13-18 : Field width has different property access than in PublicShape: (get,private set) should be (get,set)
+Main.hx:106: characters 13-18 : Field width has different property access than in Shape: (get,private set) should be (private get,set)
+Main.hx:88: characters 13-18 : Field width has different property access than in PublicShape: (get,private set) should be (get,set)

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

@@ -0,0 +1,3 @@
+--main Main2
+--interp
+-D message.reporting=classic

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

@@ -0,0 +1,50 @@
+Main2.hx:41: characters 8-16 : This expression cannot be accessed for writing
+Main2.hx:43: characters 8-21 : This property cannot be accessed for writing
+Main2.hx:44: characters 8-21 : This property cannot be accessed for writing
+Main2.hx:45: characters 8-21 : This property cannot be accessed for writing
+Main2.hx:46: characters 8-21 : This property cannot be accessed for reading
+Main2.hx:48: characters 8-21 : This property cannot be accessed for reading
+Main2.hx:49: characters 8-21 : This property cannot be accessed for reading
+Main2.hx:50: characters 8-19 : This property cannot be accessed for reading
+Main2.hx:51: characters 8-19 : This property cannot be accessed for writing
+Main2.hx:52: characters 8-19 : This property cannot be accessed for writing
+Main2.hx:53: characters 8-19 : This property cannot be accessed for writing
+Main2.hx:58: characters 25-36 : This field cannot be accessed because it is not a real variable
+Main2.hx:2: characters 2-48 : Add @:isVar here to enable it
+Main2.hx:59: characters 25-36 : This field cannot be accessed because it is not a real variable
+Main2.hx:2: characters 2-48 : Add @:isVar here to enable it
+Main2.hx:60: characters 25-36 : This field cannot be accessed because it is not a real variable
+Main2.hx:2: characters 2-48 : Add @:isVar here to enable it
+Main2.hx:60: characters 25-36 : This property cannot be accessed for reading
+Main2.hx:61: characters 25-36 : This field cannot be accessed because it is not a real variable
+Main2.hx:2: characters 2-48 : Add @:isVar here to enable it
+Main2.hx:61: characters 25-36 : This property cannot be accessed for reading
+Main2.hx:64: characters 8-25 : This property cannot be accessed for writing
+Main2.hx:65: characters 8-25 : This property cannot be accessed for writing
+Main2.hx:66: characters 8-25 : This property cannot be accessed for writing
+Main2.hx:76: characters 8-21 : This property cannot be accessed for writing
+Main2.hx:77: characters 8-21 : This property cannot be accessed for writing
+Main2.hx:78: characters 8-21 : This property cannot be accessed for writing
+Main2.hx:79: characters 8-21 : This property cannot be accessed for reading
+Main2.hx:81: characters 8-21 : This property cannot be accessed for reading
+Main2.hx:82: characters 8-21 : This property cannot be accessed for reading
+Main2.hx:83: characters 8-19 : This property cannot be accessed for reading
+Main2.hx:84: characters 8-19 : This property cannot be accessed for writing
+Main2.hx:85: characters 8-19 : This property cannot be accessed for writing
+Main2.hx:86: characters 8-19 : This property cannot be accessed for writing
+Main2.hx:92: characters 8-25 : This property cannot be accessed for writing
+Main2.hx:93: characters 8-25 : This property cannot be accessed for writing
+Main2.hx:94: characters 8-25 : This property cannot be accessed for writing
+Main2.hx:103: characters 21-35 : This property cannot be accessed for writing
+Main2.hx:104: characters 21-35 : This property cannot be accessed for writing
+Main2.hx:105: characters 21-35 : This property cannot be accessed for writing
+Main2.hx:106: characters 21-35 : This property cannot be accessed for reading
+Main2.hx:108: characters 21-35 : This property cannot be accessed for reading
+Main2.hx:109: characters 21-35 : This property cannot be accessed for reading
+Main2.hx:110: characters 21-33 : This property cannot be accessed for reading
+Main2.hx:111: characters 21-33 : This property cannot be accessed for writing
+Main2.hx:112: characters 21-33 : This property cannot be accessed for writing
+Main2.hx:113: characters 21-33 : This property cannot be accessed for writing
+Main2.hx:119: characters 21-39 : This property cannot be accessed for writing
+Main2.hx:120: characters 21-39 : This property cannot be accessed for writing
+Main2.hx:121: characters 21-39 : This property cannot be accessed for writing