[TASK] Do not allow string values for rules anymore
String values had not been allowed for rules, and should not be. (Passing string values was a bug in the Emogrifier library.)
@see https://github.com/MyIntervals/emogrifier/pull/1144
This reverts commit 67a6e95f8ad2c9c3d8d745f1943eda8da81383a4.
I’m not sure this is true. When the rule value is an identifier, it will be a string stored in mValue:
.test {
white-space: nowrap;
}
will parse into the following Rule:
object(Sabberworm\CSS\Rule\Rule)#8 (7) {
["sRule":"Sabberworm\CSS\Rule\Rule":private]=>
string(11) "white-space"
["mValue":"Sabberworm\CSS\Rule\Rule":private]=>
string(6) "nowrap"
["bIsImportant":"Sabberworm\CSS\Rule\Rule":private]=>
bool(false)
["aIeHack":"Sabberworm\CSS\Rule\Rule":private]=>
array(0) {
}
["iLineNo":protected]=>
int(2)
["iColNo":protected]=>
int(19)
["aComments":protected]=>
array(0) {
}
}
AFAICT the type for mValue should be Value|string. IMHO we should also get rid of null and just use "" for the default value.
When the rule value is an identifier, it will be a string stored in
mValue:
Would introducing an Identifier (or Keyword) subclass of Value help resolve this? Such a class would just wrap a string, but also allow the unique Value superclass type to be used whenever a value is expected or provided (e.g. see #507 which results in lots of type specifications of Value|string).
IMHO we should also get rid of
nulland just use""for the default value.
It seems that the default, with no value having been set, does not represent a valid property declaration. This would not arise from parsing, since Value::parseValue would throw an exception, but may arise from manipulation by user code instantiating additional Rules and failing to set their values. Rules with a missing value would currently be rendered as key:;, and discarded upon reparsing - but should probably be rendered as an empty string.
The constructor enforces that a key is provided. Maybe it should also enforce that a value is provided - though that would be a BC.
Marking this as draft for now. I'd like to pick this up again when we've covered the code with more unit tests. After that, I'd like to reduce multi-types as much as possible in order to simplify things and help static analysis.