Skip to content

Refactor IndexMatcher control flow to reduce CodeQL complexity hotspots#311

Merged
albe merged 3 commits into
mainfrom
copilot/reduce-complexity-indexmatcher
May 10, 2026
Merged

Refactor IndexMatcher control flow to reduce CodeQL complexity hotspots#311
albe merged 3 commits into
mainfrom
copilot/reduce-complexity-indexmatcher

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 10, 2026

IndexMatcher was flagged for elevated cyclomatic complexity, with forEachMatch called out as the primary hotspot and the deepest nesting around candidate match evaluation. This change reduces branching depth and conditional nesting in-place, without restructuring via helper extraction.

  • forEachMatch: flatten candidate resolution and fallback evaluation

    • Replaced nested condition blocks with guard-continue flow when discriminant values are missing/invalid.
    • Kept discriminant lookup + full matches(...) verification semantics unchanged.
    • Preserved separate iteration over unclassified and function matchers while removing unnecessary per-call allocations.
  • add / remove: simplify classification branches

    • Converted nested object/discriminant checks into early-return paths.
    • Kept matcher routing behavior identical:
      • function matchers → functionMatchers
      • object matchers with discriminant → lookup table
      • everything else → unclassifiedMatchers
  • Behavioral intent

    • No algorithmic redesign; this is a control-flow refactor to lower complexity/readability risk in the existing matcher pipeline.
for (const propPath of this.properties) {
    const docValue = getPropertyAtPath(document, propPath);
    if (docValue === undefined || docValue === null || typeof docValue === 'object') continue;

    const indexSet = this.table.get(propPath)?.get(String(docValue));
    if (!indexSet) continue;

    for (const indexName of indexSet) {
        if (matches(document, this.matchers.get(indexName))) iterationHandler(indexName);
    }
}

@albe albe marked this pull request as ready for review May 10, 2026 12:15
@coveralls
Copy link
Copy Markdown

coveralls commented May 10, 2026

Coverage Status

coverage: 98.106% (+0.1%) from 97.964% — copilot/reduce-complexity-indexmatcher into main

@albe
Copy link
Copy Markdown
Owner

albe commented May 10, 2026

@copilot where is the reduced code coverage coming from? If the logic didn't change coverage shouldn't be affected, no?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 10, 2026

@copilot where is the reduced code coverage coming from? If the logic didn't change coverage shouldn't be affected, no?

Good catch — coverage dropped because the refactor changed how c8/istanbul instruments branches, even though behavior stayed the same. In IndexMatcher.js the new guard-style early returns/continues in add/remove/forEachMatch created separately counted branch paths, and some of those specific paths are not exercised by current tests (currently reported around lines 95-112). So this is a branch-coverage accounting change from control-flow shape, not a logic regression. If you want, I can add focused tests for those missing paths to restore the previous coverage level.

Copilot finished work on behalf of albe May 10, 2026 12:19
Copilot AI requested a review from albe May 10, 2026 12:19
@albe
Copy link
Copy Markdown
Owner

albe commented May 10, 2026

@copilot yes add new tests that covers this code branch to keep coverage same or even improve it

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 10, 2026

@copilot yes add new tests that covers this code branch to keep coverage same or even improve it

Added focused tests in commit 33e19d5 to cover the missing IndexMatcher.remove() branches (unknown index, function matcher, non-object matcher, and object matcher without discriminant). Coverage is now improved and IndexMatcher.js reports 100% statements/branches/functions/lines.

Copilot finished work on behalf of albe May 10, 2026 13:15
@albe albe merged commit dcdd143 into main May 10, 2026
13 checks passed
@albe albe deleted the copilot/reduce-complexity-indexmatcher branch May 10, 2026 13:29
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