By: Tom Sydney Kerckhove <syd@cs-syd.eu>
Say what the types promise, and promise it
A review of this branch found that isValid does not mean Prometheus will take
it, which is a thing the README came close to claiming. Four kinds of value
were valid here and refused there. Three of them are closed:
* a selector of nothing but negative matchers,
* one whose only matcher has an empty value,
* one whose only matcher is a regular expression that matches the empty
string.
All three are the same rule. A matcher that matches the empty string also
matches every series that does not carry the label, so a selector made only of
those still asks for everything, and Prometheus says so. Validity decides it
now, for every regular expression this type builds itself. A Pattern is taken
at its word: whether one matches the empty string is a question of running a
regular expression, and refusing the ones that might would turn a legal query,
such as the {unit=~"prefix.+"} a log stream is picked out with, into a value
this type will not hold.
The fourth is PromQL's type system, which stays where it was: the generators
know it and the types do not. The README says that plainly now rather than
leaving it to be read out of the word "generators".
Also from the review:
* two of the three end-to-end properties compared two ParsedQuery values,
and two refusals are equal as readily as two trees. They say they got a
tree before they compare them.
* weeder rooted "^PromQL\." which is every binding in the library, so it
could never report anything. The roots are the API, written out, and the
internals are rooted by being reachable from it.
* a duration variable reaches the query without being looked at, so what it
may hold is now the one thing that is.
* the bool modifier was a Bool, which says nothing at a call site about
which of the two it asked for. It is Answering.
* a string's tabs and carriage returns are escaped. Prometheus takes them
raw, so this is not what stops it reading the query; it is that a query is
read back out of a dashboard's JSON and an alerting rule's YAML.
* two comments were not true. One pointed at a NaN function this library
does not have and the other at a comparisonBool that never existed.