Added: `function-url-scheme-blacklist` rule. #2626

alexander-akait · 2017-06-12T16:48:41Z

Which issue, if any, is this issue related to?

#2513

Is there anything in the PR that needs further explanation?

No, it's self explanatory.

alexander-akait · 2017-06-12T16:48:49Z

/cc @stylelint/core

jeddy3

Looking good.

Minor doc request. Additionally, can you update:

docs/user-guide/example-config.md.
docs/user-guide/rules.md.

If we ever add any more rules which operate on the scheme of a URL I suggest we abstract some of the code this and the whitelist rule, e.g. is schemeless url, into utilities. I think it's fine at the moment, though.

jeddy3 · 2017-06-12T17:40:51Z

lib/rules/function-url-scheme-blacklist/README.md

@@ -0,0 +1,67 @@
+# function-url-scheme-blacklist
+
+Specify a blacklist of allowed url schemes.


Specify a blacklist of disallowed url schemes.

alexander-akait · 2017-06-12T17:52:39Z

@jeddy3 stupid error, sorry

alexander-akait · 2017-06-12T17:57:18Z

/cc @jeddy3

jeddy3

LGTM thanks

hudochenkov

Great work!

I have few questions, though, in comments.

Also, please, make all “url” → “URL” in readme, because “URL” is an acronym.

hudochenkov · 2017-06-15T20:20:43Z

lib/rules/function-url-scheme-blacklist/index.js

