fix(migrations): keep brackets balanced when updating theme properties

[ChronosSF]

Closes #17642

What

UpdateChanges.updateThemeProps() matched the owner call with a non paren-aware regex:

const searchPattern = String.raw`${change.owner}\([\s\S]+?\);`;

For a theme function nested in another call — the documented @include scrollbar(scrollbar-theme($sb-size: 6px)); pattern — the match ran to the first ); and therefore swallowed the closing bracket of the surrounding call. That stray ) ended up attached to the last argument, and when that argument was one of the removed ones it was dropped along with it, producing invalid SCSS:

@include scrollbar(scrollbar-theme();

Since update-22_2_0 removes the whole $sb-* set from scrollbar-theme, practically every such usage broke. Real fallout in the samples app: IgniteUI/igniteui-angular-samples#4031.

The bug is in the shared UpdateChanges code, not in the 22.2.0 changes file, so any future type: "property" / remove: true theme change would have hit it too.

How

• findFunctionCalls / findClosingBracket locate each owner( and scan forward counting brackets to find that call's own ). Brackets inside strings and comments are ignored, and isLineCommentStart keeps url(https://…) from being mistaken for a // comment.
• The rewritten argument list is spliced back in by index instead of via String.replace(match, …), and the calls are rewritten back to front so the collected indices stay valid.
• The whitespace in front of the closing bracket is preserved separately, so multi-line theme calls keep their formatting exactly as before.
• splitFunctionProps now skips strings as well, so a ( or , inside a quoted value no longer throws off the argument split.

Dropping the \); anchor also means a theme call that is not terminated by ; on the same statement is now migrated instead of silently skipped.

Behavior

Input	Before	After
@include scrollbar(scrollbar-theme($sb-size: 6px));	@include scrollbar(scrollbar-theme(); ❌	@include scrollbar(scrollbar-theme());
@include scrollbar(scrollbar-theme($thumb-bg: red, $sb-size: 6px));	@include scrollbar(scrollbar-theme($thumb-bg: red); ❌	@include scrollbar(scrollbar-theme($thumb-bg: red));
@include scrollbar(scrollbar-theme($sb-size: 6px, $thumb-bg: red));	correct	unchanged
$t: scrollbar-theme($sb-size: 6px);	correct	unchanged

Formatting of the non-nested, single-theme-per-statement case — by far the common one — is byte-for-byte what it was; the existing specs cover that and pass untouched.

Tests

• UpdateChanges.spec.ts — new spec over the generic path: nested call with the only / last / first argument removed, a rename inside a nested call, and brackets and commas hiding in a string, a url() and a trailing comment.
• update-22_2_0/index.spec.ts — the two @include scrollbar(scrollbar-theme(…)) shapes from the samples repo, plus a theme call with no terminating ;.

Both new specs fail on master and pass with the fix. npm run test:schematics: 468 specs, 0 failures.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes migration parsing so nested theme calls remain balanced and safely handle strings/comments.

Changes:

• Replaces regex matching with parenthesis-aware call detection.
• Preserves formatting and supports calls without semicolons.
• Adds migration regression tests.

File summaries

File	Description
projects/igniteui-angular/migrations/update-22_2_0/index.spec.ts	Updated as part of this pull request.
projects/igniteui-angular/migrations/common/UpdateChanges.ts	Updated as part of this pull request.
projects/igniteui-angular/migrations/common/UpdateChanges.spec.ts	Updated as part of this pull request.

Review details

Suppressed comments (1)

projects/igniteui-angular/migrations/common/UpdateChanges.ts:968

• splitFunctionProps still treats commas in // comments as argument separators. For example, igx-theme-func($remove-me: 1, // keep this, too\n $prop: red) is split at keep this,, so the migration can leave comment text as a synthetic parameter and change or corrupt the call instead of only removing $remove-me. Since this parser is now used for the new nested-call path, skip line/block comments here as findClosingBracket does, and add a comma-in-comment regression case.

            if (char === '\'' || char === '"') {
                i = this.skipString(body, i);
                continue;
            }

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

[Copilot]

🔵 Needs a closer look

Argument parsing still treats commas inside Sass comments as separators.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

projects/igniteui-angular/migrations/common/UpdateChanges.ts:999

• splitFunctionProps still treats commas inside Sass comments as argument separators. For example, a removed property followed by // keep, this note can split the comment in the middle and the rebuilt argument list can lose the real separator before the next property, even though the new scanner explicitly promises to ignore comments. Skip skipNonCode spans here as well before processing commas/parentheses so comments cannot affect argument parsing.

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite