styled-ppx icon indicating copy to clipboard operation
styled-ppx copied to clipboard

Implement better errors

Open davesnx opened this issue 1 year ago • 3 comments

Current errors

The current errors are insuficient to learn how to resolve the issue when the parser is complaining, and lack a bit of context, they are classified by:

Lexing/Parsing errors

The current parser errors regarding variables and missing features is very bad.

Properties raises

  • Unsupported feature

ppx raise

  • API error usage

Improving Parser errors

Maybe we can improve ParseError and display the error with the expected next values?

Property errors

Type-check errors

Render error messages with the value ppx payload

E9yxbcbXMAE7Nlc.jpeg

Wrong value

display: blocki;
         ^^^^^^

Not a valid value for `display`.
display defintion expects one of `'block' | 'contents' | 'flex' | 'flow' | 'flow-root' | 'grid' | 'inline' | 'inline-block' | 'inline-flex' | 'inline-grid' | 'inline-list-item' | 'inline-table' | 'list-item' | 'none' | 'ruby' | 'ruby-base' | 'ruby-base-container' | 'ruby-text' | 'ruby-text-container' | 'run-in' | 'table' | 'table-caption' | 'table-cell' | 'table-column' | 'table-column-group' | 'table-footer-group' | 'table-header-group' | 'table-row' | 'table-row-group' | '-ms-flexbox' | '-ms-inline-flexbox' | '-ms-grid' | '-ms-inline-grid' | '-webkit-flex' | '-webkit-inline-flex' | '-webkit-box' | '-webkit-inline-box' | '-moz-inline-stack' | '-moz-box' | '-moz-inline-box'`
(optional) Maybe you mean `block`?

Wrong value format

Detect when partial matching happens and render the closest as an error message

border: 1px solid;
^^^^^^^^^^^^^^^^^

Missing value for `border`.
border defintion is `<line-width> || <line-style> || <color>`
Missing `<color>`

// ----
// TODO: More examples

Wrong type

border-width: 1%;
			  ^^

Wrong value type for `border-width`
border-width definition is `[ <line-width> ]{1,4}`. (For example: `2px`)

Missing property

When a not valid property is passed to our parser (we should differentiate when the property is a valid CSS but we don't support vs a invalid property)

border-width-left: 1%;
^^^^^^^^^^^^^^^^^

Invalid property `border-width-left`.
(optional) Maybe you mean `border-left-width`?

Unsupported interpolation in property

resize: $(lola);
        ^^^^^^^

`resize` property doesn't support interpolation `$(lola)`. 

Interpolation broken format

// Empty
margin-left: $();

// NotClosed
margin-left: $(aasdf;

// NotClosedAtEof
margin-left: $(aasdf

// InvalidName
margin-left: $(λ);
margin-left: $(🐪);

// Non-valid syntaxes
???  

// Unscaped
margin-left: $name;

// NotStarted
margin-left: $);

// OnlyIdentifiersAreValid
// Inline calling functions aren't supported.
// Move it into a let assignment above
margin-left: $(`lola(value));
margin-left: $(lola(value));

How?

  • [ ] Remove raises on the ppx to add errors in the AST https://ocaml.org/p/ppxlib/0.27.0/doc/manual.html#embedding-the-errors-in-the-ast

  • [ ] Error messages content snippets need to be on ReScript syntax

  • [ ] Make combine_xor generate an error from their combinations (This might be very diff)

  • [ ] Read about errors in Parser combinators: https://abstractfun.com/2018-11-19-introduction-to-parser-combinators

  • [ ] https://github.com/ebresafegaga/mehir-error-messages

  • [ ] Check how other parsers check errors

    • https://github.com/mdxtoc/qtpi/blob/master/parseutils.ml#L90
    • https://github.com/emillon/ppxlib-parsing/blob/master/ppxlib_parse.ml
    • http://gallium.inria.fr/blog/parser-construction-menhir-appetizers/
    • Example https://gitlab.inria.fr/fpottier/menhir/-/tree/master
    • Anything grain-lang: lexer, driver, parser: https://github.com/grain-lang/grain/blob/main/compiler/src/parsing/driver.re

    [Menhir advanced API example](https://www.notion.so/Menhir-advanced-API-example-85fd4618181c4337aaded2a97b1d4559?pvs=21)

    • https://github.com/sufrin/InteractiveSedlexMenhirExample
    • Simple example of Interactive api: https://github.com/yusufyildirim/menhir-sedlex-example
  • [ ] Understand how menhir can help

    • [ ] Tutorial on menhir incremental API: https://baturin.org/blog/declarative-parse-error-reporting-with-menhir
    • [ ] Understand better menhir, and their flags/modes.
    • [ ] --exn-carries-state
    • [ ] with .messages
    • [ ] %on_reduce_error

Remove parsing reduce/shifts

Currently our css_parser.mly generates a ton of warnings. This causes menhir to do a decent job on reducing those, based on the order of tokens, but when you are refactoring the parser this breaks into a lot of pieces.

It’s currently a little unreliable:

  • stylesheets
  • declarations
  • declarations_list

Use the BNF syntax to parse CSS

  • Go back to a BNF syntax (probably from SASS) or other places
  • Try to look for occurences where our syntax should be tricter, document them
  • Decide how to use the PropertyParser.re

Consider using the Incremental API

  • Watch a few Parsing with menhir videos
    • https://www.youtube.com/watch?v=IiBQBldP4S0&ab_channel=DavidBroman
    • https://www.youtube.com/watch?v=ly7yvyaDj08&ab_channel=DavidBroman
  • Check https://github.com/yurug/menhir-error-recovery for error recovery
  • Check driver from polaris: https://github.com/Innf107/polaris/blob/a2ab5b4d5b7604bb7f3529a1b4ecf45f1cec6066/src/driver.ml
  • Check https://github.com/colis-anr/morbig for Incremental API
  • https://gist.github.com/felipecrv/1180a7c8878578149781f94ef9c19acc

Consider rewriting with a handmade parser

Like rescript/syntax: https://github.com/rescript-lang/rescript-compiler/tree/master/res_syntax/src

  • [ ] Read http://ocamlverse.net/content/monadic-parsers-angstrom.html
  • [ ] See if make sense for replacing current [Parser.re](http://Parser.re) Rule and Match
  • [ ] See if make sense to use [Parser.re](http://Parser.re) defintions to support parsing the rest of the CSS spec

davesnx avatar Mar 07 '24 09:03 davesnx

File "xxx/PasswordStrength_Css.ml", line 16, characters 8-9:
Error: Parse error while reading token '['

Actual issue is on line 170, incorrect selector:

  hover [
    opacity: 0.7;
  ];

CleanShot 2024-03-08 at 13 32 06@2x

davesnx avatar Mar 08 '24 13:03 davesnx

File "frontend/packages/ahkit/src/bindings/PasswordStrength_Css.ml", line 79, characters 25-26:
Error: Parse error while reading token ';'
mask-image: $("url(" ^ eyeCrossedIcon ^ ")");

CleanShot 2024-03-08 at 13 23 22@2x

davesnx avatar Mar 08 '24 13:03 davesnx