+const ruleName = "function-url-scheme-blacklist"
+
+const messages = ruleMessages(ruleName, {
+  rejected: scheme => `Unexpected url scheme "${scheme}:"`,


Upper case “URL”.

done, also in whitelist rule.

hudochenkov · 2017-06-15T20:25:31Z

lib/rules/function-url-scheme-blacklist/index.js

+  return (root, result) => {
+    const validOptions = validateOptions(result, ruleName, {
+      actual: blacklist,
+      possible: [_.isString],


Readme says it accepts array|string. I'm not sure if it checks that. There're no tests if only one string was in config (not array with one string).

@hudochenkov we use this in all *-blacklist/*-whitelist rules.

hudochenkov · 2017-06-15T20:31:14Z

lib/rules/function-url-scheme-blacklist/__tests__/index.js

+
+testRule(rule, {
+  ruleName,
+  config: [[]],


Why this test group rejects https and data? Is it an undocumented behaviour?

@hudochenkov this used if your want add all schemes to blacklist, analogy in whitelist

But still behaviour isn't documented anywhere. We should document this.

hudochenkov · 2017-06-15T20:31:37Z

lib/rules/function-url-scheme-blacklist/__tests__/index.js

+
+testRule(rule, {
+  ruleName,
+  config: [""],


Why this test group rejects https and data? Is it an undocumented behaviour?

@hudochenkov we already have same tests in function-url-scheme-whitelist

But still behaviour isn't documented anywhere. We should document this.

@hudochenkov can your help me with this, I would like the current time to concentrate on the new rules (#2528) and I'm bad at documentation 😞

@hudochenkov Good catch! Sorry I missed this. This isn't the correct behaviour. Both [""] and [[]] should allow all schemes.

This is how the other -blacklist rules behave. The -whitelist rules behave differently because whitelists and blacklists are different.

It's useful to think as:

blacklist: [] = "blacklist nothing" i.e. "everything is ok"
whitelist: [] = "whitelist nothing" i.e. "nothing is ok".

alexander-akait · 2017-06-20T14:08:32Z

/cc @hudochenkov

jeddy3 · 2017-06-20T16:39:21Z

lib/rules/function-url-scheme-blacklist/__tests__/index.js

+
+testRule(rule, {
+  ruleName,
+  config: [""],


@hudochenkov Good catch! Sorry I missed this. This isn't the correct behaviour. Both [""] and [[]] should allow all schemes.

This is how the other -blacklist rules behave. The -whitelist rules behave differently because whitelists and blacklists are different.

It's useful to think as:

blacklist: [] = "blacklist nothing" i.e. "everything is ok"
whitelist: [] = "whitelist nothing" i.e. "nothing is ok".

alexander-akait · 2017-06-20T16:44:50Z

@jeddy3 can your help with documentation this?

jeddy3 · 2017-06-20T18:32:35Z

@jeddy3 can your help with documentation this?

It's a behaviour issue, not a documentation one. I'm pushing up a commit to fix though.

jeddy3 · 2017-06-20T19:21:55Z

I've added to tests to the property-*list rules to show how the whitelist and blacklist rule behaviours are correctly different when given empty strings or arrays as primary options.
This behaviour comes for free when using the matchesStringOrRegExp util. I've updated function-url-scheme-blacklist to use it.
Likewise, support for regex also comes for free when using matchesStringOrRegExp.
I've removed the tests for case insensitive primary options. None of the other *-*list rules have case-insensitive primary options. If we want to revisit this behaviour, so should do so across the board and in a separate issue.

alexander-akait · 2017-06-20T20:07:50Z

@jeddy3 After commits what are my actions here? Or just wait when @hudochenkov do review?

jeddy3 · 2017-06-20T20:20:53Z

Or just wait when @hudochenkov do review?

Just wait on @hudochenkov as my commits came off the back of his original review.

hudochenkov

RegEx addition is 🔥

And again about "" and [] configurations. We won't document them?

hudochenkov · 2017-06-20T20:48:28Z

lib/rules/function-url-scheme-blacklist/README.md

+
+A [URL scheme](https://url.spec.whatwg.org/#syntax-url-scheme) consists of alphanumeric, `+`, `-`, and `.` characters. It can appear at the start of a URL and is followed by `:`.
+
+This rule treats URL schemes as case insensitive (`https` and `HTTPS` are the same).


I've removed the tests for case insensitive primary options. None of the other *-*list rules have case-insensitive primary options. If we want to revisit this behaviour, so should do so across the board and in a separate issue. — @jeddy3

Is this line in readme still valid?

Yes. I've added tests to show this.

Like all the other *-*list rules, this rule now only accepts lowercase primary options but is case insensitive when checking for matches.

So function-url-scheme-blacklist: "http" will disallow both http://g.co/x.jpg and HTTP://g.co/x.jpg.

#2660 is about allowing the user to specify primary options in any case e.g. function-url-scheme-blacklist: "HTTP", or function-blacklist: "translateX".

jeddy3 · 2017-06-20T21:28:51Z

And again about "" and [] configurations. We won't document them?

I'm not sure there's any need to do that now because the behaviour is intuitive. We don't document this behaviour in any of the other *-blacklist rules for this reason.

scheme-blacklist: [] = "blacklist nothing" i.e. "all schemes are ok" seems pretty clear to me.

hudochenkov

Thank you for your work, @evilebottnawi and @jeddy3!

jeddy3 · 2017-06-20T22:43:03Z

Changelog

Added: function-url-scheme-blacklist rule (#2626).

alexander-akait added the PR: review needed label Jun 12, 2017

jeddy3 requested changes Jun 12, 2017

View reviewed changes

jeddy3 added PR: needs revision and removed PR: review needed labels Jun 12, 2017

alexander-akait force-pushed the function-url-scheme-blacklist branch from 76e07a7 to 9f7c21f Compare June 12, 2017 17:56

alexander-akait added PR: review needed and removed PR: needs revision labels Jun 12, 2017

jeddy3 approved these changes Jun 12, 2017

View reviewed changes

hudochenkov requested changes Jun 15, 2017

View reviewed changes

hudochenkov added PR: needs revision and removed PR: review needed labels Jun 15, 2017

alexander-akait force-pushed the function-url-scheme-blacklist branch from 9f7c21f to 93b5a52 Compare June 20, 2017 14:08

Added: function-url-scheme-blacklist rule.

bae664d

alexander-akait force-pushed the function-url-scheme-blacklist branch from 93b5a52 to bae664d Compare June 20, 2017 14:08

jeddy3 requested changes Jun 20, 2017

View reviewed changes

jeddy3 added 3 commits June 20, 2017 20:12

Add empty config tests to property-*list

33a96eb

Update function-url-scheme-whitelist snapshot

b83d724

Add regex support and correct empty config behaviour

b01bbae

jeddy3 added PR: review needed and removed PR: needs revision labels Jun 20, 2017

jeddy3 mentioned this pull request Jun 20, 2017

Add regex support to function-url-scheme-whitelist #2659

Closed

jeddy3 mentioned this pull request Jun 20, 2017

Ensure consistent case sensitivity across *-list rules and ignore* options #2660

Closed

hudochenkov reviewed Jun 20, 2017

View reviewed changes

Add tests for case insensitivity

0d9def6

hudochenkov approved these changes Jun 20, 2017

View reviewed changes

jeddy3 approved these changes Jun 20, 2017

View reviewed changes

jeddy3 added PR: review accepted and removed PR: review needed labels Jun 20, 2017

jeddy3 merged commit c3b4788 into master Jun 20, 2017

jeddy3 deleted the function-url-scheme-blacklist branch June 20, 2017 22:41

renovate bot mentioned this pull request Jun 26, 2017

Update dependency stylelint to version 7.12.0 singapore/lint-condo#316

Merged

dependabot-preview bot mentioned this pull request Mar 14, 2018

Bump stylelint from 7.10.1 to 9.1.2 nathanmsmith/RadioxDB-flatpage#26

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added: `function-url-scheme-blacklist` rule. #2626

Added: `function-url-scheme-blacklist` rule. #2626

alexander-akait commented Jun 12, 2017

alexander-akait commented Jun 12, 2017

jeddy3 left a comment

jeddy3 Jun 12, 2017

alexander-akait commented Jun 12, 2017

alexander-akait commented Jun 12, 2017

jeddy3 left a comment

hudochenkov left a comment

hudochenkov Jun 15, 2017

alexander-akait Jun 20, 2017

hudochenkov Jun 15, 2017

alexander-akait Jun 20, 2017

hudochenkov Jun 15, 2017

alexander-akait Jun 20, 2017 •

edited

Loading

hudochenkov Jun 20, 2017

hudochenkov Jun 15, 2017

alexander-akait Jun 20, 2017

hudochenkov Jun 20, 2017

alexander-akait Jun 20, 2017

jeddy3 Jun 20, 2017 •

edited

Loading

alexander-akait commented Jun 20, 2017

jeddy3 Jun 20, 2017 •

edited

Loading

alexander-akait commented Jun 20, 2017

jeddy3 commented Jun 20, 2017

jeddy3 commented Jun 20, 2017

alexander-akait commented Jun 20, 2017

jeddy3 commented Jun 20, 2017

hudochenkov left a comment

hudochenkov Jun 20, 2017

jeddy3 Jun 20, 2017

jeddy3 commented Jun 20, 2017

hudochenkov left a comment

jeddy3 commented Jun 20, 2017

		@@ -0,0 +1,67 @@
		# function-url-scheme-blacklist

		Specify a blacklist of allowed url schemes.


		A [URL scheme](https://url.spec.whatwg.org/#syntax-url-scheme) consists of alphanumeric, `+`, `-`, and `.` characters. It can appear at the start of a URL and is followed by `:`.

		This rule treats URL schemes as case insensitive (`https` and `HTTPS` are the same).

Added: function-url-scheme-blacklist rule. #2626

Added: function-url-scheme-blacklist rule. #2626

Conversation

alexander-akait commented Jun 12, 2017

alexander-akait commented Jun 12, 2017

jeddy3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait commented Jun 12, 2017

alexander-akait commented Jun 12, 2017

jeddy3 left a comment

Choose a reason for hiding this comment

hudochenkov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeddy3 Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

alexander-akait commented Jun 20, 2017

jeddy3 Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

alexander-akait commented Jun 20, 2017

jeddy3 commented Jun 20, 2017

jeddy3 commented Jun 20, 2017

alexander-akait commented Jun 20, 2017

jeddy3 commented Jun 20, 2017

hudochenkov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeddy3 commented Jun 20, 2017

hudochenkov left a comment

Choose a reason for hiding this comment

jeddy3 commented Jun 20, 2017

Added: `function-url-scheme-blacklist` rule. #2626

Added: `function-url-scheme-blacklist` rule. #2626

alexander-akait Jun 20, 2017 •

edited

Loading

jeddy3 Jun 20, 2017 •

edited

Loading

jeddy3 Jun 20, 2017 •

edited

Loading