From 78c6dfa88601b7215c4ed4c0392d86a20e78e7bc Mon Sep 17 00:00:00 2001 From: Torben Ewert Date: Sat, 13 Jan 2024 19:41:22 +0100 Subject: [PATCH] feat: only allow identifiers as string interpolation placeholders --- lib/pinc_backend/Pinc_Interpreter.ml | 7 ++-- lib/pinc_frontend/Pinc_Ast.ml | 2 +- lib/pinc_frontend/Pinc_Parser.ml | 38 ++++++++++++++-------- test/context.t/SubComponent.pi | 3 +- test/parse_error.t/run.t | 13 ++++++++ test/parse_error.t/string_interpolation.pi | 7 ++++ test/template.t/data.pi | 5 ++- 7 files changed, 54 insertions(+), 21 deletions(-) create mode 100644 test/parse_error.t/string_interpolation.pi diff --git a/lib/pinc_backend/Pinc_Interpreter.ml b/lib/pinc_backend/Pinc_Interpreter.ml index 4fadc53..3f26654 100644 --- a/lib/pinc_backend/Pinc_Interpreter.ml +++ b/lib/pinc_backend/Pinc_Interpreter.ml @@ -567,8 +567,11 @@ and eval_string_template ~state template = match string_template.Ast.string_template_desc with | StringText s -> s - | StringInterpolation e -> - eval_expression ~state e |> State.get_output |> Value.to_string) + | StringInterpolation (Lowercase_Id (id, loc)) -> + id + |> eval_lowercase_identifier ~loc ~state + |> State.get_output + |> Value.to_string) |> String.concat "" |> Value.of_string ~value_loc:(Location.make ~s:!start_loc.loc_start ~e:!end_loc.loc_end ())) diff --git a/lib/pinc_frontend/Pinc_Ast.ml b/lib/pinc_frontend/Pinc_Ast.ml index cdacbd5..8f2374d 100644 --- a/lib/pinc_frontend/Pinc_Ast.ml +++ b/lib/pinc_frontend/Pinc_Ast.ml @@ -54,7 +54,7 @@ and string_template = { } and string_template_desc = - | StringInterpolation of expression + | StringInterpolation of lowercase_identifier | StringText of string and expression = { diff --git a/lib/pinc_frontend/Pinc_Parser.ml b/lib/pinc_frontend/Pinc_Parser.ml index fdb77e7..93c1593 100644 --- a/lib/pinc_frontend/Pinc_Parser.ml +++ b/lib/pinc_frontend/Pinc_Parser.ml @@ -64,7 +64,7 @@ let make ~filename src = ;; module Helpers = struct - let expect_identifier ?(typ = `All) t = + let expect_identifier ?error_message ?(typ = `All) t = let location = Location.make ~s:t.token.location.loc_start ~e:t.token.location.loc_end () in @@ -78,36 +78,37 @@ module Helpers = struct | token when typ = `Lower -> Diagnostics.error location - (match token with - | Token.IDENT_UPPER i -> + (match (error_message, token) with + | Some message, _ -> message + | None, Token.IDENT_UPPER i -> "Expected to see a lowercase identifier at this point. Did you mean " ^ String.lowercase_ascii i ^ " instead of " ^ i ^ "?" - | t when Token.is_keyword t -> - "`" ^ Token.to_string t ^ "` is a keyword. Please choose another name." - | t -> + | None, t -> "Expected to see a lowercase identifier at this point. Instead saw " ^ Token.to_string t) | token when typ = `Upper -> Diagnostics.error location - (match token with - | Token.IDENT_LOWER i -> + (match (error_message, token) with + | Some message, _ -> message + | None, Token.IDENT_LOWER i -> "Expected to see an uppercase identifier at this point. Did you mean " ^ String.capitalize_ascii i ^ " instead of " ^ i ^ "?" - | _ -> "Expected to see an uppercase identifier at this point.") + | None, _ -> "Expected to see an uppercase identifier at this point.") | token -> Diagnostics.error location - (match token with - | t when Token.is_keyword t -> + (match (error_message, token) with + | Some message, _ -> message + | None, t when Token.is_keyword t -> "\"" ^ Token.to_string t ^ "\" is a keyword. Please choose another name." - | _ -> "Expected to see an identifier at this point.") + | None, _ -> "Expected to see an identifier at this point.") ;; let separated_list ~sep ~fn t = @@ -146,9 +147,18 @@ module Rules = struct Some (Ast.StringText s) | Token.OPEN_TEMPLATE_LITERAL -> next t; - let* expression = parse_expression t in + let identifier = + t + |> Helpers.expect_identifier + ~typ:`Lower + ~error_message: + "Only lowercase identifiers are allowed as string interpolation \ + placeholders.\n\ + If you want to use more complex constructs, you can always assign \ + them to a let binding." + in t |> expect Token.RIGHT_PAREN; - Some (Ast.StringInterpolation expression) + Some (Ast.StringInterpolation (Lowercase_Id identifier)) | _ -> None in let string_template_end = t.token.location.loc_end in diff --git a/test/context.t/SubComponent.pi b/test/context.t/SubComponent.pi index 30ee5f4..748a987 100644 --- a/test/context.t/SubComponent.pi +++ b/test/context.t/SubComponent.pi @@ -1,7 +1,8 @@ component SubComponent { let background = #GetContext; -
+ let backgroundClass = if (background == "grey") "Sub--grey" else ""; +

Faucibus mus velit ut iaculis cubilia taciti conubia, ad magna sem mattis tincidunt eleifend, egestas eget neque curae molestie donec. Magna ipsum netus euismod vitae justo dictum adipiscing lobortis, posuere tortor penatibus tempus laoreet habitant orci, turpis blandit nullam ornare fames nostra sed.

} diff --git a/test/parse_error.t/run.t b/test/parse_error.t/run.t index dc4af36..e0222d5 100644 --- a/test/parse_error.t/run.t +++ b/test/parse_error.t/run.t @@ -34,6 +34,7 @@ Expected expression as right hand side of let declaration [1] + $ print ./mutation_without_expression.pi Component ERROR in file ./mutation_without_expression.pi:3:3-12 @@ -44,3 +45,15 @@ Expected expression as right hand side of mutation statement [1] + + $ print ./string_interpolation.pi Component + + ERROR in file ./string_interpolation.pi:4:25-27 + + 3 │ + 4 │
+ 5 │ {fail} + + Only lowercase identifiers are allowed as string interpolation placeholders. + If you want to use more complex constructs, you can always assign them to a let binding. + [1] diff --git a/test/parse_error.t/string_interpolation.pi b/test/parse_error.t/string_interpolation.pi new file mode 100644 index 0000000..43f5f83 --- /dev/null +++ b/test/parse_error.t/string_interpolation.pi @@ -0,0 +1,7 @@ +component Component { + let fail = #String + +
+ {fail} +
+} diff --git a/test/template.t/data.pi b/test/template.t/data.pi index 8c6f39e..bafe75f 100644 --- a/test/template.t/data.pi +++ b/test/template.t/data.pi @@ -3,10 +3,9 @@ component Component( icon: "/images/icons/group.svg", group: "Structure", ) { - - let name = "String Template"; let intro = true; - let content = "Hello, $(if (intro) { name } else { "World" })!"; + let name = if (intro) "String Template" else "World"; + let content = "Hello, $(name)!";