On invalid LESS, throw exception rather than emit blanks

[atdt]

This patch makes lessphp throw errors when it is called to compile LESS code
that references undefined variables or mixins. A mixin invocation will cause an
exception to be thrown if it fails to match, whether it is because no mixin of
that name exists, or because the invocation fails to match against the
signatures or guards of defined mixins. This is consistent with the behavior of
the reference LESS implementation.

The patch removes cases from tests/inputs/* that fail to compile. A new test
suite, ErrorHandlingTest.php, verifies the new error-throwing behavior.

[jgonera]

Fixes #475.

[leafo]

Awesome, thanks for the patch

[mitom]

After this patch trying to compile twitter bootstrap results in a
Fatal Error: .pull-right is undefined: bootstrap/less/bootstrap.less on line 60

[atdt]

Hrm, I'll investigate.

[jdlrobson]

@mitom I'm pretty sure this is an issue with Bootstrap...

Looking at the bootstrap LESS implementation I found a minimum test case:

.dropdown-menu {
  position: absolute;
}

@media (min-width: 100px) {
  .navbar-right {
    .dropdown-menu {
      .pull-right > .dropdown-menu();
    }
  }
}

If you render this before the patch you get

.dropdown-menu {
  position: absolute;
}

After the patch you get the error you are reporting.
I'm pretty sure that media query is invalid LESS so it is doing exactly what it is meant to be doing (it gets confused by the syntax and thinks there is a mixin called pull-right)

Can anyone confirm?

[jdlrobson]

This seems to be consistent with the nodejs implementation of less
When I try to render this I get:

NameError: .pull-right > .dropdown-menu is undefined in less/common/common.less:11:6
10 .dropdown-menu {
11 .pull-right > .dropdown-menu();
12 }

[jdlrobson]

I'm running lessc 1.3.3 (LESS Compiler) [JavaScript] so it seems this is new syntax introduced since then that bootstrap is using that lessphp doesn't yet support.

As an interim measure until we reflect that new syntax we might want to consider a variable --hide-errors or something similar? Although it seems surfacing the error is a much better thing exactly for this reason.

[tomsarduy]

#483

[mattleff]

@atdt said:

A mixin invocation will cause an exception to be thrown if it fails to match, whether it is because no mixin of that name exists, or because the invocation fails to match against the signatures or guards of defined mixins. This is consistent with the behavior of the reference LESS implementation.

This is not consistent with reference LESS implementation, specifically as it relates to guards. If a guard must match in order to avoid throwing an exception there is no point to having the guard. Guards as they exist in lessphp now are either matching (in which case the selectors, properties, etc. will be added) or they throw an exception in which case the whole .less file not be rendered. The last two tests in tests/ErrorHandlingTest.php disagree with the results from Less.js. (See here and here).

It is not possible to run the doc examples anymore. IMO, it is better to allow guards by reverting the undefined mixin error to a break.

cc/ @leafo