Variables first declared in control flow rules should not be accessible in outer scopes

[andrew-skybound]

Variables assigned in block scopes should not be accessible outside of the scope, unless they have been previously assigned in an outer scope (or the outer scope is global and the !global flag is specified).

Libsass gets these semantics correct for style, @at-root, @media, and other CSS @-rules, as well as mixins but not for control flow rules (@if, @while, @for, and @each). Notice in the tests below that the _local variables being first declared within these rule types are leaking outside.

It is also notable that assignments that happen in non-control-flow rules do not overwrite variables that have been previously assigned in an outer scope. These assignments are denoted by variables named _global in the tests. Libsass gets this behavior correct, but I have included tests anyway to help prevent a regression.

Input:

$continue: true;

$while_global: initial;
@while $continue {
    $while_local: assigned;
    $while_global: assigned;
    $continue: false;
}

$each_global: initial;
@each $b in 1 {
    $each_local: assigned;
    $each_global: assigned;
}

$for_global: initial;
@for $b from 1 to 2 {
    $for_local: assigned;
    $for_global: assigned;
}

$if_global: initial;
@if true {
    $if_local: assigned;
    $if_global: assigned;
}

$else_global: initial;
@if false {
    /* nothing */
}
@else {
    $else_local: assigned;
    $else_global: assigned;
}

$at_root_global: initial;
@at-root {
    $at_root_local: assigned;
    $at_root_global: assigned;
}

$stylerule_global: initial;
stylerule {
    $stylerule_local: assigned;
    $stylerule_global: assigned;
}

$mixin_global: initial;
@mixin set_variable() {
    $mixin_local: assigned;
    $mixin_global: assigned;
}

@include set_variable();

locals {
    while: variable-exists(while_local);
    each: variable-exists(each_local);
    for: variable-exists(for_local);
    if: variable-exists(if_local);
    else: variable-exists(else_local);
    at_root: variable-exists(at_root_local);
    stylerule: variable-exists(stylerule_local);
    mixin: variable-exists(mixin_local);
}

globals {
    while: $while_global;
    each: $each_global;
    for: $for_global;
    if: $if_global;
    else: $else_global;
    at_root: $at_root_global;
    stylerule: $stylerule_global;
    mixin: $mixin_global;
}

Ruby SASS Output:

locals {
  while: false;
  each: false;
  for: false;
  if: false;
  else: false;
  at_root: false;
  stylerule: false;
  mixin: false;
}

globals {
  while: assigned;
  each: assigned;
  for: assigned;
  if: assigned;
  else: assigned;
  at_root: initial;
  stylerule: initial;
  mixin: initial;
}

Libsass Output (3.3.2):

locals {
  while: true;
  each: true;
  for: true;
  if: true;
  else: true;
  at_root: false;
  stylerule: false;
  mixin: false;
}

globals {
  while: assigned;
  each: assigned;
  for: assigned;
  if: assigned;
  else: assigned;
  at_root: initial;
  stylerule: initial;
  mixin: initial;
}

[mgreter]

Got some results (aka more problems) from a debug session. The following sample should show the problematic behavior (certain scopes seem the behave slightly different):

$continue: true;
$while_global1: initial;
$while_global2: initial;
$other_global1: initial;
$other_global2: initial;
// bleads into global
@while $continue {
    $while_global1: assigned;
    $while_global2: assigned !global;
    $continue: false;
}
// does not blead into global
foobar {
    $other_global1: assigned;
    $other_global2: assigned !global;
}
// different results for global
globals {
    while1: $while_global1;
    while2: $while_global2;
    other1: $other_global1;
    other2: $other_global2;
}

Result by ruby sass 3.4.19

globals {
  while1: assigned;
  while2: assigned;
  other1: initial;
  other2: assigned; }

I guess we need a flag on Environment for this.

Here another currently failing spec test which shows a related problem:

$var: 7;

.selector {
  $var: 13;
  var: $var;
  .inner {
    $foo: "dummy";
    $var: $var + 3;
  }
  foo: variable-exists(foo);
  var: $var;
}

.other-selector {
  content: $var;
}

Dev Info: Need flag on env to check in set_lexical. Push a new env to env_stack in ruleset expanding.
Also note that we have code for For, Each etc. in both expand and eval phase. Could use more specs.

[xzyfer]

Spec added sass/sass-spec#625