Skip to content

Rust: Update legacy MaD models 3 #19946

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.

Sign up for GitHub

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
geoffw0 merged 14 commits into from
Jul 14, 2025
Merged

Rust: Update legacy MaD models 3 #19946

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3027f75
Rust: Translate more legacy models -> new models (from data).
geoffw0 Jun 19, 2025
8d0c14c
Merge branch 'main' into models3b
geoffw0 Jul 9, 2025
097ac69
Rust: Current sources test regressions.
geoffw0 Jul 9, 2025
a034e29
Rust: Simplify the test a little.
geoffw0 Jul 9, 2025
47a4ba3
Rust: Fix typo in models (also fixed in another open PR).
geoffw0 Jul 9, 2025
7ba18fa
Merge branch 'main' into models3b
geoffw0 Jul 10, 2025
7507834
Rust: Accept .expected changes (mostly renumberings).
geoffw0 Jul 10, 2025
01c75e3
Rust: The rusqlite row.get() calls are missing a canonical path.
geoffw0 Jul 10, 2025
6de5a61
Rust: Accept consistency changes as well.
geoffw0 Jul 10, 2025
33ea822
Rust: Workaround for type inference issue in the test.
geoffw0 Jul 11, 2025
68a37f9
Rust: Add something similar as a type inference test case.
geoffw0 Jul 11, 2025
e20ae48
Merge branch 'main' into models3b
geoffw0 Jul 11, 2025
05e1cd4
Rust: Fix garbled merge.
geoffw0 Jul 11, 2025
be7db80
Rust: Accept consistency check change (from CI).
geoffw0 Jul 14, 2025
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
6 changes: 0 additions & 6 deletions rust/ql/test/library-tests/frameworks/rusqlite/Rusqlite.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +0,0 @@
| main.rs:34:37:34:54 | //... | Missing result: database-read |
| main.rs:35:37:35:54 | //... | Missing result: database-read |
| main.rs:36:37:36:54 | //... | Missing result: database-read |
| main.rs:43:41:43:58 | //... | Missing result: database-read |
| main.rs:44:41:44:58 | //... | Missing result: database-read |
| main.rs:45:41:45:58 | //... | Missing result: database-read |
16 changes: 8 additions & 8 deletions rust/ql/test/library-tests/frameworks/rusqlite/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,27 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
)",
(),
)?;

let query = format!("INSERT INTO person (name, age) VALUES ('{}', '{}')", name, age);

connection.execute(&query, ())?; // $ sql-sink

let person = connection.query_row(&query, (), |row| { // $ sql-sink
Ok(Person {
Comment thread
geoffw0 marked this conversation as resolved.
id: row.get(0)?, // $ database-read
name: row.get(1)?, // $ database-read
age: row.get(2)?, // $ database-read
id: row.get(0)?, // $ MISSING: database-read
Copy link
Copy Markdown
Contributor

@aibaars aibaars Jul 10, 2025

Choose a reason for hiding this comment

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

Are you sure the problem is a missing canonical path, or is the problem that getStaticTarget fails (due to type inferencer problem)?

Copy link
Copy Markdown
Contributor

@aibaars aibaars Jul 10, 2025

Choose a reason for hiding this comment

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

Just checked the canonical path exists, the problem is the type inferencer, most likely the closure argument.

Copy link
Copy Markdown
Contributor Author

@geoffw0 geoffw0 Jul 10, 2025

Choose a reason for hiding this comment

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

Yep you're right, the getStaticTarget fails (and thus getStaticTarget().(Addressable).getCanonicalPath() fails too) so it's likely a type inference problem.

Copy link
Copy Markdown
Contributor

@aibaars aibaars Jul 11, 2025

Choose a reason for hiding this comment

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

Yeah that's why I use a concat in my test predicate:

predicate test(MethodCallExpr e, Function target, string path) {
  target = e.getStaticTarget() and
  path = concat(target.getCanonicalPath())
}
  • no result means getStaticTarget() failed
  • empty string means: a target was found, but getCanonicalPath() failed

Copy link
Copy Markdown
Contributor

@hvitved hvitved Jul 11, 2025

Choose a reason for hiding this comment

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

most likely the closure argument.

Correct; type inference for closures is a known gap.

Yeah that's why I use a concat in my test predicate:

I can also recommend the various debug predicates in TypeInference.qll; change getRelevantLocatable to the relevant file+line, and then quick-eval debugInferType et al.

Copy link
Copy Markdown
Contributor Author

@geoffw0 geoffw0 Jul 11, 2025

Choose a reason for hiding this comment

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

I will try the TypeInference.qll debug predicates, thanks for reminding me those exist.

name: row.get(1)?, // $ MISSING: database-read
age: row.get(2)?, // $ MISSING: database-read
})
})?;

let mut stmt = connection.prepare("SELECT id, name, age FROM person")?; // $ sql-sink
let people = stmt.query_map([], |row| {
Ok(Person {
Comment thread
geoffw0 marked this conversation as resolved.
id: row.get_unwrap(0), // $ database-read
name: row.get_unwrap(1), // $ database-read
age: row.get_unwrap(2), // $ database-read
id: row.get_unwrap(0), // $ MISSING: database-read
name: row.get_unwrap(1), // $ MISSING: database-read
age: row.get_unwrap(2), // $ MISSING: database-read
})
})?;

Ok(())
}
}