Skip to content

feat(calc): add implicit intersection and fix regex criteria anchoring#2268

Merged
xuri merged 5 commits intoqax-os:masterfrom
jpoz:jpoz/fix-matrix
Mar 8, 2026
Merged

feat(calc): add implicit intersection and fix regex criteria anchoring#2268
xuri merged 5 commits intoqax-os:masterfrom
jpoz:jpoz/fix-matrix

Conversation

@jpoz
Copy link
Contributor

@jpoz jpoz commented Feb 27, 2026

PR Details

Description

This PR implements two related fixes for the calculation engine:

  1. Implicit Intersection: Adds implicitIntersect method that resolves matrix arguments to scalar values based on the formula cell's row position. This matches Excel's behavior where range references in non-array formulas resolve to the cell in the same row as the formula. Applied to ABS, ISNUMBER, and IF functions.

  2. Regex Criteria Anchoring Fix: Moves ^/$ anchor insertion from formulaCriteriaParser to formulaCriteriaEval. The previous approach (anchoring at parse time) interfered with numeric criteria parsing since "^5$" cannot convert to a number. Anchoring at eval time preserves correct ToNumber() behavior while still enforcing exact matching in SUMIF/COUNTIF.

  3. ISNUMBER matrix bug fix: The previous matrix handling code in ISNUMBER had a missing else branch, causing it to append both true and false for number cells, corrupting the output matrix. Replaced with implicitIntersect for correct scalar resolution.

Related Issue

Fixes implicit intersection behavior for scalar functions receiving range/matrix arguments.

Motivation and Context

When a whole-column or range reference is passed to a scalar function like ABS(A1:A5), Excel resolves it to the single cell in the same row as the formula cell (implicit intersection). Without this fix, the calculation engine could not handle these common spreadsheet patterns correctly. The regex anchoring fix ensures criteria-based functions (SUMIF, COUNTIF, etc.) correctly match exact values without breaking numeric criteria handling.

How Has This Been Tested

  • Added TestCalcImplicitIntersect: validates ABS, ISNUMBER, and IF with range references across multiple rows, confirming each resolves to the correct row via implicit intersection (11 assertions)
  • Added TestCalcCriteriaRegexpAnchoring: validates exact matching, wildcard patterns (*, ?), and numeric criteria for SUMIF/COUNTIF
  • All existing tests pass including TestCalcSUMIFExactMatch, TestCalcAVERAGEIF, and TestCalcCellValue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

jpoz added 2 commits February 20, 2026 16:10
Modified formulaCriteriaParser to wrap regex patterns with ^ and $.
Prevents partial string matches in SUMIF/COUNTIF functions when
wildcards are not explicitly used.

Test verifies "administrative" matches exactly 2 cells, not 4
substring matches like "cyclical_flat_administrative".
Implements Excel's implicit intersection behavior where matrix
arguments to scalar functions resolve to single cells based on
formula position. Functions like ABS and IF now correctly handle
range references in non-array formulas.

Fixes regex criteria anchoring to prevent partial string matches
in SUMIF/COUNTIF functions. Patterns now anchor properly at
runtime rather than during parsing.
@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.56%. Comparing base (37b6a8f) to head (8611d37).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2268   +/-   ##
=======================================
  Coverage   99.56%   99.56%           
=======================================
  Files          32       32           
  Lines       26316    26322    +6     
=======================================
+ Hits        26201    26207    +6     
  Misses         60       60           
  Partials       55       55           
Flag Coverage Δ
unittests 99.56% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Cover the error path (invalid cell name) and out-of-bounds row index
branches to satisfy codecov patch coverage requirements.
@xuri xuri added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 27, 2026
Copy link

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.

Pull request overview

This PR updates the calculation engine to better match Excel semantics by adding implicit intersection handling for certain scalar functions and adjusting how criteria regex anchoring is applied during SUMIF/COUNTIF-style evaluations.

Changes:

  • Added implicitIntersect helper and applied it to ABS, ISNUMBER, and IF to resolve matrix/range arguments to scalars.
  • Moved regex ^/$ anchoring behavior into formulaCriteriaEval for criteriaRegexp.
  • Added tests covering implicit intersection and criteria regex anchoring behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
calc.go Adds implicitIntersect, applies it to functions, and changes regex anchoring logic in criteria evaluation.
calc_test.go Adds new tests for implicit intersection and criteria anchoring behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@xuri xuri moved this to Features in Excelize v2.11.0 Mar 6, 2026
Copy link
Member

@xuri xuri left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

@xuri xuri merged commit 7634227 into qax-os:master Mar 8, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

Status: Features

Development

Successfully merging this pull request may close these issues.

4 participants