Deny light-mode definitions in selectors

library/Icinga/Less/LightMode.php

@@ -2,48 +2,50 @@
 
 namespace Icinga\Less;
 
+use ArrayIterator;
 use InvalidArgumentException;
+use IteratorAggregate;
 use Less_Environment;
 
 /**
  * Registry for light modes and the environments in which they are defined
  */
-class LightMode
+class LightMode implements IteratorAggregate
 {
+    /** @var array Mode environments as mode-environment pairs */
     protected $envs = [];
 
+    /** @var array Assoc list of modes */
     protected $modes = [];
 
-    public function add($mode, $module = null)
+    /** @var array Mode selectors as mode-selector pairs */
+    protected $selectors = [];
+
+    /**
+     * @param string $mode
+     *
+     * @return $this
+     *
+     * @throws InvalidArgumentException If the mode already exists
+     */
+    public function add($mode)
     {
         if (array_key_exists($mode, $this->modes)) {
             throw new InvalidArgumentException("$mode already exists");
         }
 
-        $this->modes[$mode] = $module ?: null;
+        $this->modes[$mode] = true;
 
         return $this;
     }
 
-    public function isModule($mode)
+    /**
-    {
+     * @param string $mode
-        return isset($this->modes[$mode]);
+     *
-    }
+     * @return Less_Environment
-
+     *
-    public function list()
+     * @throws InvalidArgumentException If there is no environment for the given mode
-    {
+     */
-        $modes = [];
-
-        foreach ($this->modes as $mode => $module) {
-            $modes[$module][] = $mode;
-        }
-
-        $byModule = $modes;
-        unset($byModule[null]);
-
-        return [isset($modes[null]) ? $modes[null] : [], $byModule];
-    }
-
     public function getEnv($mode)
     {
         if (! isset($this->envs[$mode])) {
@@ -53,6 +55,14 @@ class LightMode
         return $this->envs[$mode];
     }
 
+    /**
+     * @param string           $mode
+     * @param Less_Environment $env
+     *
+     * @return $this
+     *
+     * @throws InvalidArgumentException If an environment for given the mode already exists
+     */
     public function setEnv($mode, Less_Environment $env)
     {
         if (array_key_exists($mode, $this->envs)) {
@@ -63,4 +73,54 @@ class LightMode
 
         return $this;
     }
+
+    /**
+     * @param string $mode
+     *
+     * @return bool
+     */
+    public function hasSelector($mode)
+    {
+        return isset($this->selectors[$mode]);
+    }
+
+    /**
+     * @param string $mode
+     *
+     * @return string
+     *
+     * @throws InvalidArgumentException If there is no selector for the given mode
+     */
+    public function getSelector($mode)
+    {
+        if (! isset($this->selectors[$mode])) {
+            throw new InvalidArgumentException("$mode does not exist");
+        }
+
+        return $this->selectors[$mode];
+    }
+
+    /**
+     * @param string $mode
+     * @param string $selector
+     *
+     * @return $this
+     *
+     * @throws InvalidArgumentException If a selector for given the mode already exists
+     */
+    public function setSelector($mode, $selector)
+    {
+        if (array_key_exists($mode, $this->selectors)) {
+            throw new InvalidArgumentException("$mode already exists");
+        }
+
+        $this->selectors[$mode] = $selector;
+
+        return $this;
+    }
+
+    public function getIterator()
+    {
+        return new ArrayIterator(array_keys($this->modes));
+    }
 }

library/Icinga/Less/LightModeDefinition.php

@@ -3,7 +3,9 @@
 namespace Icinga\Less;
 
 use Less_Environment;
+use Less_Exception_Compiler;
 use Less_Tree_DetachedRuleset;
+use Less_Tree_Ruleset;
 
 /**
  * Register the environment in which the light mode is defined
@@ -54,6 +56,17 @@ class LightModeDefinition extends Less_Tree_DetachedRuleset
     {
         $drs = parent::compile($env);
 
+        /** @var $frame Less_Tree_Ruleset */
+        foreach ($env->frames as $frame) {
+            if ($frame->variable($this->getName())) {
+                if (! empty($frame->first_oelements) && ! isset($frame->first_oelements['.icinga-module'])) {
+                    throw new Less_Exception_Compiler('Light mode definition not allowed in selectors');
+                }
+
+                break;
+            }
+        }
+
         $this->getLightMode()->setEnv($this->getName(), $env->copyEvalEnv($env->frames));
 
         return $drs;

library/Icinga/Less/LightModeVisitor.php

@@ -15,13 +15,9 @@ class LightModeVisitor extends Less_VisitorReplacing
 
     public function visitRulesetCall($c)
     {
-        if ($this->getLightMode()->isModule($c->variable)) {
         return LightModeCall::fromRulesetCall($c)->setLightMode($this->getLightMode());
     }
 
-        return $c;
-    }
-
     public function run($node)
     {
         return $this->visitObj($node);

library/Icinga/Less/Visitor.php

@@ -84,14 +84,16 @@ CSS;
         if ($this->variableOrigin->name === '@' . static::LIGHT_MODE_NAME) {
             $this->variableOrigin->name .= '-' . substr(sha1(uniqid(mt_rand(), true)), 0, 7);
 
-            $this->lightMode->add($this->variableOrigin->name, $this->moduleSelector);
+            $this->lightMode->add($this->variableOrigin->name);
 
             if ($this->moduleSelector !== false) {
+                $this->lightMode->setSelector($this->variableOrigin->name, $this->moduleSelector);
+            }
+
             $drs = LightModeDefinition::fromDetachedRuleset($drs)
                 ->setLightMode($this->lightMode)
                 ->setName($this->variableOrigin->name);
         }
-        }
 
         // Since a detached ruleset is a variable definition in the first place,
         // just reset that we define a variable.
@@ -190,14 +192,14 @@ CSS;
         $evald = $this->visitObj($node);
 
         // The visitor has registered all light modes in visitDetachedRuleset, but has not called them yet.
-        // Now the light mode calls are prepared with the appropriate module CSS selector.
+        // Now the light mode calls are prepared with the appropriate CSS selectors.
         $calls = [];
-        list($modes, $moduleModes) = $this->lightMode->list();
+        foreach ($this->lightMode as $mode) {
-        if (! empty($modes)) {
+            if ($this->lightMode->hasSelector($mode)) {
-            $calls[] = implode("();\n", $modes) . '();';
+                $calls[] = "{$this->lightMode->getSelector($mode)} {\n$mode();\n}";
+            } else {
+                $calls[] = "$mode();";
             }
-        foreach ($moduleModes as $module => $modes) {
-            $calls[] = "$module {\n" . implode("();\n", $modes) . "();\n}";
         }
 
         if (! empty($calls)) {

test/php/library/Icinga/Util/LessParserTest.php

@@ -5,6 +5,7 @@ namespace Tests\Icinga\Util;
 
 use Icinga\Test\BaseTestCase;
 use Icinga\Util\LessParser;
+use Less_Exception_Compiler;
 
 class LessParserTest extends BaseTestCase
 {
@@ -459,7 +460,7 @@ LESS
   :root {
     --my-other-color: green;
   }
-  .wrapper {
+  .icinga-module.module-test {
     --greenish-color: lime;
     --blueish-color: navy;
   }
@@ -483,7 +484,7 @@ CSS
   }
 };
 
-.wrapper {
+.icinga-module.module-test {
   @light-mode: {
     @more-light-colors();
   };
@@ -497,4 +498,20 @@ LESS
             )
         );
     }
+
+    public function testLightModeDefinitionRestrictedInSelectors()
+    {
+        $this->expectException(Less_Exception_Compiler::class);
+
+        $this->compileLess(<<<LESS
+.selector {
+  @light-mode: {
+    :root {
+      --my-color: orange;
+    }
+  };
+}
+LESS
+        );
+    }
 }