Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed: value-keyword-case is now ignore counter-reset property and counter, counters and attr functions. #2407

Merged
merged 2 commits into from
Mar 11, 2017
Merged

Fixed: value-keyword-case is now ignore counter-reset property and counter, counters and attr functions. #2407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b26cff7
Fixed: `value-keyword-case` is now ignore counter-reset property.
alexander-akait Mar 2, 2017
c2080f5
Fixed: `value-keyword-case` is now ignore counter, counters and attr …
alexander-akait Mar 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/reference/keywordSets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ keywordSets.camelCaseKeywords = new Set([
// https://developer.mozilla.org/docs/Web/CSS/counter-increment
keywordSets.counterIncrementKeywords = uniteSets(keywordSets.basicKeywords, ["none"])

keywordSets.counterResetKeywords = uniteSets(keywordSets.basicKeywords, ["none"])

keywordSets.gridRowKeywords = uniteSets(keywordSets.basicKeywords, [
"auto",
"span",
Expand Down
74 changes: 74 additions & 0 deletions lib/rules/value-keyword-case/__tests__/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,32 @@ testRule(rule, {
}, {
code: "a::before { content: \"TeSt tEsT\"}",
description: "ignore value within quotes",
}, {
code: "a { content: url(http://www.example.com/test.png) ; }",
}, {
code: "a { content: attr(value-string); }",
}, {
code: "a { content: attr(vAlUe-StRiNg); }",
}, {
code: "a { content: attr(VALUE-STRING); }",
}, {
code: "a { content: counter(counter-name); }",
}, {
code: "a { content: counter(cOuNtEr-NaMe); }",
Copy link
Contributor

Choose a reason for hiding this comment

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

If you've already tested the line about toLowerCase (with the value-string tests above), I don't think we need to multiply every test with alternate cases.

Copy link
Member Author

@alexander-akait alexander-akait Mar 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@davidtheclark difference functions to ignore attr and counter

}, {
code: "a { content: counter(COUNTER-NAME); }",
}, {
code: "a { content: counters(counter-name); }",
}, {
code: "a { content: counters(cOuNtEr-NaMe); }",
}, {
code: "a { content: counters(COUNTER-NAME); }",
}, {
code: "a { content: '[' counter(counter-name) ']' ; }",
}, {
code: "a { content: '[' counter(cOuNtEr-NaMe) ']'; }",
}, {
code: "a { content: '[' counter(COUNTER-NAME) ']'; }",
}, {
code: "a { font-size: $fsBLOCK; }",
description: "ignore preprocessor variable includes value keyword",
Expand Down Expand Up @@ -149,6 +175,12 @@ testRule(rule, {
code: "a { counter-increment: cOuNtEr-NaMe; }",
}, {
code: "a { counter-increment: COUNTER-NAME; }",
}, {
code: "a { counter-reset: counter-name; }",
}, {
code: "a { counter-reset: cOuNtEr-NaMe; }",
}, {
code: "a { counter-reset: COUNTER-NAME; }",
}, {
code: "a { grid-row: auto / auto; }",
}, {
Expand Down Expand Up @@ -355,6 +387,11 @@ testRule(rule, {
message: messages.expected("INHERIT", "inherit"),
line: 1,
column: 24,
}, {
code: "a { counter-reset: INHERIT; }",
message: messages.expected("INHERIT", "inherit"),
line: 1,
column: 20,
}, {
code: "a { grid-row: AUTO / auto; }",
message: messages.expected("AUTO", "auto"),
Expand Down Expand Up @@ -662,6 +699,32 @@ testRule(rule, {
}, {
code: "a::before { content: \"TeSt tEsT\"}",
description: "ignore value within quotes",
}, {
code: "a { content: url(http://www.example.com/test.png) ; }",
}, {
code: "a { content: attr(value-string); }",
}, {
code: "a { content: attr(vAlUe-StRiNg); }",
}, {
code: "a { content: attr(VALUE-STRING); }",
}, {
code: "a { content: counter(counter-name); }",
}, {
code: "a { content: counter(cOuNtEr-NaMe); }",
}, {
code: "a { content: counter(COUNTER-NAME); }",
}, {
code: "a { content: counters(counter-name); }",
}, {
code: "a { content: counters(cOuNtEr-NaMe); }",
}, {
code: "a { content: counters(COUNTER-NAME); }",
}, {
code: "a { content: '[' counter(counter-name) ']' ; }",
}, {
code: "a { content: '[' counter(cOuNtEr-NaMe) ']'; }",
}, {
code: "a { content: '[' counter(COUNTER-NAME) ']'; }",
}, {
code: "a { font-size: $fsblock; }",
description: "ignore preprocessor variable includes value keyword",
Expand Down Expand Up @@ -752,6 +815,12 @@ testRule(rule, {
code: "a { counter-increment: cOuNtEr-NaMe; }",
}, {
code: "a { counter-increment: COUNTER-NAME; }",
}, {
code: "a { counter-reset: counter-name; }",
}, {
code: "a { counter-reset: cOuNtEr-NaMe; }",
}, {
code: "a { counter-reset: COUNTER-NAME; }",
}, {
code: "a { grid-row: AUTO / AUTO; }",
}, {
Expand Down Expand Up @@ -953,6 +1022,11 @@ testRule(rule, {
message: messages.expected("inherit", "INHERIT"),
line: 1,
column: 24,
}, {
code: "a { counter-reset: inherit; }",
message: messages.expected("inherit", "INHERIT"),
line: 1,
column: 20,
}, {
code: "a { grid-row: auto / AUTO; }",
message: messages.expected("auto", "AUTO"),
Expand Down
6 changes: 5 additions & 1 deletion lib/rules/value-keyword-case/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const keywordSets = require("../../reference/keywordSets")
const declarationValueIndex = require("../../utils/declarationValueIndex")
const getUnitFromValueNode = require("../../utils/getUnitFromValueNode")
const isCounterIncrementCustomIdentValue = require("../../utils/isCounterIncrementCustomIdentValue")
const isCounterResetCustomIdentValue = require("../../utils/isCounterResetCustomIdentValue")
const isStandardSyntaxValue = require("../../utils/isStandardSyntaxValue")
const matchesStringOrRegExp = require("../../utils/matchesStringOrRegExp")
const report = require("../../utils/report")
Expand Down Expand Up @@ -64,7 +65,7 @@ const rule = function (expectation, options) {
}

// Ignore keywords within `url` and `var` function
if (node.type === "function" && (valueLowerCase === "url" || valueLowerCase === "var")) {
if (node.type === "function" && (valueLowerCase === "url" || valueLowerCase === "var" || valueLowerCase === "counter" || valueLowerCase === "counters" || valueLowerCase === "attr")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How does attr relate to the issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

@davidtheclark I thought that I would correct this behavior in one commit

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine. In the future please make sure to point this out in the comment about your PR, so we remember to place it in the Changelog. Is it tested?

Copy link
Member Author

Choose a reason for hiding this comment

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

@davidtheclark yep

stylelint/lib/rules/value-keyword-case/__tests__/index.js

Line 63 in a726845

code: "a { content: attr(value-string); }",

return false
}

Expand All @@ -90,6 +91,9 @@ const rule = function (expectation, options) {
if (prop === "counter-increment" && isCounterIncrementCustomIdentValue(valueLowerCase)) {
return
}
if (prop === "counter-reset" && isCounterResetCustomIdentValue(valueLowerCase)) {
return
}
if (prop === "grid-row" && !keywordSets.gridRowKeywords.has(valueLowerCase)) {
return
}
Expand Down
18 changes: 18 additions & 0 deletions lib/utils/__tests__/isCounterResetCustomIdentValue.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
"use strict"

const isCounterResetCustomIdentValue = require("../isCounterResetCustomIdentValue")

it("isCustomIdents", () => {
expect(isCounterResetCustomIdentValue("counter")).toBeTruthy()
expect(isCounterResetCustomIdentValue("cOuNtEr")).toBeTruthy()
expect(isCounterResetCustomIdentValue("COUNTER")).toBeTruthy()
expect(isCounterResetCustomIdentValue("counter-name")).toBeTruthy()
expect(isCounterResetCustomIdentValue("counter1")).toBeTruthy()
expect(isCounterResetCustomIdentValue("counter2")).toBeTruthy()
expect(isCounterResetCustomIdentValue("none")).toBeFalsy()
expect(isCounterResetCustomIdentValue("inherit")).toBeFalsy()
expect(isCounterResetCustomIdentValue("initial")).toBeFalsy()
expect(isCounterResetCustomIdentValue("unset")).toBeFalsy()
expect(isCounterResetCustomIdentValue("-1")).toBeFalsy()
expect(isCounterResetCustomIdentValue("1")).toBeFalsy()
})
19 changes: 19 additions & 0 deletions lib/utils/isCounterResetCustomIdentValue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/* @flow */
"use strict"

const keywordSets = require("../reference/keywordSets")
const _ = require("lodash")

/**
* Check value is a custom ident
*/

module.exports = function (value/*: string*/)/*: boolean*/ {
const valueLowerCase = value.toLowerCase()

if (keywordSets.counterResetKeywords.has(valueLowerCase) || _.isFinite(parseInt(valueLowerCase))) {
return false
}

return true
}