Skip to content

Commit

Permalink
Better error messages for property-related syntax errors
Browse files Browse the repository at this point in the history
Closes #11280
  • Loading branch information
kLabz committed Sep 15, 2023
1 parent 0920ed7 commit 0972725
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 1 deletion.
5 changes: 5 additions & 0 deletions src/syntax/grammar.mly
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,9 @@ and parse_class_field tdecl s =
| [< '(Kwd Final,p1) >] ->
check_redundant_var p1 s;
begin match s with parser
| [< opt,name = questionable_dollar_ident; '(POpen,_); i1 = property_ident; '(Comma,_); i2 = property_ident; '(PClose,_); t = popt parse_type_hint; e,p2 = parse_var_field_assignment >] ->
let meta = check_optional opt name in
name,punion p1 p2,FProp(i1,i2,t,e),(al @ [AFinal,p1]),meta
| [< opt,name = questionable_dollar_ident; t = popt parse_type_hint; e,p2 = parse_var_field_assignment >] ->
let meta = check_optional opt name in
name,punion p1 p2,FVar(t,e),(al @ [AFinal,p1]),meta
Expand Down Expand Up @@ -1235,6 +1238,8 @@ and parse_array_decl p1 s =
and parse_var_decl_head final s =
let meta = parse_meta s in
match s with parser
| [< name, p = dollar_ident; '(POpen,p1); _ = property_ident; '(Comma,_); _ = property_ident; '(PClose,p2); t = popt parse_type_hint >] ->
syntax_error (Custom "Cannot define property accessors for local vars") ~pos:(Some (punion p1 p2)) s (meta,name,final,t,p)
| [< name, p = dollar_ident; t = popt parse_type_hint >] -> (meta,name,final,t,p)
| [< >] ->
(* This nonsense is here for the var @ case in issue #9639 *)
Expand Down
6 changes: 5 additions & 1 deletion src/typing/typeloadFields.ml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ module FieldError = struct
let invalid_modifier_only com fctx m c p =
maybe_display_error com fctx (Printf.sprintf "Invalid modifier: %s is only supported %s" m c) p

let invalid_modifier_on_property com fctx m p =
maybe_display_error com fctx (Printf.sprintf "Invalid modifier: %s is not supported on properties" m) p

let missing_expression com fctx reason p =
maybe_display_error com fctx (Printf.sprintf "%s" reason) p

Expand Down Expand Up @@ -1631,9 +1634,10 @@ let init_field (ctx,cctx,fctx) f =
if not (has_class_flag c CExtern) && not (Meta.has Meta.Native f.cff_meta) then Typecore.check_field_name ctx name p;
List.iter (fun acc ->
match (fst acc, f.cff_kind) with
| APublic, _ | APrivate, _ | AStatic, _ | AFinal, _ | AExtern, _ -> ()
| APublic, _ | APrivate, _ | AStatic, _ | AFinal, FVar _ | AFinal, FFun _ | AExtern, _ -> ()
| ADynamic, FFun _ | AOverride, FFun _ | AMacro, FFun _ | AInline, FFun _ | AInline, FVar _ | AAbstract, FFun _ | AOverload, FFun _ -> ()
| AEnum, (FVar _ | FProp _) -> ()
| AFinal, FProp _ -> invalid_modifier_on_property ctx.com fctx (Ast.s_placed_access acc) (snd acc)
| _, FVar _ -> display_error ctx.com ("Invalid accessor '" ^ Ast.s_placed_access acc ^ "' for variable " ^ name) (snd acc)
| _, FProp _ -> display_error ctx.com ("Invalid accessor '" ^ Ast.s_placed_access acc ^ "' for property " ^ name) (snd acc)
| _, FFun _ -> display_error ctx.com ("Invalid accessor '" ^ Ast.s_placed_access acc ^ "' for function " ^ name) (snd acc)
Expand Down
3 changes: 3 additions & 0 deletions tests/misc/Issue11280/Main.hx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
function main() {
var foo(get, never):Int;
}
6 changes: 6 additions & 0 deletions tests/misc/Issue11280/Main2.hx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class Main {
public final bar(get, never):Int;
function get_bar():Int return 42;

static function main() {}
}
3 changes: 3 additions & 0 deletions tests/misc/Issue11280/compile-fail.hxml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-main Main
-D message.reporting=pretty
-D message.no-color
6 changes: 6 additions & 0 deletions tests/misc/Issue11280/compile-fail.hxml.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[ERROR] Main.hx:2: characters 9-21

2 | var foo(get, never):Int;
| ^^^^^^^^^^^^
| Cannot define property accessors for local vars

3 changes: 3 additions & 0 deletions tests/misc/Issue11280/compile2-fail.hxml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-main Main2
-D message.reporting=pretty
-D message.no-color
6 changes: 6 additions & 0 deletions tests/misc/Issue11280/compile2-fail.hxml.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[ERROR] Main2.hx:2: characters 9-14

2 | public final bar(get, never):Int;
| ^^^^^
| Invalid modifier: final is not supported on properties

2 comments on commit 0972725

@Simn
Copy link
Member

@Simn Simn commented on 0972725 Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that this added tests/misc/Issue11280 which I don't think is picked up by anyone.

@kLabz
Copy link
Contributor Author

@kLabz kLabz commented on 0972725 Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhhh not sure how that happened :/

Please sign in to comment.