Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust: merge current status of experimental extractor into main #17435

Merged
merged 101 commits into from
Sep 11, 2024

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Sep 11, 2024

This will require an internal PR for integration of the rust extractor in the internal build system, but should be independent of that for CI purposes.

For ruby, the changes involve getting rid of the fake create workaround as described with more context here. This involved creating a cargo workspace at the root of codeql including the shared tree-sitter extractor and the two extractors using them (that we build with bazel, I'm ignoring ql here).

This had the side effect of moving the cargo output directory up in the repository root, which is what causes the other changes around ruby.

Paolo Tranquilli and others added 30 commits August 16, 2024 10:37
This should make the rust-analyzer library look for a rust toolchain and
try to start the proc-macro server.
This generates test source files from code blocks in class docstrings.

By default the test code is generated as is, but it can optionally:
* be wrapped in a function providing an adequate context using
  `@rust.doc_test_function(name, *, lifetimes=(), return_type="()", **kwargs)`,
  with `kwargs` providing both generic and normal params depending on
  capitalization
* be skipped altogether using `@rust.skip_doc_test`

So for example an annotation like
```python
@rust.doc_test_function("foo",
                        lifetimes=("a",),
                        T="Eq",
                        x="&'a T",
                        y="&'a T",
                        return_type="&'a T")
```
will result in the following wrapper:
```rust
fn foo<'a, T: Eq>(x: &'a T, y: &'a T) -> &'a T {
    // example code here
}
```
Bazel: remove non-working fake `tree-sitter-extractor` workaround
@redsun82 redsun82 requested review from a team as code owners September 11, 2024 07:28
@redsun82 redsun82 added the no-change-note-required This PR does not need a change note label Sep 11, 2024
final override predicate shouldBePrinted() { shouldPrint(ast) }

override predicate hasChild(PrintAstNode child, int index, string label) {
child = TPrintLocatable(any(Locatable c | c = getChildAndAccessor(ast, index, label)))

Check warning

Code scanning / CodeQL

Expression can be replaced with a cast Warning

The assignment to
c
in this any(..) can be replaced with an inline cast.
@redsun82 redsun82 marked this pull request as draft September 11, 2024 07:42
@redsun82 redsun82 added the Rust Pull requests that update Rust code label Sep 11, 2024
@redsun82 redsun82 marked this pull request as ready for review September 11, 2024 10:06
* that starts a basic block to `cfn` along the `intraBBSucc` relation.
*/
cached
predicate bbIndex(CfgNode bbStart, CfgNode cfn, int i) =

Check warning

Code scanning / CodeQL

Missing QLDoc for parameter Warning

The QLDoc has no documentation for bbStart, but the QLDoc mentions bb

private import rust
private import internal.ControlFlowGraphImpl
private import internal.Completion

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
internal.ControlFlowGraphImpl
.
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seem to be a number of changes in Ruby - we should presumably get a 👍 from someone on the Ruby team to merge this?

- id: end-of-file-fixer
exclude: /test/.*$(?<!\.ql)(?<!\.qll)(?<!\.qlref)|.*\.patch
exclude: /test/.*$(?<!\.qlref)|.*\.patch$|.*\.qll?$
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was .ql deleted from this on purpose?

Copy link
Contributor Author

@redsun82 redsun82 Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before, ql files were included, now they are excluded always (it's in the qll? pattern): I found a corner case where this check was not agreeing with our ql formatter, who was inserting two newlines at the end. So we can leave our formatter deal with ql and qll files, and no other linter to interfere with that.

@@ -4,6 +4,5 @@
--ql-output=ql/lib/codeql/swift/generated
--ql-stub-output=ql/lib/codeql/swift/elements
--ql-test-output=ql/test/extractor-tests/generated
--generated-registry=ql/.generated.list
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's going on here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is now a default deduced from the other outputs (i.e. the longest common path, which amounts to the same location)

@redsun82
Copy link
Contributor Author

There seem to be a number of changes in Ruby - we should presumably get a 👍 from someone on the Ruby team to merge this?

I'll call it out, good point. It was confirmed by DCA, but it's still good to have eyes on those changes.

Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ruby 👍

@redsun82 redsun82 merged commit f9e4c0a into main Sep 11, 2024
48 checks passed
@redsun82 redsun82 deleted the rust-experiment branch September 11, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation no-change-note-required This PR does not need a change note Ruby Rust Pull requests that update Rust code Swift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants