From 7063d92e70f240e12a9b3b662ab068883822c67b Mon Sep 17 00:00:00 2001 From: Simon Krajewski Date: Fri, 22 Sep 2017 23:15:28 +0200 Subject: [PATCH 1/4] [matcher] allow and encourage `case var ` (see #6207) --- src/syntax/parser.mly | 6 +++++- src/typing/matcher.ml | 25 +++++++++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/syntax/parser.mly b/src/syntax/parser.mly index 290c011ddb8..f874aa631c7 100644 --- a/src/syntax/parser.mly +++ b/src/syntax/parser.mly @@ -1551,6 +1551,10 @@ and parse_guard = parser | [< '(Kwd If,p1); '(POpen,_); e = expr; '(PClose,_); >] -> e +and expr_or_var = parser + | [< '(Kwd Var,p1); name,p2 = dollar_ident; >] -> EVars [(name,p2),None,None],punion p1 p2 + | [< e = expr >] -> e + and parse_switch_cases eswitch cases = parser | [< '(Kwd Default,p1); '(DblDot,_); s >] -> let b,p2 = (try block_with_pos [] p1 s with Display e -> display (ESwitch (eswitch,cases,Some (Some e,punion p1 (pos e))),punion (pos eswitch) (pos e))) in @@ -1561,7 +1565,7 @@ and parse_switch_cases eswitch cases = parser let l , def = parse_switch_cases eswitch cases s in (match def with None -> () | Some _ -> error Duplicate_default p1); l , Some b - | [< '(Kwd Case,p1); el = psep Comma expr; eg = popt parse_guard; '(DblDot,_); s >] -> + | [< '(Kwd Case,p1); el = psep Comma expr_or_var; eg = popt parse_guard; '(DblDot,_); s >] -> (match el with | [] -> error (Custom "case without a pattern is not allowed") p1 | _ -> diff --git a/src/typing/matcher.ml b/src/typing/matcher.ml index 737c34be1fd..dd68ef453d9 100644 --- a/src/typing/matcher.ml +++ b/src/typing/matcher.ml @@ -160,7 +160,7 @@ module Pattern = struct | TAbstract(a,_) -> unify ctx (TAbstract(a,[mk_mono()])) t p | _ -> assert false - let rec make pctx t e = + let rec make pctx toplevel t e = let ctx = pctx.ctx in let p = pos e in let fail () = @@ -259,6 +259,7 @@ module Pattern = struct if not (is_lower_ident s) && (match s.[0] with '`' | '_' -> false | _ -> true) then begin display_error ctx "Capture variables must be lower-case" p; end; + if toplevel then pctx.ctx.com.warning "case has been deprecated, use case var instead" p; let v = add_local s p in PatVariable v end @@ -313,7 +314,7 @@ module Pattern = struct (* Allow using final _ to match "multiple" arguments *) (PatAny,p) :: (match tl with [] -> [] | _ -> loop el tl) | e :: el,(_,_,t) :: tl -> - make pctx t e :: loop el tl + make pctx false t e :: loop el tl | [],(_,true,t) :: tl -> (PatAny,pos e) :: loop [] tl | [],[] -> @@ -343,7 +344,7 @@ module Pattern = struct | TFun(tl,tr) when tr == fake_tuple_type -> let rec loop el tl = match el,tl with | e :: el,(_,_,t) :: tl -> - let pat = make pctx t e in + let pat = make pctx false t e in pat :: loop el tl | [],[] -> [] | [],_ -> error "Not enough arguments" p @@ -353,7 +354,7 @@ module Pattern = struct PatTuple patterns | TInst({cl_path=[],"Array"},[t2]) | (TDynamic _ as t2) -> let patterns = ExtList.List.mapi (fun i e -> - make pctx t2 e + make pctx false t2 e ) el in PatConstructor(ConArray (List.length patterns),patterns) | _ -> @@ -404,7 +405,7 @@ module Pattern = struct try if pctx.in_reification && cf.cf_name = "pos" then raise Not_found; let e1 = Expr.field_assoc cf.cf_name fl in - make pctx t e1 :: patterns,cf.cf_name :: fields + make pctx false t e1 :: patterns,cf.cf_name :: fields with Not_found -> if is_matchable cf then (PatAny,cf.cf_pos) :: patterns,cf.cf_name :: fields @@ -415,9 +416,9 @@ module Pattern = struct PatConstructor(ConFields fields,patterns) | EBinop(OpOr,e1,e2) -> let pctx1 = {pctx with current_locals = PMap.empty} in - let pat1 = make pctx1 t e1 in + let pat1 = make pctx1 toplevel t e1 in let pctx2 = {pctx with current_locals = PMap.empty; or_locals = Some (pctx1.current_locals)} in - let pat2 = make pctx2 t e2 in + let pat2 = make pctx2 toplevel t e2 in PMap.iter (fun name (v,p) -> if not (PMap.mem name pctx2.current_locals) then verror name p; pctx.current_locals <- PMap.add name (v,p) pctx.current_locals @@ -428,13 +429,13 @@ module Pattern = struct | (EConst (Ident s),p) -> let v = add_local s p in if in_display then ignore(Typer.display_expr ctx e (mk (TLocal v) v.v_type p) (WithType t) p); - let pat = make pctx t e2 in + let pat = make pctx false t e2 in PatBind(v,pat) | (EParenthesis e1,_) -> loop in_display e1 | (EDisplay(e1,_),_) -> loop true e1 | _ -> fail() - in - loop false e1 + in + loop false e1 | EBinop(OpArrow,e1,e2) -> let restore = save_locals ctx in ctx.locals <- pctx.ctx_locals; @@ -442,7 +443,7 @@ module Pattern = struct let e1 = type_expr ctx e1 Value in v.v_name <- "tmp"; restore(); - let pat = make pctx e1.etype e2 in + let pat = make pctx toplevel e1.etype e2 in PatExtractor(v,e1,pat) | EDisplay(e,iscall) -> let pat = loop e in @@ -463,7 +464,7 @@ module Pattern = struct or_locals = None; in_reification = false; } in - make pctx t e + make pctx true t e end module Case = struct From 5ee7b84ebcb668cbda80015d4f94d6fb1cae55a3 Mon Sep 17 00:00:00 2001 From: Simon Krajewski Date: Fri, 22 Sep 2017 23:25:25 +0200 Subject: [PATCH 2/4] [std] add `case var` everywhere --- std/StringTools.hx | 4 ++-- std/cs/_std/sys/io/Process.hx | 2 +- std/haxe/io/Path.hx | 2 +- std/haxe/macro/TypeTools.hx | 2 +- std/haxe/rtti/Rtti.hx | 4 ++-- std/java/_std/sys/io/Process.hx | 4 ++-- std/java/internal/Exceptions.hx | 4 ++-- std/php7/_std/StringTools.hx | 2 +- std/python/_std/Sys.hx | 4 ++-- tests/unit/src/unit/TestMatch.hx | 10 +++++----- tests/unit/src/unit/issues/Issue2914.hx | 8 ++++---- tests/unit/src/unit/issues/Issue3031.hx | 2 +- tests/unit/src/unit/issues/Issue3150.hx | 2 +- tests/unit/src/unit/issues/Issue4677.hx | 2 +- tests/unit/src/unit/issues/Issue5392.hx | 2 +- 15 files changed, 27 insertions(+), 27 deletions(-) diff --git a/std/StringTools.hx b/std/StringTools.hx index 80899bc39d6..4081935b6f3 100644 --- a/std/StringTools.hx +++ b/std/StringTools.hx @@ -93,7 +93,7 @@ class StringTools { ret.addChar(cast c1); ret.addChar(cast c2); } - case chr: + case var chr: ret.addChar(cast chr); } } @@ -548,7 +548,7 @@ class StringTools { result.add(bs); bs_buf = new StringBuf(); result.add('\\"'); - case c: + case var c: // Normal char if (bs_buf.length > 0) { result.add(bs_buf.toString()); diff --git a/std/cs/_std/sys/io/Process.hx b/std/cs/_std/sys/io/Process.hx index 3eb9a59522f..fccb855551d 100644 --- a/std/cs/_std/sys/io/Process.hx +++ b/std/cs/_std/sys/io/Process.hx @@ -63,7 +63,7 @@ class Process { case "Windows": native.StartInfo.FileName = switch (Sys.getEnv("COMSPEC")) { case null: "cmd.exe"; - case comspec: comspec; + case var comspec: comspec; } native.StartInfo.Arguments = '/C "$cmd"'; case _: diff --git a/std/haxe/io/Path.hx b/std/haxe/io/Path.hx index 48ad19c1123..938e2f1535a 100644 --- a/std/haxe/io/Path.hx +++ b/std/haxe/io/Path.hx @@ -235,7 +235,7 @@ class Path { colon = true; case "/".code if (!colon): slashes = true; - case i: + case var i: colon = false; if (slashes) { acc.add("/"); diff --git a/std/haxe/macro/TypeTools.hx b/std/haxe/macro/TypeTools.hx index da48f410fc7..9f2cc3589d3 100644 --- a/std/haxe/macro/TypeTools.hx +++ b/std/haxe/macro/TypeTools.hx @@ -283,7 +283,7 @@ class TypeTools { case TMono(tm): switch(tm.get()) { case null: t; - case t: f(t); + case var t: f(t); } case TEnum(_, []) | TInst(_, []) | TType(_, []): t; diff --git a/std/haxe/rtti/Rtti.hx b/std/haxe/rtti/Rtti.hx index c7c0e432b69..65c8b494840 100644 --- a/std/haxe/rtti/Rtti.hx +++ b/std/haxe/rtti/Rtti.hx @@ -25,7 +25,7 @@ import haxe.rtti.CType; /** Rtti is a helper class which supplements the `@:rtti` metadata. - + @see **/ class Rtti { @@ -47,7 +47,7 @@ class Rtti { var infos = new haxe.rtti.XmlParser().processElement(x); switch (infos) { case TClassdecl(c): return c; - case t: throw 'Enum mismatch: expected TClassDecl but found $t'; + case var t: throw 'Enum mismatch: expected TClassDecl but found $t'; } } diff --git a/std/java/_std/sys/io/Process.hx b/std/java/_std/sys/io/Process.hx index 0095349fbe2..dba301dd8a4 100644 --- a/std/java/_std/sys/io/Process.hx +++ b/std/java/_std/sys/io/Process.hx @@ -46,7 +46,7 @@ class Process { pargs = new NativeArray(3); pargs[0] = cmd = switch (Sys.getEnv("COMSPEC")) { case null: "cmd.exe"; - case comspec: comspec; + case var comspec: comspec; } pargs[1] = '/C'; pargs[2] = '"$cmdStr"'; @@ -102,7 +102,7 @@ class Process { return null; } } - + cast(stdout, ProcessInput).bufferContents(); cast(stderr, ProcessInput).bufferContents(); try diff --git a/std/java/internal/Exceptions.hx b/std/java/internal/Exceptions.hx index 34cd57d152c..d58c2f8d2eb 100644 --- a/std/java/internal/Exceptions.hx +++ b/std/java/internal/Exceptions.hx @@ -73,13 +73,13 @@ class Exceptions { { return "Haxe Exception: " + obj; } - + @:overload override public function getMessage():String { return switch (super.getMessage()) { case null: Std.string(obj); - case message: message; + case var message: message; } } diff --git a/std/php7/_std/StringTools.hx b/std/php7/_std/StringTools.hx index 410b5ec9234..e025c4563df 100644 --- a/std/php7/_std/StringTools.hx +++ b/std/php7/_std/StringTools.hx @@ -166,7 +166,7 @@ import php.*; result.add(bs); bs_buf = new StringBuf(); result.add('\\"'); - case c: + case var c: // Normal char if (bs_buf.length > 0) { result.add(bs_buf.toString()); diff --git a/std/python/_std/Sys.hx b/std/python/_std/Sys.hx index fecb5af10ac..01e0cfd92e0 100644 --- a/std/python/_std/Sys.hx +++ b/std/python/_std/Sys.hx @@ -90,7 +90,7 @@ class Sys { public static function systemName() : String { return switch (python.lib.Sys.platform) { - case x if (StringTools.startsWith(x, "linux")): + case var x if (StringTools.startsWith(x, "linux")): "Linux"; case "darwin": "Mac"; case "win32" | "cygwin" : "Windows"; @@ -142,7 +142,7 @@ class Sys { case "Windows": python.lib.Msvcrt.getch().decode("utf-8").charCodeAt(0); - case x : + case var x : throw "platform " + x + " not supported"; } if (echo) { diff --git a/tests/unit/src/unit/TestMatch.hx b/tests/unit/src/unit/TestMatch.hx index 409a80ea9a9..5313feac878 100644 --- a/tests/unit/src/unit/TestMatch.hx +++ b/tests/unit/src/unit/TestMatch.hx @@ -82,7 +82,7 @@ class TestMatch extends Test { case ["b"]: "2"; case [a]: "3:" + a; case [a, b]: "4:" + a + "," +b; - case a if (a.length == 3): "5:" + a.length; + case var a if (a.length == 3): "5:" + a.length; case []: "6"; case _: "7"; } @@ -125,7 +125,7 @@ class TestMatch extends Test { return switch(cl) { case String: "String"; case MyClass: "unit.MyClass"; - case a: "other: " +Type.getClassName(a); + case var a: "other: " +Type.getClassName(a); } } @@ -231,7 +231,7 @@ class TestMatch extends Test { case val = (4 | 5 | 6) if (val == 5): "1"; case 4, 5, 6: "2"; case 8, 9: "3"; - case x: '_:$x'; + case var x: '_:$x'; } var results = ["_:0", "0", "0", "0", "2", "1", "2", "_:7", "3", "3", "_:10"]; for (i in 0...results.length) { @@ -496,7 +496,7 @@ class TestMatch extends Test { return switch(i) { case [x]: 1; case isPair(_) => Some(p) : p.a+p.b; - case arr: 3; + case var arr: 3; } } @@ -552,7 +552,7 @@ class TestMatch extends Test { case isPair(_) => Some({ a : is(even)(_) => Some(a), b : b }) : a+b; case isPair(_) => Some({ a : isNot(even)(_) => Some(a), b : b }) : a*b; case testArgs(1, "foo", _) => "[99,98,97]": 99; - case arr: 3; + case var arr: 3; } } diff --git a/tests/unit/src/unit/issues/Issue2914.hx b/tests/unit/src/unit/issues/Issue2914.hx index 33cdf92f287..3a5af877ae4 100644 --- a/tests/unit/src/unit/issues/Issue2914.hx +++ b/tests/unit/src/unit/issues/Issue2914.hx @@ -6,18 +6,18 @@ class Issue2914 extends Test { t(unit.HelperMacros.typeError({ var x = switch o { - case x if (x > 1): x; + case var x if (x > 1): x; } })); var x = switch o { - case x if (x > 1): x; - case x: 1; + case var x if (x > 1): x; + case var x: 1; } eq(1, x); switch (o) { - case y if (y < 1): + case var y if (y < 1): x = 5; } eq(5, x); diff --git a/tests/unit/src/unit/issues/Issue3031.hx b/tests/unit/src/unit/issues/Issue3031.hx index e94184605c0..4d7ea99b087 100644 --- a/tests/unit/src/unit/issues/Issue3031.hx +++ b/tests/unit/src/unit/issues/Issue3031.hx @@ -12,7 +12,7 @@ class Issue3031 extends Test { var a = ""; switch (new StringBuf()) { case null: a = "null"; - case notNull: a = "not null"; + case var notNull: a = "not null"; } eq("not null", a); } diff --git a/tests/unit/src/unit/issues/Issue3150.hx b/tests/unit/src/unit/issues/Issue3150.hx index f02b081f426..0c8ffd8aced 100644 --- a/tests/unit/src/unit/issues/Issue3150.hx +++ b/tests/unit/src/unit/issues/Issue3150.hx @@ -5,7 +5,7 @@ class Issue3150 extends Test { t(unit.HelperMacros.typeError( var d = switch (2) { case 3: 90; - case f if (f == 0 | 2): -135; + case var f if (f == 0 | 2): -135; } )); } diff --git a/tests/unit/src/unit/issues/Issue4677.hx b/tests/unit/src/unit/issues/Issue4677.hx index 95152c3bb5d..99308b9061b 100644 --- a/tests/unit/src/unit/issues/Issue4677.hx +++ b/tests/unit/src/unit/issues/Issue4677.hx @@ -3,7 +3,7 @@ package unit.issues; class Issue4677 extends Test { function test() { var x = switch ("foo") { - case _1: _1; + case var _1: _1; } eq("foo", x); } diff --git a/tests/unit/src/unit/issues/Issue5392.hx b/tests/unit/src/unit/issues/Issue5392.hx index 12344700b5e..151222988a5 100644 --- a/tests/unit/src/unit/issues/Issue5392.hx +++ b/tests/unit/src/unit/issues/Issue5392.hx @@ -3,7 +3,7 @@ package unit.issues; private abstract A(Int) from Int { public function size() { return switch (this) { - case size: size; + case var size: size; } } } From 08e7cd49399a3719e48fb7da4126daef8dfd5f86 Mon Sep 17 00:00:00 2001 From: Simon Krajewski Date: Fri, 22 Sep 2017 23:43:19 +0200 Subject: [PATCH 3/4] [matcher] give similarity warnings for plain idents --- src/typing/matcher.ml | 17 ++++++++++++++++- src/typing/type.ml | 9 +++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/typing/matcher.ml b/src/typing/matcher.ml index dd68ef453d9..340186ba5e5 100644 --- a/src/typing/matcher.ml +++ b/src/typing/matcher.ml @@ -259,7 +259,22 @@ module Pattern = struct if not (is_lower_ident s) && (match s.[0] with '`' | '_' -> false | _ -> true) then begin display_error ctx "Capture variables must be lower-case" p; end; - if toplevel then pctx.ctx.com.warning "case has been deprecated, use case var instead" p; + let sl = match follow t with + | TEnum(en,_) -> + en.e_names + | TAbstract({a_impl = Some c} as a,pl) when Meta.has Meta.Enum a.a_meta -> + ExtList.List.filter_map (fun cf -> + if Meta.has Meta.Impl cf.cf_meta && Meta.has Meta.Enum cf.cf_meta then Some cf.cf_name else None + ) c.cl_ordered_statics + | _ -> + [] + in + begin match StringError.get_similar s sl with + | [] -> + if toplevel then + pctx.ctx.com.warning (Printf.sprintf "`case %s` has been deprecated, use `case var %s` instead" s s) p + | l -> pctx.ctx.com.warning ("Potential typo detected (expected similar values are " ^ (String.concat ", " l) ^ "). Consider using `var " ^ s ^ "` instead") p + end; let v = add_local s p in PatVariable v end diff --git a/src/typing/type.ml b/src/typing/type.ml index 795bb5c6f6b..90263fc4b7f 100644 --- a/src/typing/type.ml +++ b/src/typing/type.ml @@ -2848,11 +2848,16 @@ module StringError = struct in loop cl - let string_error_raise s sl msg = - if sl = [] then msg else + let get_similar s sl = + if sl = [] then [] else let cl = List.map (fun s2 -> s2,levenshtein s s2) sl in let cl = List.sort (fun (_,c1) (_,c2) -> compare c1 c2) cl in let cl = filter_similar (fun s2 i -> i <= (min (String.length s) (String.length s2)) / 3) cl in + cl + + let string_error_raise s sl msg = + if sl = [] then msg else + let cl = get_similar s sl in match cl with | [] -> raise Not_found | [s] -> Printf.sprintf "%s (Suggestion: %s)" msg s From 77bc0c5b3661b34c67f2649e9656c64639534fa4 Mon Sep 17 00:00:00 2001 From: Simon Krajewski Date: Sun, 24 Sep 2017 10:47:06 +0200 Subject: [PATCH 4/4] fix tests --- tests/misc/projects/Issue4907/compile-fail.hxml.stderr | 3 ++- tests/misc/projects/Issue5268/Main.hx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/misc/projects/Issue4907/compile-fail.hxml.stderr b/tests/misc/projects/Issue4907/compile-fail.hxml.stderr index c863d79cfd0..bcad934a46f 100644 --- a/tests/misc/projects/Issue4907/compile-fail.hxml.stderr +++ b/tests/misc/projects/Issue4907/compile-fail.hxml.stderr @@ -1 +1,2 @@ -Main.hx:7: characters 18-23 : Capture variables must be lower-case \ No newline at end of file +Main.hx:7: characters 18-23 : Capture variables must be lower-case +Main.hx:7: characters 18-23 : Warning : `case VALUE` has been deprecated, use `case var VALUE` instead \ No newline at end of file diff --git a/tests/misc/projects/Issue5268/Main.hx b/tests/misc/projects/Issue5268/Main.hx index 66446e6b994..a605e2c524f 100644 --- a/tests/misc/projects/Issue5268/Main.hx +++ b/tests/misc/projects/Issue5268/Main.hx @@ -6,7 +6,7 @@ class Main { case Some(v): var value = switch v { case 1: 1; - case m: return None; + case var m: return None; } value; case None: