Skip to content

Add validation for unbalanced parentheses in rules - #393

Merged
Giulia Stocco (gfs) merged 2 commits into
mainfrom
gfs/ValidatorReportUnbalancedParens
Aug 13, 2026
Merged

Add validation for unbalanced parentheses in rules#393
Giulia Stocco (gfs) merged 2 commits into
mainfrom
gfs/ValidatorReportUnbalancedParens

Conversation

@gfs

Copy link
Copy Markdown
Contributor

No description provided.

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.

Pull request overview

Adds additional validation to detect unbalanced parentheses in rule expressions during analysis, and extends unit coverage to catch a previously-accepted malformed expression.

Changes:

  • Add a post-parse validation in Analyzer.EnumerateRuleIssues(Rule rule) to flag expressions with unmatched parentheses counts.
  • Add a unit test case for an expression with an opening parenthesis attached to a token (e.g., "(0 AND 1").
Show a summary per file
File Description
OAT/Analyzer.cs Adds an end-of-parse check intended to detect unbalanced parentheses in rule expressions.
OAT.Tests/ExpressionsTests.cs Adds a regression test for a rule expression with an unmatched opening parenthesis without a separating space.

Review details

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread OAT/Analyzer.cs

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.

Review details

Suppressed comments (2)

OAT/Analyzer.cs:398

  • The new unbalanced-parentheses validation only checks total '(' vs ')' counts. This misses expressions where parentheses close before they open but totals still match (e.g. 0) AND (1), which can currently pass validation. Consider also rejecting any expression where the running parentheses balance ever goes negative, while still keeping the end-of-expression count check.
                if (foundStarts != foundEnds)
                {
                    yield return new Violation(string.Format(Strings.Get("Err_ClauseUnbalancedParentheses"), expression, rule.Name), rule);
                }

OAT.Tests/ExpressionsTests.cs:438

  • Tests cover extra opening/closing parentheses, but not the case where the total counts match while the order is invalid (a close before any open), which is an easy regression to miss (e.g. 0) AND (1). Adding a case here will lock in the intended validation behavior.
            Assert.False(analyzer.IsRuleValid(invalidRule));

            invalidRule = new Rule("UnbalancedOpenParentheses")
            {
                Expression = "(0 AND 1",
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@gfs
Giulia Stocco (gfs) merged commit 9f50413 into main Aug 13, 2026
14 checks passed
@gfs
Giulia Stocco (gfs) deleted the gfs/ValidatorReportUnbalancedParens branch August 13, 2026 14:12
Giulia Stocco (gfs) added a commit to microsoft/ApplicationInspector that referenced this pull request Aug 16, 2026
The engine's expression validation only reported unbalanced parentheses when
closing ones outnumbered opening ones, so a trailing unclosed group passed
validation and then threw part way through a scan. That is fixed upstream in
microsoft/OAT#393, and the characterization tests now assert the unclosed cases
are reported rather than missed.

Evaluating an unbalanced expression still throws, so verification remains the
gate that keeps a malformed rule away from the analyzer, and the balance check
in RulesVerifier is kept: it produces an actionable rule error rather than an
engine violation, and it still holds if the package is pinned back.

Both projects referencing the package are moved together; AppInspector.Commands
also pinned it.
Giulia Stocco (gfs) added a commit to microsoft/ApplicationInspector that referenced this pull request Aug 18, 2026
The engine's expression validation only reported unbalanced parentheses when
closing ones outnumbered opening ones, so a trailing unclosed group passed
validation and then threw part way through a scan. That is fixed upstream in
microsoft/OAT#393, and the characterization tests now assert the unclosed cases
are reported rather than missed.

Evaluating an unbalanced expression still throws, so verification remains the
gate that keeps a malformed rule away from the analyzer, and the balance check
in RulesVerifier is kept: it produces an actionable rule error rather than an
engine violation, and it still holds if the package is pinned back.

Both projects referencing the package are moved together; AppInspector.Commands
also pinned it.
Giulia Stocco (gfs) added a commit to microsoft/ApplicationInspector that referenced this pull request Aug 18, 2026
The engine's expression validation only reported unbalanced parentheses when
closing ones outnumbered opening ones, so a trailing unclosed group passed
validation and then threw part way through a scan. That is fixed upstream in
microsoft/OAT#393, and the characterization tests now assert the unclosed cases
are reported rather than missed.

Evaluating an unbalanced expression still throws, so verification remains the
gate that keeps a malformed rule away from the analyzer, and the balance check
in RulesVerifier is kept: it produces an actionable rule error rather than an
engine violation, and it still holds if the package is pinned back.

Both projects referencing the package are moved together; AppInspector.Commands
also pinned it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants