SomMeri / less4j

Less language is an extension of css and less4j compiles it into regular css. Less adds several dynamic features into css: variables, expressions, nested rules, and so on. Less was designed to be compatible with css and any correct css file is also correct less file.
145 stars 47 forks source link

CRIT ISSUE with LESS4J scope #342

Open twhoff opened 8 years ago

twhoff commented 8 years ago

Scope inheritance in LESS4J is not working as intended in LESS.

Basically, child mixins inherit global scope OR parent scope before any operations on variables take place.

Simple example

In the following code @width is inherited from the global scope in .fluid-col().

The .calculate() mixin then converts the number to a valid percentage, and as @width is not explicitly set within the scope of .fluid-col(), the unlocked @width variable should override the inherited one, as is specified in LESS documentation:

Variables and mixins defined in a mixin are visible and can be used in caller's scope. There is only one exception, a variable is not copied if the caller contains a variable with the same name (that includes variables defined by another mixin call). Only variables present in callers local scope are protected. Variables inherited from parent scopes are overridden.

The overridden @width variable should now be inherited by the .inner-scope-test() mixin, however, in LESS4J this is not the case.

/* Scope test */

// Global scope
@width: 1/3;

// Caller
.test {
    .fluid-col();
}

// Mixin
.fluid-col() {
    // @width should be changed to 33.33333333%
    .calculate(@width);
    width: @width;

    .inner-scope-test();
    .inner-scope-test() {
        width2: @width;
    }
}

// Operator
.calculate(@arg) {
    @width: unit(@arg * 100, %);
}

Correct output from LESS compiler

/* Scope test */
.test {
  width: 33.33333333%;
  width2: 33.33333333%;
}

Incorrect output from LESS4J compiler

.test {
  width: 33.33333333333333%;
  width2: 0.3333333333333333;
}

Workaround Passing the operated variable as a parameter to the inner mixin produces the correct result, however, this is less than ideal when you are dealing with multiple parameters which you then need to keep track of manually.

/* Scope test */

// Global scope
@width: 1/3;

// Caller
.test {
    .fluid-col();
}

// Mixin
.fluid-col() {
    // @width should be changed to 33.33333333%
    .calculate(@width);
    width: @width;

    .inner-scope-test(@width);
    .inner-scope-test(@width) {
        width2: @width;
    }
}

// Operator
.calculate(@arg) {
    @width: unit(@arg * 100, %);
}

Output

.test {
  width: 33.33333333333333%;
  width2: 33.33333333333333%;
}
SomMeri commented 8 years ago

Note to self: The issue is caused by these lines: https://github.com/SomMeri/less4j/blob/bca5cb262c018f38ea8c58175df344c7d2345148/src/main/java/com/github/sommeri/less4j/core/compiler/stages/MixinsRulesetsSolver.java#L139-L144

The toIndependentWorkingCopy ensures that second call does not see imported variables. Solution: the joinIfIndependent method needs to work differently for local calls.