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.

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

Fix at-rule-no-unknown end positions #655

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions jest-setup.js
@@ -1,3 +1,10 @@
const getTestRule = require("jest-preset-stylelint/getTestRule");

global.testRule = getTestRule({ plugins: ["./src"] });

// Inspired by https://www.npmjs.com/package/dedent-tag
global.dedent = function dedent(strings) {
return strings[0]
.replace(/^[ \t]+/gm, "") // remove indentation
.replace(/^\s*\n/gm, ""); // remove empty lines
};
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -57,6 +57,7 @@
"lodash"
],
"globals": {
"dedent": true,
"testRule": true
},
"rules": {
Expand Down
132 changes: 64 additions & 68 deletions src/rules/at-rule-no-unknown/__tests__/index.js
Expand Up @@ -7,195 +7,191 @@ testRule({

accept: [
{
code: "@charset 'UTF-8';"
code: "@charset 'UTF-8';",
},
{
code: "@CHARSET 'UTF-8';"
code: "@CHARSET 'UTF-8';",
},
{
code: "@charset 'iso-8859-15'"
code: "@charset 'iso-8859-15'",
},
{
code: '@import url("fineprint.css") print;'
code: '@import url("fineprint.css") print;',
},
{
code: "@import 'custom.css'"
code: "@import 'custom.css'",
},
{
code: "@import url('landscape.css') screen and (orientation:landscape);"
code: "@import url('landscape.css') screen and (orientation:landscape);",
},
{
code: "@namespace url(http://www.w3.org/1999/xhtml);"
code: "@namespace url(http://www.w3.org/1999/xhtml);",
},
{
code: "@namespace prefix url(XML-namespace-URL);"
code: "@namespace prefix url(XML-namespace-URL);",
},
{
code: "@media print { body { font-size: 10pt } }"
code: "@media print { body { font-size: 10pt } }",
},
{
code: "@media (max-width: 960px) { body { font-size: 13px } }"
code: "@media (max-width: 960px) { body { font-size: 13px } }",
},
{
code: "@media screen, print { body { line-height: 1.2 } }"
code: "@media screen, print { body { line-height: 1.2 } }",
},
{
code: "@supports (--foo: green) { body { color: green; } }"
code: "@supports (--foo: green) { body { color: green; } }",
},
{
code:
"@supports ( (perspective: 10px) or (-webkit-perspective: 10px) ) { font-size: 10pt }"
code: "@supports ( (perspective: 10px) or (-webkit-perspective: 10px) ) { font-size: 10pt }",
},
{
code:
"@counter-style win-list { system: fixed; symbols: url(gold-medal.svg); suffix: ' ';}"
code: "@counter-style win-list { system: fixed; symbols: url(gold-medal.svg); suffix: ' ';}",
},
{
code:
"@document url(http://www.w3.org/), url-prefix(http://www.w3.org/Style/), domain(mozilla.org), regexp('https:.*')"
code: "@document url(http://www.w3.org/), url-prefix(http://www.w3.org/Style/), domain(mozilla.org), regexp('https:.*')",
},
{
code: "@page :left { margin-left: 4cm; }"
code: "@page :left { margin-left: 4cm; }",
},
{
code:
'@font-face { font-family: MyHelvetica; src: local("Helvetica"), url(MgOpenModern.ttf); }'
code: '@font-face { font-family: MyHelvetica; src: local("Helvetica"), url(MgOpenModern.ttf); }',
},
{
code:
"@keyframes identifier { 0% { top: 0; left: 0; } 30% { top: 50px; } 68%, 100% { top: 100px; left: 100%; } }"
code: "@keyframes identifier { 0% { top: 0; left: 0; } 30% { top: 50px; } 68%, 100% { top: 100px; left: 100%; } }",
},
{
code:
"@-webkit-keyframes identifier { 0% { top: 0; left: 0; } 30% { top: 50px; } 68%, 100% { top: 100px; left: 100%; } }"
code: "@-webkit-keyframes identifier { 0% { top: 0; left: 0; } 30% { top: 50px; } 68%, 100% { top: 100px; left: 100%; } }",
},
{
code: "@viewport { min-width: 640px; max-width: 800px; }"
code: "@viewport { min-width: 640px; max-width: 800px; }",
},
{
code: "@viewport { orientation: landscape; }"
code: "@viewport { orientation: landscape; }",
},
{
code:
'@counter-style winners-list { system: fixed; symbols: url(gold-medal.svg); suffix: " "; }'
code: '@counter-style winners-list { system: fixed; symbols: url(gold-medal.svg); suffix: " "; }',
},
{
code: "@font-feature-values Font One { @styleset { nice-style: 12; } }"
code: "@font-feature-values Font One { @styleset { nice-style: 12; } }",
},
{
code: ".foo { color: red; @nest .parent & { color: blue; } }"
code: ".foo { color: red; @nest .parent & { color: blue; } }",
},
{
code: '@forward "functions";'
code: '@forward "functions";',
},
{
code: "@function foo () {}"
code: "@function foo () {}",
},
{
code: "@extend %p"
code: "@extend %p",
},
{
code: "@if ($i) {}"
code: "@if ($i) {}",
},
{
code: "@if ($i) {} @else {}"
code: "@if ($i) {} @else {}",
},
{
code: "@if ($i) {} @else if {} @else {}"
code: "@if ($i) {} @else if {} @else {}",
},
{
code: '@use "bootstrap";'
}
code: '@use "bootstrap";',
},
],

reject: [
{
code: `
@funciton floo ($n) {}
`,
line: 2,
code: dedent`
@funciton floo ($n) {}
`,
line: 1,
column: 1,
endLine: 1,
endColumn: 10,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[note] It seems to me that one case change is enough to test end positions.

description: "",
message: messages.rejected("@funciton")
message: messages.rejected("@funciton"),
},
{
code: `
@Function foo () { @return 1; }
`,
line: 2,
description: "",
message: messages.rejected("@Function")
message: messages.rejected("@Function"),
},
{
code: `
@While ($i == 1) {}
`,
line: 2,
description: "",
message: messages.rejected("@While")
}
]
message: messages.rejected("@While"),
},
],
});

