Skip to content

fix(mixing): resolves issue #4234#4473

Open
puckowski wants to merge 1 commit into
less:masterfrom
puckowski:master_fix_4234
Open

fix(mixing): resolves issue #4234#4473
puckowski wants to merge 1 commit into
less:masterfrom
puckowski:master_fix_4234

Conversation

@puckowski

@puckowski puckowski commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

What:

Fix mixin arity issue for mixins that provide a default value for arguments early in the list that caused wrong number of arguments error. Resolves #4234

Why:

This Less:

.mixin(@a) {
  value: @a;
}
.mixin(@a: 1, @b) {
  value: @a + @b;
}

.box {
  .mixin(3);
}

results in wrong number of arguments for .mixin (1 for 2) but should be valid.

Checklist:

  • Documentation
  • Added/updated unit tests
  • Code complete

Summary by CodeRabbit

  • Bug Fixes

    • Improved LESS mixin argument matching for positional and named arguments.
    • Added more accurate validation for required, optional, patterned, and variadic parameters.
    • Corrected handling of overloaded mixins with different argument patterns.
  • Tests

    • Added coverage for mixins called with positional and named arguments.
    • Added expected CSS output for the new arity-matching scenarios.

* Fix mixin arity issue for mixins that provide a default value for
  arguments early in the list that caused wrong number of arguments
  error.
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jul 19, 2026
@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b5cd4b2d-7572-409c-b1ce-4f0453c37462

📥 Commits

Reviewing files that changed from the base of the PR and between 937300c and 944caa6.

📒 Files selected for processing (3)
  • packages/less/lib/less/tree/mixin-definition.js
  • packages/test-data/tests-unit/mixins-named-args/mixins-named-args.css
  • packages/test-data/tests-unit/mixins-named-args/mixins-named-args.less

📝 Walkthrough

Walkthrough

Changes

The mixin matcher now assigns named and positional arguments by parameter slot, validates arity and required values, and evaluates pattern-like parameters across the full mixin arity. Tests add overloaded mixins with positional and named calls and expected CSS output.

Mixin argument matching

Layer / File(s) Summary
Argument assignment and pattern matching
packages/less/lib/less/tree/mixin-definition.js
Definition.matchArgs separates positional arguments, assigns named arguments to unfilled parameters, validates missing or extra arguments, and compares evaluated pattern values.
Positional and named arity coverage
packages/test-data/tests-unit/mixins-named-args/mixins-named-args.less, packages/test-data/tests-unit/mixins-named-args/mixins-named-args.css
Adds overloaded mixin definitions and verifies positional and named calls produce values 3 and 4.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately points to the mixin arity fix tied to issue #4234.
Linked Issues check ✅ Passed The change addresses the reported arity-matching bug and adds tests for positional and named mixin calls.
Out of Scope Changes check ✅ Passed The changes stay focused on mixin arity matching and related test fixtures, with no obvious unrelated edits.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 19, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes argument matching for Less mixins with required parameters after defaulted parameters. The main changes are:

  • Maps named arguments to their parameter slots before positional arguments.
  • Rejects unknown and excess arguments after slot assignment.
  • Adds positional and named regression cases for the corrected arity behavior.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • Argument matching now follows the same named-first binding model used during parameter evaluation.
  • Required, defaulted, variadic, and literal-pattern parameter paths retain explicit checks.

Reviews (1): Last reviewed commit: "fix(mixing): resolves issue #4234" | Re-trigger Greptile

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

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding a default value to a mixin causes arity match failure

1 participant