dart-sass icon indicating copy to clipboard operation
dart-sass copied to clipboard

Highlight only invalid function arguments

Open mgreter opened this issue 6 years ago • 4 comments

Given:

foo {
  x: rgba("foo", 0.5);
}

Gives the following error in dart sass:

Error: $color: "foo" is not a color.
  ,
2 |   x: rgba("foo", 0.5);
  |      ^^^^^^^^^^^^^^^^
  '
  sel.scss 2:6  root stylesheet

IMHO it would be more useful to just highlight the invalid arg instead:

Error: $color: "foo" is not a color.
  ,
2 |   x: rgba("foo", 0.5);
  |           ^^^^^
  '
  sel.scss 2:6  root stylesheet

We seem to get the later rather freely in LibSass ...

mgreter avatar Aug 09 '19 06:08 mgreter

I'm currently experimenting a bit in LibSass with error reporting. Got it to a point where I think dart-sass might want to adapt it.

Given _foobar.scss

$number: 3;

And given main.scss

@import "foobar.scss";

@function uq($input) {
  @return unquote($input);
}

foo {
  test: uq($number);
}

My current LibSass experiment gives:

Error: 3 is not a string.
  ,
1 | $number: 3;
  |          ^
  '
  _foobar.scss 1:10

  ,
4 |   @return unquote($input);
  |           ^^^^^^^^^^^^^^^
  '
  main.scss 4:11  unquote(3)

  ,
8 |   test: uq($number);
  |         ^^^^^^^^^^^
  '
  main.scss 8:9  uq(3)

For the example above it yields:

Error: "foo" is not a color.
  ,
2 |   x: rgba("foo", 0.5);
  |           ^^^^^
  '
  sel.scss 2:11

  ,
2 |   x: rgba("foo", 0.5);
  |      ^^^^^^^^^^^^^^^^
  '
  sel.scss 2:6  rgba("foo", 0.5)

I also included the "passed" arguments to the functions in the stack trace, although I'm unsure if it is safe to assume this doesn't error on its own. It might be possible to add a cli-flag or some heuristics to know how many code fragments to print if the call stack is very deeply nested (e.g. only the last two). I have yet to implement this for multi-line spans :)

mgreter avatar Aug 09 '19 08:08 mgreter

We don't (currently) do this because we don't have the context—the error is thrown in the function implementation, which doesn't have access to the argument spans. We could pipe them through, I suppose, but this would make the user-facing function API more complicated as well and I'm not sure it's worth it.

How does LibSass track this context?

nex3 avatar Aug 10 '19 05:08 nex3

We have attached a parser state (span) on every sass value, so we know where the actual number that was passed to the function came from. Variable assignments are "transparent" and the function calls are put on the call stack. I see the issue with the API, as e.g. when the number is coming from a custom function, but I think it might be worthwhile to allow API users to define an optional source span. Although I haven't checked the feasibility of this yet.

mgreter avatar Aug 10 '19 16:08 mgreter

Tracking spans for each value object seems very expensive, because it means you have to create new values every time the logical span changes. I don't think we can afford to do that in Dart Sass.

I'd rather add a heuristic like, if the error message begins with a $<name>: and matches a parameter name, highlight that parameter.

nex3 avatar Aug 12 '19 21:08 nex3