update the style guide on alert-messages

[erik-krogh]

and add a QL-for-QL query detecting some violations of the guidance.

---

This updates the style guide as suggested by @felicitymay.
I've tried to write down the findings from asking about how the alert-message should look, and my own findings from revising a lot of alert-messages.

---

I would like a doc review on the changes to docs/query-metadata-style-guide.md, and whether the guidance seems reasonable for what an alert-message should look like.

[esbena]

In this case, I think a thorough documentation on each predicate is warranted, with at least one good and bad example. Then the docteam can even confirm that the query is doing the right thing.

[esbena]

For once, the docteam should look at the predicate documentation. See 88f1d2a in particular. Review it like this: does the docstring express the intent of the style guide?

[kaeluka]

Overall, looks good. Some small remarks, I'm pretty open about how you're going to address them — which is why I'm giving an approval.

One bonus comment:

I noticed that an alert message like this "Pointer arithmetic here may be done with the wrong type because of the cast $@.", source, "here" triggers two instances of the alert. I guess one for each string constants.

But an alert like "$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".", (where one string contains both flows to and here) triggers only one alert that contains both messages.

This is really minor and not worth making the query harder to read and I suspect that making this more consistent will be doable without making the implementation more complex; but I'm bringing it up in case it is possible easily.

[kaeluka]

It's not obvious to me why 'this location' would be better. I don't mind the rule, because consistent writing is a good thing — but can you add a half sentence to explain the reasoning?

[erik-krogh]

I remember someone suggesting it to avoid the "click here" anti-pattern, but now I can't find that review comment.

I've inserted a short sentence referencing the "click here" anti-pattern in the end.

[erik-krogh]

I'm wondering a bit if we should keep this line.

@aibaars suggested in a Slack comment that the "this location" phrasing got around the general "click-here" anti-pattern.
But as @smowton points out, it might not always make the text clearer: #10413 (comment)

Maybe the guidance should be that a link-text shouldn't be "here"?

[smowton]

In most cases could you just linkify the immediately preceding text, so something like

This sensitive data is written to a logfile unescaped /here/ -> This sensitive data is /written to a logfile unescaped/?

[smowton]

Then you can keep here when it refers to "[some statement, expression, argument or result on] the line of this alert" and replace it when it's referring to a second thing involved in the alert.

[erik-krogh]

I've tried to revise the text a bit, and moved the the parts about link-texts into a separate sub-section.

In most cases could you just linkify the immediately preceding text.

Yes, and the guide now includes that 👍

---

I still have the guidance about "this location" when you have an alert message without links, for when you can't get around mentioning the current location in the message.

[erik-krogh]

I noticed that an alert message like this "Pointer arithmetic here may be done with the wrong type because of the cast $@.", source, "here" triggers two instances of the alert. I guess one for each string constants.

But an alert like "$@ flows to here and is used in arithmetic, potentially causing an " + effect + ".", (where one string contains both flows to and here) triggers only one alert that contains both messages.

That's because the location I'm using is the string-constant that contains the style-violation.
In the latter case both problems are contained in the same string-constant, so the alert-location is exactly the same.
I could put the alert-location on the select, but that would break when the string-constant is outside the from-where-select.

[hubwriter]

Docs team reviewer:
I've added a few (minor) comments.
👍

[kaeluka]

lgtm, thanks for the answers

[smowton]

Style guide lgtm (haven't reviewed the ql-for-ql bit)