Add unique-command-use.ql

[robertbrignull]

Introduces a new CodeQL query for detecting when a VS Code command is used in more than one location. By "location" we mean a UI element or being called from the code or being available in the command palette. The intention is to make our telemetry more useful, because in the telemetry we can't tell which place the command was called from. To remedy an alert, the command should effectively be duplicated with a new name but referencing the same implementation.

This is still a draft, to share the query and get thoughts. I'll come and add more comments, and I'm sure the CodeQL team will have some helpful advice.

Checklist

• CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
• Issues have been created for any UI or other user-facing changes made by this pull request.
• [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

[aeisenberg]

Overall, looks reasonable. A few questions inline.

[adityasharad]

Skipping the first usage is clever. I can also see it being confusing if you want to audit all the places that call a command and decide which one you want to preserve (which may not be the first). As a consumer of the alert, would it be useful to just create an alert for each use? Then you don't need isFirstUsage.
You can also put the total count in the message, e.g. count(CommandUsage use | use.getCommandName() = ...).

[robertbrignull]

The original aim of omitting the first usage was so that going from one usage to two usages only creates one alert rather than two. But you make a very good point that the one that gets alerted isn't necessarily the one you want to remove/update. If everyone is ok with it perhaps it would be best to just alert on every instance so it's easier to find the usages.

Including the number of usages in the message is also a good idea. If you go from two usages to three, would than count as 1 alert added, or would it count as two alerts removed and three alerts added 🤔

[robertbrignull]

After discussion with @norascheuch we have decided to go the other way and instead only alert once per command. So this means if there are multiple usages we have to pick one, and we decided to stick with this existing isFirstUsage implementation. The exact implementation doesn't matter too much and we can change it in the future. For right now we just want this query to be useful to us and we know that the location being alerted on may not be the place to change but you should look for other usages too.

To make this easier, I've changed the alert message to include the number of usages, and added some documentation to the query indicating that you should search for other usages when fixing.

[robertbrignull]

Thanks for the reviews so far. After discussing with @norascheuch we've gone a slightly different route by having only one alert for each command, but then the alert message indicates how many total usages there are.

I realise this isn't quite what was discussed or suggested before. Our aim is to just make this query useful for us right now, and not get too bogged down into making it perfect. There are other ways we could implement this but the question is whether it's worth the implementation cost.

Also note that it's very easy to change this query behaviour in the future. We could change which locations we alert on, or how the message is presented, and the only fallout from that would be some alert on the PR changing the query. So I'm not too stressed about getting it perfect first time.

[robertbrignull]

@norascheuch, you can see the new alerts at https://github.com/github/vscode-codeql/pull/2101/checks?check_run_id=11630028878. Let me know how they look to you.

[adityasharad]

Looks sensible. Some minor suggestions on description and QL structure.

[norascheuch]

Alerts look good, thank you!