Skip to content

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Jan 28, 2025

Modelling work aimed at getting more results on tests.

  • make existing environment sources more accurate by modelling that they return a Result or Option.
  • add models for .expect and more variants of .unwrap.
  • allow implicit reads at taint sinks in the tests, since some of the existing test cases are written with that assumption.

@geoffw0 geoffw0 added no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code labels Jan 28, 2025
Copilot AI review requested due to automatic review settings January 28, 2025 08:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (5)

rust/ql/test/library-tests/dataflow/local/main.rs:241

  • This new test code is missing a hasValueFlow check, indicating incomplete coverage for the value flow from 48. Please add or update the test annotation.
    sink(s2.unwrap_or_else(|| source(48))); // $ MISSING: hasValueFlow=48

rust/ql/test/library-tests/dataflow/local/main.rs:268

  • An empty string is an uninformative error message; consider providing a descriptive message.
    sink(s1.expect("")); // $ hasValueFlow=78

rust/ql/test/library-tests/dataflow/local/main.rs:269

  • An empty string is an uninformative error message; consider providing a descriptive message.
    sink(s1.expect_err(""));

rust/ql/test/library-tests/dataflow/local/main.rs:272

  • An empty string is an uninformative error message; consider providing a descriptive message.
    sink(s2.expect(""));

rust/ql/test/library-tests/dataflow/local/main.rs:273

  • An empty string is an uninformative error message; consider providing a descriptive message.
    sink(s2.expect_err("")); // $ hasValueFlow=79

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

@geoffw0
Copy link
Contributor Author

geoffw0 commented Jan 28, 2025

The DCA run was uneventful.

@geoffw0
Copy link
Contributor Author

geoffw0 commented Jan 28, 2025

Second DCA run also LGTM.

@geoffw0 geoffw0 merged commit a42c0f6 into github:main Jan 29, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants