Fix error line number coming from parser (#478)

davesnx · Jun 20, 2024 · ce38506 · ce38506
1 parent 7a6a739
commit ce38506
Show file tree

Hide file tree

Showing 18 changed files with 216 additions and 74 deletions.
diff --git a/packages/parser/lib/Parser_location.ml b/packages/parser/lib/Parser_location.ml
@@ -45,3 +45,18 @@ let update_loc_with_delimiter (loc : Ppxlib.location) delimiter :
     { loc.loc_start with pos_cnum = loc.loc_start.pos_cnum + offset }
   and loc_end = { loc.loc_end with pos_cnum = loc.loc_end.pos_cnum + offset } in
   { loc_start; loc_end; loc_ghost = false }
+
+let update_pos_lnum (a : Ppxlib.location) (b : Ppxlib.location) =
+  let loc_start =
+    {
+      a.loc_start with
+      pos_lnum = a.loc_start.pos_lnum + b.loc_start.pos_lnum - 1;
+    }
+  in
+  let loc_end =
+    {
+      a.loc_end with
+      pos_lnum = a.loc_end.pos_lnum + b.loc_start.pos_lnum - 1;
+    }
+  in
+  { a with loc_start; loc_end }
diff --git a/packages/parser/lib/driver.re b/packages/parser/lib/driver.re
@@ -4,7 +4,7 @@ module Location = Ppxlib.Location;
 
 let menhir = MenhirLib.Convert.Simplified.traditional2revised;
 
-let parse = (skip_whitespaces, lexbuf, parser) => {
+let parse = (~loc: Ppxlib.location, skip_whitespaces, lexbuf, parser) => {
   Lexer.skip_whitespace.contents = skip_whitespaces;
 
   let last_token = ref((Parser.EOF, Lexing.dummy_pos, Lexing.dummy_pos));
@@ -20,7 +20,8 @@ let parse = (skip_whitespaces, lexbuf, parser) => {
     Error((loc, msg));
   | _ =>
     let (token, start_pos, end_pos) = last_token^;
-    let loc = Parser_location.to_ppxlib_location(start_pos, end_pos);
+    let token_loc = Parser_location.to_ppxlib_location(start_pos, end_pos);
+    let loc = Parser_location.update_pos_lnum(token_loc, loc)
     let msg =
       Printf.sprintf(
         "Parse error while reading token '%s'",
@@ -32,11 +33,10 @@ let parse = (skip_whitespaces, lexbuf, parser) => {
 
 let last_buffer = ref(None);
 
-let parse_string =
-    (~skip_whitespace, ~loc as _: Ppxlib.location, parser, string) => {
+let parse_string = (~skip_whitespace, ~loc, parser, string) => {
   let buffer = Sedlexing.Latin1.from_string(string);
   last_buffer := Some(Sedlexing.Latin1.from_string(string));
-  parse(skip_whitespace, buffer, parser);
+  parse(~loc, skip_whitespace, buffer, parser);
 };
 
 let parse_declaration_list = (~loc, input: string) => {

diff --git a/packages/parser/test/css_parser_test.re b/packages/parser/test/css_parser_test.re
@@ -10,7 +10,7 @@ let parse = input => {
     open Styled_ppx_css_parser.Ast;
     let pos = loc.loc_start;
     let curr_pos = pos.pos_cnum;
-    let lnum = pos.pos_lnum;
+    let lnum = pos.pos_lnum + 1;
     let pos_bol = pos.pos_bol;
     let err =
       Printf.sprintf(

diff --git a/packages/ppx/src/css_to_emotion.re b/packages/ppx/src/css_to_emotion.re
@@ -64,6 +64,8 @@ let concat = (~loc, expr, acc) => {
 
 let rec render_at_rule = (~loc, at_rule: at_rule) => {
   let (at_rule_name, at_rule_name_loc) = at_rule.name;
+  let at_rule_name_loc =
+    Styled_ppx_css_parser.Parser_location.intersection(loc, at_rule_name_loc);
   switch (at_rule_name) {
   | "media" => render_media_query(~loc, at_rule)
   | "keyframes" =>
@@ -151,10 +153,49 @@ and render_declaration = (~loc: Ppxlib.location, d: declaration) => {
   let value_source = source_code_of_loc(value_loc) |> String.trim;
 
   let declaration_location =
-    Styled_ppx_css_parser.Parser_location.intersection(loc, d.loc);
+    Styled_ppx_css_parser.Parser_location.update_pos_lnum(
+      {
+        let offset =
+          value_loc.loc_start.pos_lnum == 1
+            ? loc.loc_start.pos_cnum - loc.loc_start.pos_bol + 1 : 0;
+        {
+          ...value_loc,
+          loc_start: {
+            ...value_loc.loc_start,
+            pos_cnum: value_loc.loc_start.pos_cnum + offset,
+          },
+          loc_end: {
+            ...value_loc.loc_end,
+            pos_cnum: value_loc.loc_end.pos_cnum + offset,
+          },
+        };
+      },
+      loc,
+    );
 
   let property_location =
-    Styled_ppx_css_parser.Parser_location.intersection(loc, name_loc);
+    Styled_ppx_css_parser.Parser_location.update_pos_lnum(
+      {
+        let offset =
+          name_loc.loc_end.pos_lnum == 1
+            ? loc.loc_start.pos_cnum - loc.loc_start.pos_bol + 1 : 0;
+        {
+          ...name_loc,
+          loc_start: {
+            ...name_loc.loc_start,
+            pos_lnum: name_loc.loc_end.pos_lnum,
+            pos_bol: name_loc.loc_end.pos_bol,
+            pos_cnum:
+              name_loc.loc_end.pos_cnum + offset - String.length(property),
+          },
+          loc_end: {
+            ...name_loc.loc_end,
+            pos_cnum: name_loc.loc_end.pos_cnum + offset,
+          },
+        };
+      },
+      loc,
+    );
 
   switch (
     Declarations_to_emotion.parse_declarations(
@@ -307,14 +348,11 @@ and render_selectors = (~loc, selectors) => {
 }
 and render_style_rule = (~loc, rule: style_rule) => {
   let (prelude, prelude_loc) = rule.prelude;
-  let (_block, block_loc) = rule.block;
   let selector_location =
     Styled_ppx_css_parser.Parser_location.intersection(loc, prelude_loc);
-  let block_location =
-    Styled_ppx_css_parser.Parser_location.intersection(loc, block_loc);
 
   let selector_expr =
-    render_declarations(~loc=block_location, rule.block)
+    render_declarations(~loc, rule.block)
     |> Builder.pexp_array(~loc=selector_location);
 
   let (delimiter, attrs) =

diff --git a/packages/ppx/src/generate.re b/packages/ppx/src/generate.re
@@ -40,7 +40,13 @@ let generateDynamicStyles =
   let styles =
     switch (functionExpr.pexp_desc) {
     /* styled.div () => "string" */
-    | Pexp_constant(Pconst_string(str, loc, _label)) =>
+    | Pexp_constant(Pconst_string(str, loc, delimiter)) =>
+      let loc =
+        Styled_ppx_css_parser.Parser_location.update_loc_with_delimiter(
+          loc,
+          delimiter,
+        );
+
       switch (Styled_ppx_css_parser.Driver.parse_declaration_list(~loc, str)) {
       | Ok(declarations) =>
         declarations
@@ -49,7 +55,7 @@ let generateDynamicStyles =
         |> Builder.pexp_array(~loc)
         |> Css_to_emotion.render_style_call(~loc)
       | Error((loc, msg)) => Generate_lib.error(~loc, msg)
-      }
+      };
 
     /* styled.div () => "[||]" */
     | Pexp_array(arr) =>

diff --git a/packages/ppx/src/styled_ppx.re b/packages/ppx/src/styled_ppx.re
@@ -770,11 +770,11 @@ let _ =
           "css",
           Ppxlib.Extension.Context.Expression,
           string_payload_pattern,
-          (~loc, ~path, payload, _stringLoc, delimiter) => {
+          (~loc as _, ~path, payload, stringLoc, delimiter) => {
             File.set(path);
             let loc =
               Styled_ppx_css_parser.Parser_location.update_loc_with_delimiter(
-                loc,
+                stringLoc,
                 delimiter,
               );
             switch (
@@ -797,11 +797,11 @@ let _ =
           "styled.global",
           Ppxlib.Extension.Context.Expression,
           string_payload_pattern,
-          (~loc, ~path, payload, _stringLoc, delimiter) => {
+          (~loc as _, ~path, payload, stringLoc, delimiter) => {
             File.set(path);
             let loc =
               Styled_ppx_css_parser.Parser_location.update_loc_with_delimiter(
-                loc,
+                stringLoc,
                 delimiter,
               );
             switch (
@@ -819,11 +819,11 @@ let _ =
           "keyframe",
           Ppxlib.Extension.Context.Expression,
           string_payload_pattern,
-          (~loc, ~path, payload, _stringLoc, delimiter) => {
+          (~loc as _, ~path, payload, stringLoc, delimiter) => {
             File.set(path);
             let loc =
               Styled_ppx_css_parser.Parser_location.update_loc_with_delimiter(
-                loc,
+                stringLoc,
                 delimiter,
               );
             switch (

diff --git a/packages/ppx/test/css-support/delimiter-location-brackets-with-js.t/run.t b/packages/ppx/test/css-support/delimiter-location-brackets-with-js.t/run.t
@@ -19,9 +19,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 1, characters 7-23:
-  0 |  [%cx {|display: blocki;|}];
-             ^^^^^^^^^^^^^^^^
+  File "input.re", line 1, characters 17-23:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
 
@@ -30,9 +28,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 1, characters 19-36:
-  1 |  [%cx {|width: 100%; display: blocki;|}];
-                         ^^^^^^^^^^^^^^^^^
+  File "input.re", line 1, characters 30-36:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
 
@@ -43,9 +39,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 2, characters 24-41:
-  1 | .......
-  2 | ................; display: blocki.
+  File "input.re", line 2, characters 27-33:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
 
@@ -57,9 +51,6 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", lines 2-3, characters 24-28:
-  1 | .......
-  2 | ................;
-  3 |      display: blocki.
+  File "input.re", line 3, characters 14-20:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
diff --git a/packages/ppx/test/css-support/delimiter-location-brackets.t/run.t b/packages/ppx/test/css-support/delimiter-location-brackets.t/run.t
@@ -19,9 +19,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 1, characters 9-25:
-  0 |  [%cx {js|display: blocki;|js}];
-               ^^^^^^^^^^^^^^^^
+  File "input.re", line 1, characters 19-25:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
 
@@ -30,9 +28,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 1, characters 21-38:
-  1 |  [%cx {js|width: 100%; display: blocki;|js}];
-                           ^^^^^^^^^^^^^^^^^
+  File "input.re", line 1, characters 32-38:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
 
@@ -43,9 +39,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 2, characters 26-43:
-  1 | .........
-  2 | ................; display: blocki.
+  File "input.re", line 2, characters 27-33:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
 
@@ -57,9 +51,6 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", lines 2-3, characters 26-30:
-  1 | .........
-  2 | ................;
-  3 |      display: blocki.
+  File "input.re", line 3, characters 14-20:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
diff --git a/packages/ppx/test/css-support/delimiter-location-double-quotes.t/run.t b/packages/ppx/test/css-support/delimiter-location-double-quotes.t/run.t
@@ -19,9 +19,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 1, characters 6-22:
-  0 |  [%cx "display: blocki;"];
-            ^^^^^^^^^^^^^^^^
+  File "input.re", line 1, characters 16-22:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
 
@@ -30,9 +28,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 1, characters 18-35:
-  1 |  [%cx "width: 100%; display: blocki;"];
-                        ^^^^^^^^^^^^^^^^^
+  File "input.re", line 1, characters 29-35:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
 
@@ -43,9 +39,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 2, characters 23-40:
-  1 | ......
-  2 | ................; display: blocki.
+  File "input.re", line 2, characters 27-33:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
 
@@ -57,9 +51,6 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", lines 2-3, characters 23-27:
-  1 | ......
-  2 | ................;
-  3 |      display: blocki.
+  File "input.re", line 3, characters 14-20:
   Error: Property 'display' has an invalid value: 'blocki'
   [1]
diff --git a/packages/ppx/test/css-support/interpolation-location-error.t/run.t b/packages/ppx/test/css-support/interpolation-location-error.t/run.t
@@ -15,9 +15,7 @@ This test only runs against Css_Js_Core from styled-ppx.css_native
   > EOF
 
   $ dune build
-  File "input.re", line 2, characters 14-32:
-  2 | let a = [%cx {| display: $(grid); |}];
-                    ^^^^^^^^^^^^^^^^^^
+  File "input.re", line 2, characters 25-32:
   Error: This expression has type [> `gri ]
          but an expression was expected of type
            [< `block

diff --git a/packages/ppx/test/css-support/non-valid-property-location-error-nested.t/input.re b/packages/ppx/test/css-support/non-valid-property-location-error-nested.t/input.re
@@ -0,0 +1,9 @@
+let selectors = [%cx
+  {|
+  color: white;
+
+  &:hover {
+    colorx: red;
+  }
+|}
+];