Add queries for VS Code jump-to-definition

[jcreedcmu]

The tags on these queries are a tentative proposal for how to signal to the vscode extension that they are the right template queries to run for jump-to-definition and find-references. I am very open to anything from bikeshedding their exact names, to arguments for using other mechanisms instead of tags.

[sj]

How about:

@tags ide-contextual-queries/local-definitions

[jcreedcmu]

Good idea, done.

[jcreedcmu]

Possible arguments for "why use @tags and not @id to locate the distinguished query/queries" include
(1) Maybe we would in principle want multiple queries that all contribute definition-use pairs, instead of one?
(2) The extension can always look for the same specific tag name, across all languages, as opposed to needing to map "ok this is the c++ qlpack, so I need to prefix cpp/ to some chosen string like jump-to-definition, at least assuming we're required to follow the convention that all query ids for $LANGUAGE begin $LANGUAGE/.

[jbj]

Possible arguments for "why use @tags and not @id to locate the distinguished query/queries" include ...

Those are good arguments, so let's focus on the possibility of using @kind instead.

[alexet]

The reason is that @kind is meant to represent the type of the output of the query and how it should be processed. This is exactly the same for this definitions query as the original query. Originally I added a new kind but it shares all code paths with the existing kind. The only other use of @kind is for filtering but tags also exist for that case. I think tags make more sense for filtering than kinds (although we will have to swtich the existing suite to find the definitions query by a new tag rather than by kind) as that is the purpose of tags. I am not completely opposed to adding a new @kind but I do feel like that is bending the purpose of @kind.

[alexet]

Note that classify files does require different processing so it does need a new kind.

[jbj]

although we will have to swtich the existing suite to find the definitions query by a new tag rather than by kind

Yes, and that can be done without changing LGTM since we'll just tweak the scripts to produce a suite with exactly one @kind definitions query.

Thanks for clarifying. I have no further objections to classifying by tag.

[jbj]

LGTM except formatting

[alexet]

We do need to tweak the current codeql suites though.

[jcreedcmu]

Is the above commit sufficient for 'tweak the current codeql suites'? semmle/code/ql/cpp/ql/src/codeql-suites/cpp-lgtm.qls already doesn't include localDefinitions.ql and localReferences.ql when I codeql query resolve it, and I don't see any other .qls files in cpp.

[rdmarsh2]

Perhaps the @kind should be something like codenav rather than definitions?

[max-schaefer]

Does the script for generating the LGTM query suites need to be updated as well?

[hvitved]

Move to definitions.qll?

[jcreedcmu]

Does the script for generating the LGTM query suites need to be updated as well?

Good question; I'm trying to find a ./build target that actually erroneously includes localDefinitions.ql and localReferences.ql and I'm failing so far. I see in Semmle/code/build the build rule

    genrule(lang+'-lgtm-suite',
            inputs = [ 'buildutils-internal/scripts/generate-lgtm-suites.py' ],
            rec_inputs = [ [ 'language-packs/' + lang + '/ql/src', 'language-packs/' + lang + '/ql/src' ] ],
            outputs = [ lang + '-lgtm' ],
            commands = [
                'python2 generate-lgtm-suites.py ' +
                ' -a' +
                ' -l ' + lang +
                ' -o ' + lang + '-lgtm' +
                ' -p ' + lang +
                ' -r ' + '\'$${semmle_dist}/language-packs/' + lang + '/ql/\''
                ' language-packs/' + lang + '/ql/src'
            ])

but when I run ./build target/packs/pack-cpp-lgtm-suite.zip the invocation of generate-lgtm-suites.py I see

cd target/general/lgtm-query-suites/working && python2 /home/jcreed/semmle/code/buildutils-internal/scripts/generate-lgtm-suites.py -o cpp-alerts-lgtm semmlecode-cpp-queries

which lacks the -a flag, which consequently doesn't include localDefinitions and localReferences. (When I run the script manually with -a, it does include them)

Any idea where that script is invoked with -a somewhere? If so, then I might agree we need to modify it to exclude queries tagged ide-contextual-queries/local-{definitions,references}.

[max-schaefer]

I think ./build target/packs/pack-go-lgtm-suite.zip should do the trick.

EDIT: No, I lie. Try ./build target/general/go-lgtm-suite/output/go-lgtm.

[jcreedcmu]

I think ./build target/packs/pack-go-lgtm-suite.zip should do the trick.

EDIT: No, I lie. Try ./build target/general/go-lgtm-suite/output/go-lgtm.

Ok, both of those generated a commandline with -a for me. Any explanation for why that happens with golang and not the other languages?

[jcreedcmu]

In any event, raised https://git.semmle.com/Semmle/code/pull/36810 to exclude queries by tag.

[max-schaefer]

Any explanation for why that happens with golang and not the other languages?

The legacy languages specify parts of their LGTM suites manually. For Go everything is auto-generated.

[jbj]

Since this can have performance implications for the LGTM suite, I’d like to wait until the CPP-Differences jobs for #3400 have finished and then run CPP-differences for this PR too.

I assume it’s not urgent to merge this since it’s targeted at master, and the lgtm.com branch will be based on 1.24 for a while yet.

[jbj]

I've started https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/1107/ to test this PR for performance.

[jbj]

Performance LGTM. There's a noticeable slowdown on Wireshark, but that affects other queries too, so I think it's just noise.