Expand assert-pure.ql to cover common and be a path-problem query

[robertbrignull]

Expands the assert-pure.ql query in a couple of ways:

• Covers stuff in the src/common directory as well as src/pure. Stuff in this directory should also be independent of vscode, unless it is in the src/common/vscode directory.
• Convert the query to a path-problem query. My aim here is to make this query easier to use, by explaining where an import comes from. It can otherwise be a bit hard to understand when the import chain gets more than a couple of files.

The query now has 10 alerts 🎉 😱

All of these alerts are because of logging, either from importing extLogger or OutputChannelLogger or showAndLogErrorMessage.

The reason there's 6 alerts for selection-commands.ts is because there's 6 separate vscode imports that it transitively imports, via the giant helpers.ts file. I believe there's not a good way to compress these into a single alert without losing the information of which import it's connected to.

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]

This is really neat. Thanks for implementing.

[adityasharad]

Neat! A couple of passing suggestions.

[adityasharad]

Since you know the type you need, might as well declare it rather than casting.

Suggested change

	class VSCodeImport extends AstNode {
	VSCodeImport() { this.(Import).getImportedPath().getValue() = "vscode" }
	}
	class VSCodeImport extends Import {
	VSCodeImport() { this.getImportedPath().getValue() = "vscode" }
	}

[robertbrignull]

Changing it to Import gets some errors because Import is an abstract class.

I assume this was the reason it was like this originally. However, I should be able to change it to ImportDeclaration because all of the imports in our codebase are import rather than require. My understanding is that means we can use ImportDeclaration and not miss anything. At least right now it doesn't change the results.

[adityasharad]

Oof I didn't realise Import was abstract.
Changing it to extends ImportDeclaration is a good specific solution.
If you want something more generic, try the (relatively new) instanceof keyword: class VSCodeImport extends AstNode instanceof Import. instanceof says we are creating a subtype of Import that inherits all its methods, but does not add to its set of values as an abstract class.

[adityasharad]

Minor: this looks correct, but it is less confusing if you can easily see the relationship between edges and the select clause. Right now I think each getANonTypeOnlyImportedModule step actually contains two edges(importing module -> import -> imported module) and the getANonTypeOnlyImport step is one edge (importing module -> import).

I think you could even write this as edges+(m, v) and it would work because of the typing of v, but verify that to be sure!

[robertbrignull]

Ooh, good point. Thanks for saying this. I agree it's less confusing if it's clear the relation to edges, and we can just do edges+(m, v) because of the typing of v.