Add function-url-quotes autofix (#6558)

Closes #6500. > Is there anything in the PR that needs further explanation? ~~Two possible code smells I want to call out. In both cases, I opted to pick a "dirty" solution that doesn't change existing behaviour and minimizes/localizes the changed code. However, there are probably more correct/elegant ways to do this. Advice is welcome!~~ I was able to address these thanks to code review, but I'll leave the comments for posterity. --- First, this rule operates on both declarations and at-rules. It seems like in `checkArgs`, this type information is "erased". To resolve this, I have my utility functions `addQuotes` and `removeQuotes` take in a union type, and then use a crude `'params' in node` to differentiate the type. This ... feels wrong? I was a bit unsure if there's a better way for me to do this (either from a TS perspective, or perhaps a Stylelint util that I'm missing). --- Secondly, the previous iteration of this rule lowercases declarations and at-rule parameters before passing them in as arguments to `functionArgumentsSearch`. This ensures that the rule matches `UrL` and the like. However, this has the side effect of lowercasing the declaration value and/or at-rule parameters. Then, when I apply the autofix, I propagate this lowercase change (which means that the autofix also inadvertently lowercases the code). To resolve this problem, I pass in a case-insensitive regex to `functionArgumentsSearch`. I don't think this is *technically* wrong, but it is likely a performance regression; I didn't want to change `functionArgumentsSearch` implementation instead. With that in mind, any suggestions on better ways to do this? Thanks in advance - implementing this autofix was trickier than I thought! Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
stylelint · Jan 11, 2023 · 6c74739 · 6c74739
1 parent b4fd2d5
commit 6c74739
Show file tree

Hide file tree

Showing 7 changed files with 136 additions and 9 deletions.
diff --git a/.changeset/rude-dryers-fry.md b/.changeset/rude-dryers-fry.md
@@ -0,0 +1,5 @@
+---
+"stylelint": minor
+---
+
+Added: `function-url-quotes` autofix
diff --git a/docs/user-guide/rules.md b/docs/user-guide/rules.md
@@ -238,7 +238,7 @@ Enforce naming conventions with these `pattern` rules.
 Require or disallow quotes with these `quotes` rules.
 
 - [`font-family-name-quotes`](../../lib/rules/font-family-name-quotes/README.md): Require or disallow quotes for font family names (Autofixable).
-- [`function-url-quotes`](../../lib/rules/function-url-quotes/README.md): Require or disallow quotes for urls.
+- [`function-url-quotes`](../../lib/rules/function-url-quotes/README.md): Require or disallow quotes for urls (Autofixable).
 - [`selector-attribute-quotes`](../../lib/rules/selector-attribute-quotes/README.md): Require or disallow quotes for attribute values (Autofixable).
 
 ### Redundant

diff --git a/lib/rules/function-url-quotes/README.md b/lib/rules/function-url-quotes/README.md
@@ -9,6 +9,8 @@ a { background: url("x.jpg") }
  *             These quotes */
 ```
 
+The [`fix` option](../../../docs/user-guide/usage/options.md#fix) can automatically fix most of the problems reported by this rule.
+
 ## Options
 
 `string`: `"always"|"never"`

diff --git a/lib/rules/function-url-quotes/__tests__/index.js b/lib/rules/function-url-quotes/__tests__/index.js
@@ -5,6 +5,7 @@ const { messages, ruleName } = require('..');
 testRule({
 	ruleName,
 	config: ['always'],
+	fix: true,
 
 	accept: [
 		{
@@ -180,6 +181,7 @@ testRule({
 	reject: [
 		{
 			code: '@import url(foo.css);',
+			fixed: '@import url("foo.css");',
 			message: messages.expected('url'),
 			line: 1,
 			column: 13,
@@ -188,6 +190,7 @@ testRule({
 		},
 		{
 			code: '@import url( foo.css );',
+			fixed: '@import url( "foo.css" );',
 			message: messages.expected('url'),
 			line: 1,
 			column: 14,
@@ -196,6 +199,7 @@ testRule({
 		},
 		{
 			code: '@document url-prefix(http://www.w3.org/Style);',
+			fixed: '@document url-prefix("http://www.w3.org/Style");',
 			message: messages.expected('url-prefix'),
 			line: 1,
 			column: 22,
@@ -204,6 +208,7 @@ testRule({
 		},
 		{
 			code: '@document url-prefix( http://www.w3.org/Style );',
+			fixed: '@document url-prefix( "http://www.w3.org/Style" );',
 			message: messages.expected('url-prefix'),
 			line: 1,
 			column: 23,
@@ -212,6 +217,7 @@ testRule({
 		},
 		{
 			code: "@font-face { font-family: 'foo'; src: url(foo.ttf); }",
+			fixed: '@font-face { font-family: \'foo\'; src: url("foo.ttf"); }',
 			message: messages.expected('url'),
 			line: 1,
 			column: 43,
@@ -220,6 +226,7 @@ testRule({
 		},
 		{
 			code: "@font-face { font-family: 'foo'; src: url( foo.ttf ); }",
+			fixed: '@font-face { font-family: \'foo\'; src: url( "foo.ttf" ); }',
 			message: messages.expected('url'),
 			line: 1,
 			column: 44,
@@ -228,6 +235,7 @@ testRule({
 		},
 		{
 			code: 'a { cursor: url(foo.png); }',
+			fixed: 'a { cursor: url("foo.png"); }',
 			message: messages.expected('url'),
 			line: 1,
 			column: 17,
@@ -236,6 +244,7 @@ testRule({
 		},
 		{
 			code: 'a { background-image: url(foo.css), url("bar.css"), url("baz.css"); }',
+			fixed: 'a { background-image: url("foo.css"), url("bar.css"), url("baz.css"); }',
 			message: messages.expected('url'),
 			line: 1,
 			column: 27,
@@ -244,6 +253,7 @@ testRule({
 		},
 		{
 			code: 'a { background-image: url( foo.css ), url("bar.css"), url("baz.css"); }',
+			fixed: 'a { background-image: url( "foo.css" ), url("bar.css"), url("baz.css"); }',
 			message: messages.expected('url'),
 			line: 1,
 			column: 28,
@@ -252,6 +262,7 @@ testRule({
 		},
 		{
 			code: 'a { background-image: url("foo.css"), url(bar.css), url("baz.css"); }',
+			fixed: 'a { background-image: url("foo.css"), url("bar.css"), url("baz.css"); }',
 			message: messages.expected('url'),
 			line: 1,
 			column: 43,
@@ -260,6 +271,7 @@ testRule({
 		},
 		{
 			code: 'a { background-image: url("foo.css"), url( bar.css ), url("baz.css"); }',
+			fixed: 'a { background-image: url("foo.css"), url( "bar.css" ), url("baz.css"); }',
 			message: messages.expected('url'),
 			line: 1,
 			column: 44,
@@ -268,6 +280,7 @@ testRule({
 		},
 		{
 			code: 'a { background-image: url("foo.css"), url("bar.css"), url(baz.css); }',
+			fixed: 'a { background-image: url("foo.css"), url("bar.css"), url("baz.css"); }',
 			message: messages.expected('url'),
 			line: 1,
 			column: 59,
@@ -276,6 +289,7 @@ testRule({
 		},
 		{
 			code: 'a { background-image: url("foo.css"), url("bar.css"), url( baz.css ); }',
+			fixed: 'a { background-image: url("foo.css"), url("bar.css"), url( "baz.css" ); }',
 			message: messages.expected('url'),
 			line: 1,
 			column: 60,
@@ -288,6 +302,7 @@ testRule({
 testRule({
 	ruleName,
 	config: ['never'],
+	fix: true,
 
 	accept: [
 		{
@@ -397,6 +412,7 @@ testRule({
 	reject: [
 		{
 			code: '@import url("foo.css");',
+			fixed: '@import url(foo.css);',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 13,
@@ -405,6 +421,7 @@ testRule({
 		},
 		{
 			code: '@import uRl("foo.css");',
+			fixed: '@import uRl(foo.css);',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 13,
@@ -413,6 +430,7 @@ testRule({
 		},
 		{
 			code: '@import URL("foo.css");',
+			fixed: '@import URL(foo.css);',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 13,
@@ -421,6 +439,7 @@ testRule({
 		},
 		{
 			code: '@import url( "foo.css" );',
+			fixed: '@import url( foo.css );',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 14,
@@ -429,6 +448,7 @@ testRule({
 		},
 		{
 			code: "@import url('foo.css');",
+			fixed: '@import url(foo.css);',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 13,
@@ -437,6 +457,7 @@ testRule({
 		},
 		{
 			code: "@import url( 'foo.css' );",
+			fixed: '@import url( foo.css );',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 14,
@@ -445,6 +466,7 @@ testRule({
 		},
 		{
 			code: '@document url("http://www.w3.org/");',
+			fixed: '@document url(http://www.w3.org/);',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 15,
@@ -453,6 +475,7 @@ testRule({
 		},
 		{
 			code: '@document url( "http://www.w3.org/" );',
+			fixed: '@document url( http://www.w3.org/ );',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 16,
@@ -461,6 +484,7 @@ testRule({
 		},
 		{
 			code: "@document url-prefix('http://www.w3.org/Style');",
+			fixed: '@document url-prefix(http://www.w3.org/Style);',
 			message: messages.rejected('url-prefix'),
 			line: 1,
 			column: 22,
@@ -469,6 +493,7 @@ testRule({
 		},
 		{
 			code: "@document url-prefix( 'http://www.w3.org/Style' );",
+			fixed: '@document url-prefix( http://www.w3.org/Style );',
 			message: messages.rejected('url-prefix'),
 			line: 1,
 			column: 23,
@@ -477,6 +502,7 @@ testRule({
 		},
 		{
 			code: '@document domain("mozilla.org");',
+			fixed: '@document domain(mozilla.org);',
 			message: messages.rejected('domain'),
 			line: 1,
 			column: 18,
@@ -485,6 +511,7 @@ testRule({
 		},
 		{
 			code: '@document domain( "mozilla.org" );',
+			fixed: '@document domain( mozilla.org );',
 			message: messages.rejected('domain'),
 			line: 1,
 			column: 19,
@@ -493,6 +520,7 @@ testRule({
 		},
 		{
 			code: "@font-face { font-family: foo; src: url('foo.ttf'); }",
+			fixed: '@font-face { font-family: foo; src: url(foo.ttf); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 41,
@@ -501,6 +529,7 @@ testRule({
 		},
 		{
 			code: "@font-face { font-family: foo; src: url( 'foo.ttf' ); }",
+			fixed: '@font-face { font-family: foo; src: url( foo.ttf ); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 42,
@@ -509,6 +538,7 @@ testRule({
 		},
 		{
 			code: 'a { background: url("foo.css"); }',
+			fixed: 'a { background: url(foo.css); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 21,
@@ -517,6 +547,7 @@ testRule({
 		},
 		{
 			code: 'a { background: uRl("foo.css"); }',
+			fixed: 'a { background: uRl(foo.css); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 21,
@@ -525,6 +556,7 @@ testRule({
 		},
 		{
 			code: 'a { background: URL("foo.css"); }',
+			fixed: 'a { background: URL(foo.css); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 21,
@@ -533,6 +565,7 @@ testRule({
 		},
 		{
 			code: 'a { background: url( "foo.css" ); }',
+			fixed: 'a { background: url( foo.css ); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 22,
@@ -541,6 +574,7 @@ testRule({
 		},
 		{
 			code: 'a { background: url(  "foo.css"  ); }',
+			fixed: 'a { background: url(  foo.css  ); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 23,
@@ -549,6 +583,7 @@ testRule({
 		},
 		{
 			code: 'a { cursor: url("foo.png"); }',
+			fixed: 'a { cursor: url(foo.png); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 17,
@@ -557,6 +592,7 @@ testRule({
 		},
 		{
 			code: "a { background-image: url('foo.css'), url(bar.css), url(baz.css); }",
+			fixed: 'a { background-image: url(foo.css), url(bar.css), url(baz.css); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 27,
@@ -565,6 +601,7 @@ testRule({
 		},
 		{
 			code: "a { background-image: url( 'foo.css' ), url(bar.css), url(baz.css); }",
+			fixed: 'a { background-image: url( foo.css ), url(bar.css), url(baz.css); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 28,
@@ -573,6 +610,7 @@ testRule({
 		},
 		{
 			code: "a { background-image: url(foo.css), url('bar.css'), url(baz.css); }",
+			fixed: 'a { background-image: url(foo.css), url(bar.css), url(baz.css); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 41,
@@ -581,6 +619,7 @@ testRule({
 		},
 		{
 			code: "a { background-image: url(foo.css), url( 'bar.css' ), url(baz.css); }",
+			fixed: 'a { background-image: url(foo.css), url( bar.css ), url(baz.css); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 42,
@@ -589,6 +628,7 @@ testRule({
 		},
 		{
 			code: "a { background-image: url(foo.css), url(bar.css), url('baz.css'); }",
+			fixed: 'a { background-image: url(foo.css), url(bar.css), url(baz.css); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 55,
@@ -597,6 +637,7 @@ testRule({
 		},
 		{
 			code: "a { background-image: url(foo.css), url(bar.css), url( 'baz.css' ); }",
+			fixed: 'a { background-image: url(foo.css), url(bar.css), url( baz.css ); }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 56,
@@ -605,6 +646,7 @@ testRule({
 		},
 		{
 			code: 'a { background: url("/images/my_image@2x.png") }',
+			fixed: 'a { background: url(/images/my_image@2x.png) }',
 			message: messages.rejected('url'),
 			line: 1,
 			column: 21,