testRule({
ruleName,
config: [
true,
{
ignoreAtRules: ["unknown", "/^my-/i"]
}
ignoreAtRules: ["unknown", "/^my-/i"],
},
],
skipBasicChecks: true,

accept: [
{
code: "@unknown { }"
code: "@unknown { }",
},
{
code: "@my-at-rule { }"
code: "@my-at-rule { }",
},
{
code: "@MY-other-at-rule { }"
}
code: "@MY-other-at-rule { }",
},
],

reject: [
{
code: "@unknown-at-rule { }",
line: 1,
column: 1,
message: messages.rejected("@unknown-at-rule")
message: messages.rejected("@unknown-at-rule"),
},
{
code: "@unknown { @unknown-at-rule { font-size: 14px; } }",
line: 1,
column: 12,
message: messages.rejected("@unknown-at-rule")
message: messages.rejected("@unknown-at-rule"),
},
{
code: "@not-my-at-rule {}",
line: 1,
column: 1,
message: messages.rejected("@not-my-at-rule")
message: messages.rejected("@not-my-at-rule"),
},
{
code: "@Unknown { }",
line: 1,
column: 1,
message: messages.rejected("@Unknown")
message: messages.rejected("@Unknown"),
},
{
code: "@uNkNoWn { }",
line: 1,
column: 1,
message: messages.rejected("@uNkNoWn")
message: messages.rejected("@uNkNoWn"),
},
{
code: "@UNKNOWN { }",
line: 1,
column: 1,
message: messages.rejected("@UNKNOWN")
}
]
message: messages.rejected("@UNKNOWN"),
},
],
});

testRule({
Expand All @@ -211,14 +207,14 @@ testRule({
{
line: 1,
column: 1,
message: messages.rejected("@foo")
message: messages.rejected("@foo"),
},
{
line: 1,
column: 10,
message: messages.rejected("@bar")
}
]
}
]
message: messages.rejected("@bar"),
},
],
},
],
});
22 changes: 11 additions & 11 deletions src/rules/at-rule-no-unknown/index.js
Expand Up @@ -22,7 +22,7 @@ const sassAtRules = [
"return",
"use",
"warn",
"while"
"while",
];

const ruleToCheckAgainst = "at-rule-no-unknown";
Expand All @@ -34,11 +34,11 @@ export const messages = utils.ruleMessages(ruleName, {
return rules[ruleToCheckAgainst].messages
.rejected(...args)
.replace(` (${ruleToCheckAgainst})`, "");
}
},
});

export const meta = {
url: ruleUrl(ruleName)
url: ruleUrl(ruleName),
};

export default function rule(primaryOption, secondaryOptions) {
Expand All @@ -47,14 +47,14 @@ export default function rule(primaryOption, secondaryOptions) {
result,
ruleName,
{
actual: primaryOption
actual: primaryOption,
},
{
actual: secondaryOptions,
possible: {
ignoreAtRules: [isRegExp, isString]
ignoreAtRules: [isRegExp, isString],
},
optional: true
optional: true,
}
);

Expand All @@ -65,16 +65,16 @@ export default function rule(primaryOption, secondaryOptions) {
const optionsAtRules = secondaryOptions && secondaryOptions.ignoreAtRules;
const ignoreAtRules = sassAtRules.concat(optionsAtRules || []);
const defaultedOptions = Object.assign({}, secondaryOptions, {
ignoreAtRules
ignoreAtRules,
});

utils.checkAgainstRule(
{
ruleName: ruleToCheckAgainst,
ruleSettings: [primaryOption, defaultedOptions],
root
root,
},
warning => {
(warning) => {
const name = warning.node.name;

if (!ignoreAtRules.includes(name)) {
Expand All @@ -83,8 +83,8 @@ export default function rule(primaryOption, secondaryOptions) {
ruleName,
result,
node: warning.node,
line: warning.line,
column: warning.column
start: { line: warning.line, column: warning.column },
end: { line: warning.endLine, column: warning.endColumn },
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ybiquitous is this a backwards compatible change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I tried testing the minimum version of peerDependencies.stylelint locally:

"peerDependencies": {
"stylelint": "^14.5.1"
},

$ npm i stylelint@14.5.1
...

$ npx jest at-rule-no-unknown --no-verbose
 FAIL  src/rules/at-rule-no-unknown/__tests__/index.js
  ● scss/at-rule-no-unknown › reject › [ true ] › '@funciton floo ($n) {}\n' › no description

    expect(received).toMatchObject(expected)

    - Expected  - 2
    + Received  + 0

      Object {
        "column": 1,
    -   "endColumn": 10,
    -   "endLine": 1,
        "line": 1,
        "text": "Unexpected unknown at-rule \"@funciton\" (scss/at-rule-no-unknown)",
      }

      at Object.<anonymous> (node_modules/jest-preset-stylelint/getTestRule.js:88:33)

It seems that the backward compatibility keeps. 👍🏼

If possible, I think it's better to update peerDependencies.stylelint to ^14.7.0, but not required.

});
}
}
Expand Down