Skip to content

Conversation

@Pixel998
Copy link
Contributor

@Pixel998 Pixel998 commented Aug 27, 2025

Prerequisites checklist

What is the purpose of this pull request?

Fix the var() parsing to handle multiline fallbacks safely and avoid super-linear backtracking.

What changes did you make? (Give an overview)

  • Updated the var() regex to allow newline characters in the fallback and to avoid adjacent patterns that can cause super-linear backtracking.

Related Issues

#229 (comment)

Is there anything you'd like reviewers to focus on?

@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Aug 27, 2025
@eslint-github-bot eslint-github-bot bot added the bug Something isn't working label Aug 27, 2025
@mdjermanovic mdjermanovic changed the title fix: make var() regex multiline-safe fix: make var() regex multiline-safe in no-invalid-properties Aug 27, 2025
@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion contributor pool labels Aug 27, 2025
@mdjermanovic mdjermanovic moved this from Needs Triage to Implementing in Triage Aug 27, 2025
@mdjermanovic mdjermanovic changed the title fix: make var() regex multiline-safe in no-invalid-properties feat: make var() regex multiline-safe in no-invalid-properties Aug 27, 2025
@mdjermanovic
Copy link
Member

Changed the tag to feat since this change can produce more lint errors (fixes some false negatives).

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

To clarify the change in behavior: the new regex matches var()'s with newlines in or after the fallback text (between the fallback text and the closing parenthesis), which the old regex didn't match.

":root { --fallback-color: blue; }\na { color: VAR(--MY-COLOR, VaR(--fallback-color)) }",
"a { color: var(--my-color,\n red) }",
":root { --my-color: red; }\na { color: var(--my-color,\n blue) }",
"a { color: var(--x,\n var(--y,\n blue\n )\n) }",
Copy link
Member

Choose a reason for hiding this comment

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

This one was invalid before this change (false positive).

":root { --my-color: red; }\na { color: var(--my-color,\n blue) }",
"a { color: var(--x,\n var(--y,\n blue\n )\n) }",
"a { color: var(--x,\n red ) }",
":root { --a: var(--b,\n 10px\n); } a { padding: var(--a); }",
Copy link
Member

Choose a reason for hiding this comment

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

This one was invalid before this change (false positive).

Comment on lines +1000 to +1017
{
code: "a { padding-top: var(--a,\nvar(--b, red\n)\n); }",
options: [{ allowUnknownVariables: true }],
errors: [
{
messageId: "invalidPropertyValue",
data: {
property: "padding-top",
value: "red",
expected: "<length-percentage [0,∞]>",
},
line: 1,
column: 18,
endLine: 4,
endColumn: 2,
},
],
},
Copy link
Member

Choose a reason for hiding this comment

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

This one was valid before this change (false negative).

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Leaving open for a second review.

@mdjermanovic mdjermanovic moved this from Implementing to Second Review Needed in Triage Aug 27, 2025
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@nzakas nzakas merged commit c123f6e into eslint:main Aug 27, 2025
22 checks passed
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Aug 27, 2025
@Pixel998 Pixel998 deleted the no-invalid-properties-var-regex-multiline branch September 6, 2025 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion bug Something isn't working contributor pool feature

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

3 participants