Skip to content

Commit

Permalink
feat: only allow identifiers as string interpolation placeholders
Browse files Browse the repository at this point in the history
  • Loading branch information
eWert-Online committed Jan 13, 2024
1 parent c45e1ce commit 78c6dfa
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 21 deletions.
7 changes: 5 additions & 2 deletions lib/pinc_backend/Pinc_Interpreter.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()))
Expand Down
2 changes: 1 addition & 1 deletion lib/pinc_frontend/Pinc_Ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ and string_template = {
}

and string_template_desc =
| StringInterpolation of expression
| StringInterpolation of lowercase_identifier
| StringText of string

and expression = {
Expand Down
38 changes: 24 additions & 14 deletions lib/pinc_frontend/Pinc_Parser.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 =
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/context.t/SubComponent.pi
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
component SubComponent {
let background = #GetContext;

<div class="Sub $(if (background == "grey") "Sub--grey" else "")">
let backgroundClass = if (background == "grey") "Sub--grey" else "";
<div class="Sub $(backgroundClass)">
<p>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.</p>
</div>
}
13 changes: 13 additions & 0 deletions test/parse_error.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 │ <div class="Section $(if fail "A" else "B")">
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]
7 changes: 7 additions & 0 deletions test/parse_error.t/string_interpolation.pi
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
component Component {
let fail = #String

<div class="Section $(if fail "A" else "B")">
{fail}
</div>
}
5 changes: 2 additions & 3 deletions test/template.t/data.pi
Original file line number Diff line number Diff line change
Expand Up @@ -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)!";

<section class="Section">
<div />
Expand Down

0 comments on commit 78c6dfa

Please sign in to comment.