From b6723679606bb8d39e75025ae17ace9f1c3e2631 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Thu, 30 Sep 2021 22:07:06 -0700 Subject: [PATCH] Updating no-deprecated-colors plugin for edge cases (#132) * Checking sass variables * Make sure we replace multiple in same line * Create famous-ladybugs-report.md * Accounting for mixins also * 5.0 * don't replace all --- .changeset/famous-ladybugs-report.md | 5 ++ __tests__/__fixtures__/deprecations.json | 4 +- __tests__/no-deprecated-colors.js | 82 ++++++++++++++++++++++++ package.json | 4 +- plugins/no-deprecated-colors.js | 62 ++++++++++-------- yarn.lock | 8 +-- 6 files changed, 132 insertions(+), 33 deletions(-) create mode 100644 .changeset/famous-ladybugs-report.md diff --git a/.changeset/famous-ladybugs-report.md b/.changeset/famous-ladybugs-report.md new file mode 100644 index 00000000..7233c081 --- /dev/null +++ b/.changeset/famous-ladybugs-report.md @@ -0,0 +1,5 @@ +--- +"@primer/stylelint-config": patch +--- + +Updating no-deprecated-colors plugin for edge cases diff --git a/__tests__/__fixtures__/deprecations.json b/__tests__/__fixtures__/deprecations.json index ce401870..d35a20f9 100644 --- a/__tests__/__fixtures__/deprecations.json +++ b/__tests__/__fixtures__/deprecations.json @@ -1,5 +1,7 @@ { "text.primary": "fg.default", "text.secondary": ["fg.one", "fg.two"], - "text.white": null + "text.white": null, + "border.primary": "border.default", + "border.secondary": "border.subtle" } diff --git a/__tests__/no-deprecated-colors.js b/__tests__/no-deprecated-colors.js index 85d72ae3..23351f16 100644 --- a/__tests__/no-deprecated-colors.js +++ b/__tests__/no-deprecated-colors.js @@ -22,6 +22,88 @@ testRule({ line: 1, column: 6 }, + { + code: '@mixin double-caret($border: var(--color-border-primary)) { }', + fixed: '@mixin double-caret($border: var(--color-border-default)) { }', + message: `--color-border-primary is a deprecated color variable. Please use the replacement --color-border-default. (primer/no-deprecated-colors)`, + line: 1, + column: 1 + }, + { + code: '@mixin double-caret() { border-color: var(--color-border-primary); }', + fixed: '@mixin double-caret() { border-color: var(--color-border-default); }', + message: `--color-border-primary is a deprecated color variable. Please use the replacement --color-border-default. (primer/no-deprecated-colors)`, + line: 1, + column: 25 + }, + { + code: '.x { border: 1px solid var(--color-text-primary); .foo { color: var(--color-text-primary); } }', + fixed: '.x { border: 1px solid var(--color-fg-default); .foo { color: var(--color-fg-default); } }', + warnings: [ + { + message: + '--color-text-primary is a deprecated color variable. Please use the replacement --color-fg-default. (primer/no-deprecated-colors)', + line: 1, + column: 6 + }, + { + message: + '--color-text-primary is a deprecated color variable. Please use the replacement --color-fg-default. (primer/no-deprecated-colors)', + line: 1, + column: 58 + } + ] + }, + { + code: '.x { border-color: var(--color-border-primary) var(--color-border-primary); }', + fixed: '.x { border-color: var(--color-border-default) var(--color-border-default); }', + warnings: [ + { + message: + '--color-border-primary is a deprecated color variable. Please use the replacement --color-border-default. (primer/no-deprecated-colors)', + line: 1, + column: 6 + }, + { + message: + '--color-border-primary is a deprecated color variable. Please use the replacement --color-border-default. (primer/no-deprecated-colors)', + line: 1, + column: 6 + } + ] + }, + { + code: '.x { border-color: var(--color-border-primary) var(--color-border-secondary); }', + fixed: '.x { border-color: var(--color-border-default) var(--color-border-subtle); }', + warnings: [ + { + message: + '--color-border-primary is a deprecated color variable. Please use the replacement --color-border-default. (primer/no-deprecated-colors)', + line: 1, + column: 6 + }, + { + message: + '--color-border-secondary is a deprecated color variable. Please use the replacement --color-border-subtle. (primer/no-deprecated-colors)', + line: 1, + column: 6 + } + ] + }, + { + code: '$border: $border-width $border-style var(--color-border-primary) !default;', + fixed: '$border: $border-width $border-style var(--color-border-default) !default;', + message: `--color-border-primary is a deprecated color variable. Please use the replacement --color-border-default. (primer/no-deprecated-colors)`, + line: 1, + column: 1 + }, + { + code: '.x { border: $border-width $border-style var(--color-border-primary); }', + fixed: '.x { border: $border-width $border-style var(--color-border-default); }', + message: `--color-border-primary is a deprecated color variable. Please use the replacement --color-border-default. (primer/no-deprecated-colors)`, + line: 1, + column: 6 + }, { code: '.x { background-color: var(--color-bg-canvas); }', fixed: '.x { background-color: var(--color-canvas-default); }', diff --git a/package.json b/package.json index 18ecec98..2be30bf2 100644 --- a/package.json +++ b/package.json @@ -37,14 +37,14 @@ }, "peerDependencies": { "@primer/css": "*", - "@primer/primitives": "^5.0.0-rc.8de08c0" + "@primer/primitives": "^5.0.0" }, "devDependencies": { "@changesets/changelog-github": "0.4.1", "@changesets/cli": "2.17.0", "@github/prettier-config": "0.0.4", "@primer/css": "^13.2.0", - "@primer/primitives": "^5.0.0-rc.8de08c0", + "@primer/primitives": "^5.0.0", "dedent": "0.7.0", "eslint": "7.32.0", "eslint-plugin-github": "4.3.0", diff --git a/plugins/no-deprecated-colors.js b/plugins/no-deprecated-colors.js index 7c8b7416..af69067b 100644 --- a/plugins/no-deprecated-colors.js +++ b/plugins/no-deprecated-colors.js @@ -55,35 +55,45 @@ module.exports = stylelint.createPlugin(ruleName, (enabled, options = {}, contex }, {}) const lintResult = (root, result) => { - root.walkRules(rule => { - rule.walkDecls(decl => { - if (seen.has(decl)) { - return - } else { - seen.set(decl, true) - } - - for (const [, variableName] of matchAll(decl.value, variableReferenceRegex)) { - if (variableName in convertedCSSVars) { - let replacement = convertedCSSVars[variableName] - - if (context.fix && replacement !== null && !Array.isArray(replacement)) { - replacement = `--color-${kebabCase(replacement)}` - replacedVars[variableName] = true - newVars[replacement] = true - decl.value = decl.value.replace(variableName, replacement) - return + // Walk all declarartions + root.walk(node => { + if (seen.has(node)) { + return + } else { + seen.set(node, true) + } + // walk these nodes + if (!(node.type === 'decl' || node.type === 'atrule')) { + return + } + + for (const [, variableName] of matchAll( + node.type === 'atrule' ? node.params : node.value, + variableReferenceRegex + )) { + if (variableName in convertedCSSVars) { + let replacement = convertedCSSVars[variableName] + + if (context.fix && replacement !== null && !Array.isArray(replacement)) { + replacement = `--color-${kebabCase(replacement)}` + replacedVars[variableName] = true + newVars[replacement] = true + if (node.type === 'atrule') { + node.params = node.params.replace(variableName, replacement) + } else { + node.value = node.value.replace(variableName, replacement) } - - stylelint.utils.report({ - message: messages.rejected(variableName, replacement), - node: decl, - ruleName, - result - }) + continue } + + stylelint.utils.report({ + message: messages.rejected(variableName, replacement), + node, + ruleName, + result + }) } - }) + } }) } diff --git a/yarn.lock b/yarn.lock index 1f2da78b..22ac088d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -792,10 +792,10 @@ dependencies: object-assign "^4.1.1" -"@primer/primitives@^5.0.0-rc.8de08c0": - version "5.0.0-rc.8de08c0" - resolved "https://registry.yarnpkg.com/@primer/primitives/-/primitives-5.0.0-rc.8de08c0.tgz#b85825eb55d8b364ca31cb573e66e2caae63137a" - integrity sha512-4qjGx4ec3FuAU/7sKjsVzDXMM8ZbFW/VstxmCwaraN6hXfgruZrGY0HlEuhzor0xenhdbxMZllsPoz6FxFIxNQ== +"@primer/primitives@^5.0.0": + version "5.0.0" + resolved "https://registry.yarnpkg.com/@primer/primitives/-/primitives-5.0.0.tgz#20e5e7aae0464093f4742f5947c332cbcbc68edd" + integrity sha512-g2omeDBgfE5Q6+BQxPaflz5/shCFNjPvfpzphQMpeqIeSrV9boFyicLq7/Rd3WdsDvIMIsMCC1lWZE9JyEhR3w== "@sinonjs/commons@^1.7.0": version "1.8.3"