Fix unit-no-unknown false positives for the second and subsequent image-set() with x descriptor

[chalkygames123]

What steps are needed to reproduce the bug?

Just lint the following CSS with unit-no-unknown turned on.

test.css:

a {
	background-image: url('first.png'),
		image-set(url('second.png') 1x, url('second@2x.png') 2x);
	background-image: url('first.png'), url('second.png'),
		image-set(url('third.png') 1x, url('third@2x.png') 2x);
}

What Stylelint configuration is needed to reproduce the bug?

{
  "rules": {
    "unit-no-unknown": [
      true
    ]
  }
}

How did you run Stylelint?

CLI with stylelint test.css

Which version of Stylelint are you using?

15.3.0

What did you expect to happen?

No problems to be reported.

What actually happened?

The unit-no-unknown rule reports the following problems when using the x unit in a value for an image-set() function.

test.css
 3:32  ✖  Unexpected unknown unit "x"  unit-no-unknown
 3:57  ✖  Unexpected unknown unit "x"  unit-no-unknown
 5:31  ✖  Unexpected unknown unit "x"  unit-no-unknown
 5:55  ✖  Unexpected unknown unit "x"  unit-no-unknown

Related: #4654

Does the bug relate to non-standard syntax?

No.

Proposal to fix the bug

No response

[ybiquitous]

@chalkygames123 Thanks for the report using the template. We can reproduce the problem with the demo.

As a workaround, you can avoid the problem using the ignoreFunctions secondary option like this:

{
  "rules": {
    "unit-no-unknown": [true, {"ignoreFunctions": ["image-set"]}]
  }
}

I guess we can fix the false positive by changing the following code:

stylelint/lib/rules/unit-no-unknown/index.js

Line 121 in 964b1ba

if (/^(?:-webkit-)?image-set[\s(]/i.test(value)) {

I've labeled the issue as ready to implement. Please consider contributing if you have time.

There are steps on how to fix a bug in a rule in the Developer guide.

[chalkygames123]

Thank you for the guidance, I'm on it!

[chalkygames123]

@ybiquitous Could you give me some explanation especially about what these lines do because I'm not familiar with the Stylelint codebase?

stylelint/lib/rules/unit-no-unknown/index.js

Lines 133 to 135 in 964b1ba

	if (imageSetValueLastIndex >= valueNode.sourceIndex) {
	return;
	}

[ybiquitous]

@chalkygames123 The lines you pointed out came from PR #4427. I'm not sure since the PR is old, but maybe I guess the lines handle the following case (see left 20x after image-set())

stylelint/lib/rules/unit-no-unknown/__tests__/index.js

Lines 459 to 467 in 964b1ba

	{
	code: "a { background: /* comment */ image-set('img1x.png' 1x, 'img2x.png' 2x) left 20x / 15% 60% repeat-x; }",
	message: messages.rejected('x'),
	description: '`x` rejected with inappropriate property',
	line: 1,
	column: 80,
	endLine: 1,
	endColumn: 81,
	},

This logic's implementation has a few deeper nested if blocks, so it seems hard to read. It may be good refactoring timing if you wish.

[chalkygames123]

Thank you. So I'll open a draft PR with minimal changes, then let me have some time to refactor it.

[romainmenke]

Was looking at this because it is relevant for the migration to a different parser for media queries.

Why isn't x present in the list of known units?
x is just an alias for dppx.

https://github.com/stylelint/stylelint/blob/main/lib/reference/units.js#L79-L82

	// Resolution
	'dpi',
	'dpcm',
	'dppx',

specification : https://drafts.csswg.org/css-values-4/#resolution

mdn : https://developer.mozilla.org/en-US/docs/Web/CSS/resolution

[ybiquitous]

x is just an alias for dppx.

Oh, I didn't know it. The reason to ignore x is to avoid false positives for image-set(). See:

• Add ignoreFunctions: [] to unit-no-unknown #3424
• Add support for x to unit-no-unknown #3770

[romainmenke]

If I read those correctly the main motivation seems to be that x is often a typo for px and that x is only valid in image-set() property values and in the resolution media query?

[ybiquitous]

@romainmenke Yes, your understanding is correct. 👍🏼