Skip to content

Rust: Add telemetry for comparing against rust-analyzer #19025

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

Merged
merged 1 commit into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion rust/ql/src/queries/telemetry/ExtractorInformation.ql
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import rust
import DatabaseQuality
import RustAnalyzerComparison
import codeql.rust.Diagnostics

predicate fileCount(string key, int value) {
Expand Down Expand Up @@ -41,6 +42,20 @@ predicate extractorDiagnostics(string key, int value) {
)
}

predicate pathResolutionCompare(string key, int value) {
exists(string suffix |
PathResolutionCompare::summary(suffix, value) and
key = "Rust-analyzer path resolution comparison: " + suffix
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess I would expect the value to come after a :. Maybe , or some other symbol would work better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would rather not change it, since we already have logic in DCA that depends on the exact wording of the string.

)
}

predicate callGraphCompare(string key, int value) {
exists(string suffix |
CallGraphCompare::summary(suffix, value) and
key = "Rust-analyzer call graph comparison: " + suffix
)
}

from string key, float value
where
(
Expand All @@ -54,7 +69,9 @@ where
CallTargetStatsReport::percentageOfOk(key, value) or
MacroCallTargetStatsReport::numberOfOk(key, value) or
MacroCallTargetStatsReport::numberOfNotOk(key, value) or
MacroCallTargetStatsReport::percentageOfOk(key, value)
MacroCallTargetStatsReport::percentageOfOk(key, value) or
pathResolutionCompare(key, value) or
callGraphCompare(key, value)
) and
/* Infinity */
value != 1.0 / 0.0 and
Expand Down
134 changes: 134 additions & 0 deletions rust/ql/src/queries/telemetry/RustAnalyzerComparison.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/**
* INTERNAL: Do not use.
*
* Provides functionality for comparing data from `rust-analyzer` with data computed
* in QL.
*/

import rust

private signature module ResolvableSig {
class Source {
string toString();

Location getLocation();
}

class Target {
string toString();

Location getLocation();
}
}

private signature module CompareSig<ResolvableSig R> {
predicate isResolvable(R::Source s);

R::Target resolve(R::Source s);
}

private module Compare<ResolvableSig R, CompareSig<R> RustAnalyzer, CompareSig<R> Ql> {
private import R

predicate same(Source s, Target t) {
t = RustAnalyzer::resolve(s) and
t = Ql::resolve(s)

Check warning

Code scanning / CodeQL

Redundant assignment. Warning

The variable t
has previously been assigned
the same value
.
}

predicate sameCount(int c) { c = count(Source s | same(s, _)) }

predicate diff(Source s, Target t1, Target t2) {
t1 = RustAnalyzer::resolve(s) and
t2 = Ql::resolve(s) and
t1 != t2
}

predicate diffCount(int c) { c = count(Source s | not same(s, _) and diff(s, _, _)) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we exclude those that are in same(s, _) here? If there's overlap I guess it's because one of the resolvers produce several results for a Source.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't want to report cases where our QL implementation resolves to multiple items, and one of them is correct. We have a consistency check for multiple resolutions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, that makes sense :)


predicate rustAnalyzerUnique(Source s) {
RustAnalyzer::isResolvable(s) and
not Ql::isResolvable(s)
}

predicate rustAnalyzerUniqueCount(int c) { c = count(Source s | rustAnalyzerUnique(s)) }

predicate qlUnique(Source s) {
not RustAnalyzer::isResolvable(s) and
Ql::isResolvable(s)
}

predicate qlUniqueCount(int c) { c = count(Source s | qlUnique(s)) }

predicate summary(string key, int value) {
key = "rust-analyzer unique" and rustAnalyzerUniqueCount(value)
or
key = "QL unique" and qlUniqueCount(value)
or
key = "same" and sameCount(value)
or
key = "different" and diffCount(value)
}
}

private module PathResolution implements ResolvableSig {
class Source extends Resolvable {
Source() { not this instanceof MethodCallExpr }
}

class Target = Item;
}

private module RustAnalyzerPathResolution implements CompareSig<PathResolution> {
predicate isResolvable(PathResolution::Source s) { s.hasResolvedPath() }

Item resolve(PathResolution::Source s) { s.resolvesAsItem(result) }
}

private module QlPathResolution implements CompareSig<PathResolution> {
private import codeql.rust.internal.PathResolution

private Path getPath(Resolvable r) {
result = r.(PathExpr).getPath()
or
result = r.(RecordExpr).getPath()
or
result = r.(PathPat).getPath()
or
result = r.(RecordPat).getPath()
or
result = r.(TupleStructPat).getPath()
}

predicate isResolvable(PathResolution::Source s) { exists(resolve(s)) }

Item resolve(PathResolution::Source s) { result = resolvePath(getPath(s)) }
}

module PathResolutionCompare =
Compare<PathResolution, RustAnalyzerPathResolution, QlPathResolution>;

private module CallGraph implements ResolvableSig {
class Source = CallExprBase;

class Target = Item;
}

private module RustAnalyzerCallGraph implements CompareSig<CallGraph> {
private import codeql.rust.elements.internal.CallExprBaseImpl::Impl as CallExprBaseImpl

predicate isResolvable(CallExprBase c) {
CallExprBaseImpl::getCallResolvable(c).hasResolvedPath()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this different from exists(resolve(c))? If it isn't, isResolvable wouldn't have to be on the signature as it would be exists(resolve(c)) in all cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, there is a subtle difference: Since we don't yet have entities for all dependencies in the DB (such as built-in types), we may reference that can resolve to a canonical path, but where there is no corresponding Item with the same canonical path.

}

Item resolve(CallExprBase c) { CallExprBaseImpl::getCallResolvable(c).resolvesAsItem(result) }
}

private module QlCallGraph implements CompareSig<CallGraph> {
private import codeql.rust.internal.PathResolution as PathResolution

predicate isResolvable(CallExprBase c) { exists(resolve(c)) }

Item resolve(CallExprBase c) { result = c.getStaticTarget() }
}

module CallGraphCompare = Compare<CallGraph, RustAnalyzerCallGraph, QlCallGraph>;