[BUGFIX] Render rules in line and column number order

[JakeQZ]

Fixes #1052.

[coveralls]

coverage: 55.673% (+0.03%) from 55.643%
when pulling e7442d9 on bugfix/rule-order
into bd4e549 on main.

[oliverklee]

Please let's not use render in unit tests (I'll try to do this for the existing code in #1057). Instead, please let's check for what getSelectors returns.

[oliverklee]

(That's because we want to check the parsing here, not the rendering, and particularly not both at the same time.)

[oliverklee]

When we mix parsing and rendering in one test, it's really hard to see what the test is about when reading it. And when the test fails, it's near impossible to find out if the parsing of the rendering is broken.

[JakeQZ]

We actually want to check the rendering here (that was what was broken).

To set up the construct whose rendering is to be tested without using the parser would require quite of lot of code. It would need to use the various constructors, setters, adders, etc. to construct this:

Sabberworm\CSS\RuleSet\DeclarationBlock::__set_state(array(
   'selectors' => 
  array (
    0 => 
    Sabberworm\CSS\Property\Selector::__set_state(array(
       'selector' => '.test',
    )),
  ),
   'rules' => 
  array (
    'background-color' => 
    array (
      0 => 
      Sabberworm\CSS\Rule\Rule::__set_state(array(
         'rule' => 'background-color',
         'value' => 'transparent',
         'isImportant' => false,
         'lineNumber' => 3,
         'columnNumber' => 94,
         'comments' => 
        array (
        ),
      )),
      1 => 
      Sabberworm\CSS\Rule\Rule::__set_state(array(
         'rule' => 'background-color',
         'value' => 
        Sabberworm\CSS\Value\Color::__set_state(array(
           'sName' => 'rgb',
           'aComponents' => 
          array (
            'r' => 
            Sabberworm\CSS\Value\Size::__set_state(array(
               'fSize' => 255.0,
               'sUnit' => NULL,
               'bIsColorComponent' => true,
               'lineNumber' => 5,
            )),
            'g' => 
            Sabberworm\CSS\Value\Size::__set_state(array(
               'fSize' => 255.0,
               'sUnit' => NULL,
               'bIsColorComponent' => true,
               'lineNumber' => 5,
            )),
            'b' => 
            Sabberworm\CSS\Value\Size::__set_state(array(
               'fSize' => 255.0,
               'sUnit' => NULL,
               'bIsColorComponent' => true,
               'lineNumber' => 5,
            )),
          ),
           'sSeparator' => ',',
           'lineNumber' => 5,
        )),
         'isImportant' => false,
         'lineNumber' => 5,
         'columnNumber' => 176,
         'comments' => 
        array (
        ),
      )),
    ),
    'background' => 
    array (
      0 => 
      Sabberworm\CSS\Rule\Rule::__set_state(array(
         'rule' => 'background',
         'value' => 
        Sabberworm\CSS\Value\Color::__set_state(array(
           'sName' => 'rgb',
           'aComponents' => 
          array (
            'r' => 
            Sabberworm\CSS\Value\Size::__set_state(array(
               'fSize' => 34.0,
               'sUnit' => NULL,
               'bIsColorComponent' => true,
               'lineNumber' => 4,
            )),
            'g' => 
            Sabberworm\CSS\Value\Size::__set_state(array(
               'fSize' => 34.0,
               'sUnit' => NULL,
               'bIsColorComponent' => true,
               'lineNumber' => 4,
            )),
            'b' => 
            Sabberworm\CSS\Value\Size::__set_state(array(
               'fSize' => 34.0,
               'sUnit' => NULL,
               'bIsColorComponent' => true,
               'lineNumber' => 4,
            )),
          ),
           'sSeparator' => ',',
           'lineNumber' => 4,
        )),
         'isImportant' => false,
         'lineNumber' => 4,
         'columnNumber' => 134,
         'comments' => 
        array (
        ),
      )),
    ),
  ),
   'lineNumber' => 1,
   'comments' => 
  array (
  ),
))

If a test has been broken by a change to the parsing, then that's likely to be the culprit. Ditto for the rendering. I agree if it's an update to a library (or PHP) it's more difficult.

On the parsing side, we can have tests that confirm specific things have been parsed, and a test for DeclarationBlock parsing doesn't need to check beyond the Selector and Rules (since we would already have tests for Rule parsing the property name and Value).

So maybe a more viable approach would be to use the parser for the rendering tests, but also have corresponding parsing tests that don't use the renderer. That way we'd also be able to tell which side was broken.

[oliverklee]

For the rendering test, I've given it a go at #1059. What do you think?