diff --git a/docs/rules/no-internal-style.md b/docs/rules/no-internal-style.md new file mode 100644 index 00000000..13a918b0 --- /dev/null +++ b/docs/rules/no-internal-style.md @@ -0,0 +1,83 @@ +# Forbid passing styles affecting component internals (no-internal-style) + +Tailwind recommends reusing styles by creating components using your favorite front-end framework (e.g. React). Such components almost always need to expose a way to set classes so that users can specify layout styles (e.g. margins, size, positioning). For example, in React, ` +``` + +### `ignoredKeys` (default: `["compoundVariants", "defaultVariants"]`) + +Using libraries like `cva`, some of its object keys are not meant to contain classnames in its value(s). +You can specify which key(s) won't be parsed by the plugin using this setting. +For example, `cva` has `compoundVariants` and `defaultVariants`. +NB: As `compoundVariants` can have classnames inside its `class` property, you can also use a callee to make sure this inner part gets parsed while its parent is ignored. + +### `skipClassAttribute` (default: `false`) + +Set `skipClassAttribute` to `true` if you only want to lint the classnames inside one of the `callees`. +While, this will avoid linting the `class` and `className` attributes, it will still lint matching `callees` inside of these attributes. + +### `tags` (default: `[]`) + +Optional, if you are using tagged templates, you should provide the tags in this array. + +### `externalClassRegex` (default: `"^class(Name)?$"`) + +Optional, can be used to support custom attributes diff --git a/lib/config/groups.js b/lib/config/groups.js index 234c89db..6842ac3d 100644 --- a/lib/config/groups.js +++ b/lib/config/groups.js @@ -46,36 +46,44 @@ module.exports.groups = [ { type: 'Columns', members: 'columns\\-(?${columns})', + internal: true, }, { type: 'Break After', members: 'break\\-after\\-(?auto|avoid|all|avoid\\-page|page|left|right|column)', + internal: true, }, { type: 'Break Before', members: 'break\\-before\\-(?auto|avoid|all|avoid\\-page|page|left|right|column)', + internal: true, }, { type: 'Break Inside', members: 'break\\-inside\\-(?auto|avoid|avoid\\-page|avoid\\-column)', + internal: true, }, { type: 'Box Decoration Break', members: 'box\\-decoration\\-(?clone|slice)', + internal: true, }, { type: 'Deprecated Box Decoration Break', members: 'decoration\\-(?clone|slice)', deprecated: true, + internal: true, }, { type: 'Box Sizing', members: 'box\\-(?border|content)', + internal: true, }, { type: 'Display', members: 'block|flex|grid|flow\\-root|contents|hidden|inline(\\-(block|flex|table|grid))?|table\\-(column|footer|header|row)\\-group|table(\\-(caption|row|cell|column))?|list\\-item', + internal: true, }, { type: 'Floats', @@ -88,17 +96,21 @@ module.exports.groups = [ { type: 'Isolation', members: '(isolate|isolation\\-auto)', + internal: true, }, { type: 'Object Fit', members: 'object\\-(?contain|cover|fill|none|scale\\-down)', + internal: true, }, { type: 'Object Position', members: 'object\\-(?${objectPosition})', + internal: true, }, { type: 'Overflow', + internal: true, members: [ { type: 'overflow', @@ -122,6 +134,7 @@ module.exports.groups = [ }, { type: 'Overscroll Behavior', + internal: true, members: [ { type: 'overscroll', @@ -214,10 +227,12 @@ module.exports.groups = [ { type: 'Flex Direction', members: 'flex\\-(row|col)(\\-reverse)?', + internal: true, }, { type: 'Flex Wrap', members: 'flex\\-(wrap(\\-reverse)?|nowrap)', + internal: true, }, { type: 'Flex', @@ -248,6 +263,7 @@ module.exports.groups = [ { type: 'Grid Template Columns', members: 'grid\\-cols\\-(?${gridTemplateColumns})', + internal: true, }, { type: 'Grid Column Start / End', @@ -269,6 +285,7 @@ module.exports.groups = [ { type: 'Grid Template Rows', members: 'grid\\-rows\\-(?${gridTemplateRows})', + internal: true, }, { type: 'Grid Row Start / End', @@ -290,17 +307,21 @@ module.exports.groups = [ { type: 'Grid Auto Flow', members: 'grid\\-flow\\-(dense|(row|col)(\\-dense)?)', + internal: true, }, { type: 'Grid Auto Columns', members: 'auto\\-cols\\-(?${gridAutoColumns})', + internal: true, }, { type: 'Grid Auto Rows', members: 'auto\\-rows\\-(?${gridAutoRows})', + internal: true, }, { type: 'Gap', + internal: true, members: [ { type: 'gap', @@ -325,10 +346,12 @@ module.exports.groups = [ { type: 'Justify Content', members: 'justify\\-(start|end|center|between|around|evenly)', + internal: true, }, { type: 'Justify Items', members: 'justify\\-items\\-(start|end|center|stretch)', + internal: true, }, { type: 'Justify Self', @@ -337,10 +360,12 @@ module.exports.groups = [ { type: 'Align Content', members: 'content\\-(center|start|end|between|around|evenly|baseline)', + internal: true, }, { type: 'Align Items', members: 'items\\-(start|end|center|baseline|stretch)', + internal: true, }, { type: 'Align Self', @@ -349,10 +374,12 @@ module.exports.groups = [ { type: 'Place Content', members: 'place\\-content\\-(center|start|end|between|around|evenly|stretch|baseline)', + internal: true, }, { type: 'Place Items', members: 'place\\-items\\-(start|end|center|stretch|baseline)', + internal: true, }, { type: 'Place Self', @@ -365,6 +392,7 @@ module.exports.groups = [ members: [ { type: 'Padding', + internal: true, members: [ { type: 'p', @@ -459,6 +487,7 @@ module.exports.groups = [ }, { type: 'Space Between', + internal: true, members: [ { type: 'space-y', @@ -515,6 +544,7 @@ module.exports.groups = [ }, { type: 'Typography', + internal: true, members: [ { type: 'Font Family', @@ -626,6 +656,7 @@ module.exports.groups = [ }, { type: 'Backgrounds', + internal: true, members: [ { type: 'Background Image URL', @@ -854,6 +885,7 @@ module.exports.groups = [ }, { type: 'Divide Width', + internal: true, members: [ { type: 'divide-y', @@ -875,26 +907,32 @@ module.exports.groups = [ }, { type: 'Divide Color', + internal: true, members: 'divide\\-(?${divideColor})', }, { type: 'Divide Style', + internal: true, members: 'divide\\-(solid|dashed|dotted|double|none)', }, { type: 'Outline Width', + internal: true, members: 'outline\\-(?${outlineWidth})', }, { type: 'Outline Color', + internal: true, members: 'outline\\-(?${outlineColor})', }, { type: 'Outline Style', + internal: true, members: 'outline(\\-(none|dashed|dotted|double|hidden))?', }, { type: 'Outline Offset', + internal: true, members: '(outline\\-offset\\-(?${outlineOffset})|\\-outline\\-offset\\-(?${-outlineOffset}))', }, @@ -1047,6 +1085,7 @@ module.exports.groups = [ }, { type: 'Tables', + internal: true, members: [ { type: 'Border Collapse', @@ -1406,6 +1445,7 @@ module.exports.groups = [ }, { type: 'Line Clamp', + internal: true, members: 'line\\-clamp\\-(none|(?${lineClamp}))', }, ], diff --git a/lib/index.js b/lib/index.js index 83bef573..d20e45cd 100644 --- a/lib/index.js +++ b/lib/index.js @@ -19,6 +19,7 @@ module.exports = { 'no-arbitrary-value': require(base + 'no-arbitrary-value'), 'no-contradicting-classname': require(base + 'no-contradicting-classname'), 'no-custom-classname': require(base + 'no-custom-classname'), + 'no-internal-style': require(base + 'no-internal-style'), }, configs: { recommended: { @@ -36,6 +37,7 @@ module.exports = { 'tailwindcss/no-arbitrary-value': 'off', 'tailwindcss/no-custom-classname': 'warn', 'tailwindcss/no-contradicting-classname': 'error', + 'tailwindcss/no-internal-style': 'off', }, }, }, diff --git a/lib/rules/no-internal-style.js b/lib/rules/no-internal-style.js new file mode 100644 index 00000000..09e2ebde --- /dev/null +++ b/lib/rules/no-internal-style.js @@ -0,0 +1,224 @@ +/** + * @fileoverview Forbid using internal values in classnames passed to components + * @author Georgi Angelov + */ +'use strict'; + +const docsUrl = require('../util/docsUrl'); +const customConfig = require('../util/customConfig'); +const astUtil = require('../util/ast'); +const groupUtil = require('../util/groupMethods'); +const getOption = require('../util/settings'); +const parserUtil = require('../util/parser'); +const groups = require('../config/groups').groups; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +// Predefine message for use in context.report conditional. +// messageId will still be usable in tests. +const INTERNAL_STYLE_DETECTED_MSG = `Internal style '{{classname}}' should not be passed to component`; + +module.exports = { + meta: { + docs: { + // TODO + description: 'Forbid using arbitrary values in classnames', + category: 'Best Practices', + recommended: false, + url: docsUrl('no-arbitrary-value'), + }, + messages: { + internalStyleDetected: INTERNAL_STYLE_DETECTED_MSG, + }, + fixable: null, + schema: [ + { + type: 'object', + properties: { + externalClassRegex: { type: 'string' }, + externalCallees: { + type: 'array', + items: { type: 'string', minLength: 0 }, + uniqueItems: true, + }, + ignoredKeys: { + type: 'array', + items: { type: 'string', minLength: 0 }, + uniqueItems: true, + }, + config: { + // returned from `loadConfig()` utility + type: ['string', 'object'], + }, + tags: { + type: 'array', + items: { type: 'string', minLength: 0 }, + uniqueItems: true, + }, + }, + }, + ], + }, + + create: function (context) { + const callees = getOption(context, 'externalCallees'); + const skipClassAttribute = getOption(context, 'skipClassAttribute'); + const tags = getOption(context, 'tags'); + const twConfig = getOption(context, 'config'); + const classRegex = getOption(context, 'externalClassRegex'); + + const mergedConfig = customConfig.resolve(twConfig); + + //---------------------------------------------------------------------- + // Helpers + //---------------------------------------------------------------------- + + /** + * Recursive function crawling into child nodes + * @param {ASTNode} node The root node of the current parsing + * @param {ASTNode} arg The child node of node + * @returns {void} + */ + const parseForArbitraryValues = (node, arg = null) => { + let originalClassNamesValue = null; + if (arg === null) { + originalClassNamesValue = astUtil.extractValueFromNode(node); + } else { + switch (arg.type) { + case 'Identifier': + return; + case 'TemplateLiteral': + arg.expressions.forEach((exp) => { + parseForArbitraryValues(node, exp); + }); + arg.quasis.forEach((quasis) => { + parseForArbitraryValues(node, quasis); + }); + return; + case 'ConditionalExpression': + parseForArbitraryValues(node, arg.consequent); + parseForArbitraryValues(node, arg.alternate); + return; + case 'LogicalExpression': + parseForArbitraryValues(node, arg.right); + return; + case 'ArrayExpression': + arg.elements.forEach((el) => { + parseForArbitraryValues(node, el); + }); + return; + case 'ObjectExpression': + const isUsedByClassNamesPlugin = node.callee && node.callee.name === 'classnames'; + const isVue = node.key && node.key.type === 'VDirectiveKey'; + arg.properties.forEach((prop) => { + const propVal = isUsedByClassNamesPlugin || isVue ? prop.key : prop.value; + parseForArbitraryValues(node, propVal); + }); + return; + case 'Property': + parseForArbitraryValues(node, arg.key); + return; + case 'Literal': + originalClassNamesValue = arg.value; + break; + case 'TemplateElement': + originalClassNamesValue = arg.value.raw; + if (originalClassNamesValue === '') { + return; + } + break; + } + } + + let { classNames } = astUtil.extractClassnamesFromValue(originalClassNamesValue); + console.log('-------------------------'); + console.log(classNames); + + const forbidden = classNames.filter((cls, idx) => { + const parsed = groupUtil.parseClassname(cls, groups, mergedConfig, idx); + + // console.log({ cls, idx, parsed }); + + return parsed.internal; + }); + + forbidden.forEach((forbiddenClass) => { + context.report({ + node, + messageId: 'internalStyleDetected', + data: { + classname: forbiddenClass, + }, + }); + }); + }; + + //---------------------------------------------------------------------- + // Public + //---------------------------------------------------------------------- + + const attributeVisitor = function (node) { + if (!astUtil.isClassAttribute(node, classRegex) || skipClassAttribute) { + return; + } + if (astUtil.isLiteralAttributeValue(node)) { + parseForArbitraryValues(node); + } else if (node.value && node.value.type === 'JSXExpressionContainer') { + parseForArbitraryValues(node, node.value.expression); + } + }; + + const callExpressionVisitor = function (node) { + const calleeStr = astUtil.calleeToString(node.callee); + if (callees.findIndex((name) => calleeStr === name) === -1) { + return; + } + node.arguments.forEach((arg) => { + parseForArbitraryValues(node, arg); + }); + }; + + const scriptVisitor = { + JSXAttribute: attributeVisitor, + TextAttribute: attributeVisitor, + CallExpression: callExpressionVisitor, + TaggedTemplateExpression: function (node) { + if (!tags.includes(node.tag.name)) { + return; + } + parseForArbitraryValues(node, node.quasi); + }, + }; + + const templateVisitor = { + CallExpression: callExpressionVisitor, + /* + Tagged templates inside data bindings + https://github.com/vuejs/vue/issues/9721 + */ + VAttribute: function (node) { + switch (true) { + case !astUtil.isValidVueAttribute(node, classRegex): + return; + case astUtil.isVLiteralValue(node): + parseForArbitraryValues(node, null); + break; + case astUtil.isArrayExpression(node): + node.value.expression.elements.forEach((arg) => { + parseForArbitraryValues(node, arg); + }); + break; + case astUtil.isObjectExpression(node): + node.value.expression.properties.forEach((prop) => { + parseForArbitraryValues(node, prop); + }); + break; + } + }, + }; + + return parserUtil.defineTemplateBodyVisitor(context, templateVisitor, scriptVisitor); + }, +}; diff --git a/lib/util/groupMethods.js b/lib/util/groupMethods.js index 7b595714..a81ee014 100644 --- a/lib/util/groupMethods.js +++ b/lib/util/groupMethods.js @@ -520,7 +520,13 @@ function findInGroup(name, group, config, parentType = null) { if (!innerGroup) { return null; } else { - return findInGroup(name, innerGroup, config, group.type); + const result = findInGroup(name, innerGroup, config, group.type); + + return { + ...result, + + internal: result.internal === undefined || result.internal === null ? group.internal : result.internal, + }; } } } @@ -575,6 +581,7 @@ function parseClassname(name, arr, config, index = null) { leading: leading, trailing: trailing, important: body.substr(0, 1) === '!', + internal: slot && slot.internal, }; } diff --git a/lib/util/settings.js b/lib/util/settings.js index 6674463b..3887d694 100644 --- a/lib/util/settings.js +++ b/lib/util/settings.js @@ -1,5 +1,5 @@ 'use strict'; -const { resolveDefaultConfigPath } = require('tailwindcss/lib/util/resolveConfigPath') +const { resolveDefaultConfigPath } = require('tailwindcss/lib/util/resolveConfigPath'); function getOption(context, name) { // Options (defined at rule level) @@ -15,10 +15,14 @@ function getOption(context, name) { switch (name) { case 'callees': return ['classnames', 'clsx', 'ctl', 'cva', 'tv']; + case 'externalCallees': + return []; case 'ignoredKeys': return ['compoundVariants', 'defaultVariants']; case 'classRegex': return '^class(Name)?$'; + case 'externalClassRegex': + return '^classes$'; case 'config': return resolveDefaultConfigPath(); case 'cssFiles': diff --git a/tests/lib/rules/no-internal-style.js b/tests/lib/rules/no-internal-style.js new file mode 100644 index 00000000..b0061450 --- /dev/null +++ b/tests/lib/rules/no-internal-style.js @@ -0,0 +1,83 @@ +/** + * @fileoverview Forbid using arbitrary values in classnames + * @author François Massart + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var rule = require("../../../lib/rules/no-internal-style"); +var RuleTester = require("eslint").RuleTester; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +var parserOptions = { + ecmaVersion: 2019, + sourceType: "module", + ecmaFeatures: { + jsx: true, + }, +}; + +const options = [ + { + classRegex: "^classes$", + externalCallees: ["externcss"], + }, +]; + +var generateErrors = (classnames) => { + const errors = []; + if (typeof classnames === "string") { + classnames = classnames.split(" "); + } + classnames.map((classname) => { + errors.push({ + messageId: "internalStyleDetected", + data: { + classname: classname, + }, + }); + }); + return errors; +}; + +var ruleTester = new RuleTester({ parserOptions }); + +ruleTester.run("no-internal-style", rule, { + valid: [ + { + code: `Not using internal style property`, + options, + }, + { + code: `Not using internal style property`, + options, + }, + { + code: `Not using internal style property`, + options, + }, + { + code: `No internal styles`, + options, + }, + ], + + invalid: [ + { + code: `Padding is an internal style!`, + errors: generateErrors("pt-2"), + options, + }, + { + code: `Padding is an internal style!`, + errors: generateErrors("pt-2"), + options, + }, + ], +}); diff --git a/tests/lib/util/groupMethods.js b/tests/lib/util/groupMethods.js index 9583f067..7dc42b23 100644 --- a/tests/lib/util/groupMethods.js +++ b/tests/lib/util/groupMethods.js @@ -60,6 +60,7 @@ describe("parseClassname", function () { leading: "", trailing: "", important: false, + internal: true, }; assert.deepEqual(actual, expected); name = "md:overflow-y-auto"; @@ -69,6 +70,7 @@ describe("parseClassname", function () { expected.body = "overflow-y-"; expected.shorthand = "y"; expected.variants = "md:"; + expected.internal = true; assert.deepEqual(actual, expected); name = "lg:dark:overflow-auto"; actual = groupUtil.parseClassname(name, targetGroups, mergedConfig, 2); @@ -77,6 +79,7 @@ describe("parseClassname", function () { expected.body = "overflow-"; expected.shorthand = "all"; expected.variants = "lg:dark:"; + expected.internal = true; assert.deepEqual(actual, expected); name = "sm:dark:overscroll-x-none"; actual = groupUtil.parseClassname(name, targetGroups, mergedConfig, 3); @@ -87,6 +90,7 @@ describe("parseClassname", function () { expected.parentType = "Overscroll Behavior"; expected.body = "overscroll-x-"; expected.value = "none"; + expected.internal = true; assert.deepEqual(actual, expected); name = "inset-0"; actual = groupUtil.parseClassname(name, targetGroups, mergedConfig, 4); @@ -97,6 +101,7 @@ describe("parseClassname", function () { expected.parentType = "Top / Right / Bottom / Left"; expected.body = "inset-"; expected.value = "0"; + expected.internal = undefined; assert.deepEqual(actual, expected); name = "sm:-inset-x-1"; actual = groupUtil.parseClassname(name, targetGroups, mergedConfig, 5); @@ -106,6 +111,7 @@ describe("parseClassname", function () { expected.variants = "sm:"; expected.body = "inset-x-"; expected.value = "-1"; + expected.internal = undefined; assert.deepEqual(actual, expected); name = "sm:-inset-x-1"; actual = groupUtil.parseClassname(name, targetGroups, mergedConfig, 6); @@ -115,6 +121,7 @@ describe("parseClassname", function () { expected.variants = "sm:"; expected.body = "inset-x-"; expected.value = "-1"; + expected.internal = undefined; assert.deepEqual(actual, expected); name = "gap-px"; actual = groupUtil.parseClassname(name, targetGroups, mergedConfig, 7); @@ -125,6 +132,7 @@ describe("parseClassname", function () { expected.parentType = "Gap"; expected.body = "gap-"; expected.value = "px"; + expected.internal = true; assert.deepEqual(actual, expected); name = "p-5"; actual = groupUtil.parseClassname(name, targetGroups, mergedConfig, 8); @@ -135,6 +143,7 @@ describe("parseClassname", function () { expected.parentType = "Padding"; expected.body = "p-"; expected.value = "5"; + expected.internal = true; assert.deepEqual(actual, expected); name = "-my-px"; actual = groupUtil.parseClassname(name, targetGroups, mergedConfig, 9); @@ -145,6 +154,7 @@ describe("parseClassname", function () { expected.parentType = "Margin"; expected.body = "my-"; expected.value = "-px"; + expected.internal = undefined; assert.deepEqual(actual, expected); // "Border Radius" @@ -157,6 +167,7 @@ describe("parseClassname", function () { expected.parentType = "Border Radius"; expected.body = "rounded-tl-"; expected.value = "lg"; + expected.internal = undefined; assert.deepEqual(actual, expected); // "Border Width" @@ -169,6 +180,7 @@ describe("parseClassname", function () { expected.parentType = "Border Width"; expected.body = "border-t-"; expected.value = "4"; + expected.internal = undefined; assert.deepEqual(actual, expected); // "Border Spacing" @@ -181,6 +193,7 @@ describe("parseClassname", function () { expected.parentType = "Border Spacing"; expected.body = "border-spacing-x-"; expected.value = "96"; + expected.internal = true; assert.deepEqual(actual, expected); // "Scale" @@ -193,6 +206,7 @@ describe("parseClassname", function () { expected.parentType = "Scale"; expected.body = "scale-x-"; expected.value = "150"; + expected.internal = undefined; assert.deepEqual(actual, expected); // Margin arbitrary value @@ -205,6 +219,7 @@ describe("parseClassname", function () { expected.parentType = "Margin"; expected.body = "m-"; expected.value = "[0]"; + expected.internal = undefined; assert.deepEqual(actual, expected); // Leading / Trailing @@ -219,6 +234,7 @@ describe("parseClassname", function () { expected.value = "2"; expected.leading = " "; expected.trailing = " "; + expected.internal = true; assert.deepEqual(actual, expected); // Important @@ -235,6 +251,7 @@ describe("parseClassname", function () { expected.variants = "md:"; expected.value = "8"; expected.important = true; + expected.internal = true; assert.deepEqual(actual, expected); }); @@ -282,6 +299,7 @@ describe("getGroupIndex", function () { leading: "", trailing: "", important: false, + internal: true, }; assert.deepEqual(actual, expected); });