Skip to content

C#: Remove some FPs for cs/simplifiable-boolean-expression. - #22708

Draft
michaelnebel wants to merge 7 commits into
github:mainfrom
michaelnebel:csharp/improvesimplifyboolexpr
Draft

michaelnebel wants to merge 7 commits into
github:mainfrom
michaelnebel:csharp/improvesimplifyboolexpr

Conversation

@michaelnebel

@michaelnebel michaelnebel commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

It turns out that the query cs/simplifiable-boolean-expression produces some false positives, when the extracted data has incomplete type information.
The following example produces a false positive

public class Container {
    public bool? Field;
}

public void Fn() {
    // Emulating incomplete type information by not declaring container explicitly.
    if (container.Field == true) ;
}

If type information had been complete, for instance

public void Fn(Container container) {
       if (container.Field == true) ;
}

then the query didn't produce an alert prior to the changes in this PR. In this case an alert is NOT produced because the boolean literal true on the right hand side of == is wrapped in a cast.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Conditional rewrites still accept non-Boolean or unknown condition types, allowing invalid simplification suggestions.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Reduces false positives in C# Boolean-expression simplification when type information is incomplete or nullable.

Changes:

  • Requires Boolean operand types before suggesting rewrites.
  • Adds nullable and incomplete-type regression cases.
  • Adds a change note.
File Description
csharp/​ql/​src/​Language Abuse/​SimplifyBoolExpr.ql Adds type-aware rewrite predicates.
csharp/​ql/​test/​query-tests/​Language Abuse/​SimplifyBoolExpr/​SimplifyBoolExpr.cs Tests nullable Boolean comparison.
csharp/​ql/​test/​query-tests/​standalone/​Language Abuse/​SimplifyBoolExpr/​SimplifyBoolExpr.cs Tests incomplete type information.
csharp/​ql/​src/​change-notes/​2026-09-30-simplify-bool-expr.md Documents reduced false positives.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql
Comment thread csharp/ql/test/query-tests/Language Abuse/SimplifyBoolExpr/SimplifyBoolExpr.cs Outdated
@michaelnebel
michaelnebel requested a balanced review from Copilot September 30, 2026 10:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 Medium severity · 2 Low severity

Open (3)
Resolved since last review (2)

Comment thread csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql
Comment thread csharp/ql/src/Language Abuse/SimplifyBoolExpr.ql

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants