diff --git a/README.md b/README.md index fe88a45638..640e28aa5a 100644 --- a/README.md +++ b/README.md @@ -245,6 +245,7 @@ rules in templates can be disabled with eslint directives with mustache or html | [template-no-obsolete-elements](docs/rules/template-no-obsolete-elements.md) | disallow obsolete HTML elements | | | | | [template-no-outlet-outside-routes](docs/rules/template-no-outlet-outside-routes.md) | disallow {{outlet}} outside of route templates | | | | | [template-no-page-title-component](docs/rules/template-no-page-title-component.md) | disallow usage of ember-page-title component | | | | +| [template-simple-unless](docs/rules/template-simple-unless.md) | require simple conditions in unless blocks | | | | | [template-splat-attributes-only](docs/rules/template-splat-attributes-only.md) | disallow ...spread other than ...attributes | | | | | [template-style-concatenation](docs/rules/template-style-concatenation.md) | disallow string concatenation in inline styles | | | | diff --git a/docs/rules/template-simple-unless.md b/docs/rules/template-simple-unless.md new file mode 100644 index 0000000000..c239b7dd8c --- /dev/null +++ b/docs/rules/template-simple-unless.md @@ -0,0 +1,71 @@ +# ember/template-simple-unless + + + +Require simple conditions in `{{#unless}}` blocks. Complex expressions should use `{{#if}}` with negation instead. + +## Rule Details + +This rule enforces using simple property paths in `{{#unless}}` blocks rather than complex helper expressions. + +## Examples + +Examples of **incorrect** code for this rule: + +```gjs + +``` + +```gjs + +``` + +Examples of **correct** code for this rule: + +```gjs + +``` + +```gjs + +``` + +```gjs + +``` + +## Options + +| Name | Type | Default | Description | +| ------------ | ---------- | ------- | --------------------------------------------------------------------------- | +| `allowlist` | `string[]` | `[]` | Helper names allowed inside `{{unless}}`. | +| `denylist` | `string[]` | `[]` | Helper names explicitly denied inside `{{unless}}`. | +| `maxHelpers` | `integer` | `1` | Maximum number of helpers allowed inside `{{unless}}` (`-1` for unlimited). | + +## Related Rules + +- [no-negated-condition](template-no-negated-condition.md) + +## References + +- [Wikipedia/boolean algebra](https://en.wikipedia.org/wiki/Boolean_algebra) diff --git a/lib/rules/template-simple-unless.js b/lib/rules/template-simple-unless.js new file mode 100644 index 0000000000..b9400020e2 --- /dev/null +++ b/lib/rules/template-simple-unless.js @@ -0,0 +1,155 @@ +function isUnless(node) { + return node.path?.type === 'GlimmerPathExpression' && node.path.original === 'unless'; +} + +function isIf(node) { + return node.path?.type === 'GlimmerPathExpression' && node.path.original === 'if'; +} + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'require simple conditions in unless blocks', + category: 'Best Practices', + url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-simple-unless.md', + templateMode: 'both', + }, + schema: [ + { + type: 'object', + properties: { + allowlist: { type: 'array', items: { type: 'string' } }, + denylist: { type: 'array', items: { type: 'string' } }, + maxHelpers: { type: 'integer' }, + }, + additionalProperties: false, + }, + ], + messages: { + followingElseBlock: 'Using an `else` block with `unless` should be avoided.', + asElseUnlessBlock: 'Using an `else unless` block should be avoided.', + withHelper: '{{message}}', + }, + originallyFrom: { + name: 'ember-template-lint', + rule: 'lib/rules/simple-unless.js', + docs: 'docs/rule/simple-unless.md', + tests: 'test/unit/rules/simple-unless-test.js', + }, + }, + + create(context) { + const options = context.options[0] || {}; + const allowlist = options.allowlist || []; + const denylist = options.denylist || []; + const maxHelpers = options.maxHelpers === undefined ? 1 : options.maxHelpers; + const sourceCode = context.getSourceCode(); + + function isElseUnlessBlock(node) { + if (!node) { + return false; + } + if (node.path?.type === 'GlimmerPathExpression' && node.path.original === 'unless') { + const text = sourceCode.getText(node); + return text.startsWith('{{else '); + } + return false; + } + + function checkWithHelper(node) { + let helperCount = 0; + let nextParams = node.params || []; + + do { + const currentParams = nextParams; + nextParams = []; + + for (const param of currentParams) { + if (param.type === 'GlimmerSubExpression') { + helperCount++; + const helperName = param.path?.original || ''; + + if (maxHelpers > -1 && helperCount > maxHelpers) { + context.report({ + node: param, + messageId: 'withHelper', + data: { + message: `Using {{unless}} in combination with other helpers should be avoided. MaxHelpers: ${maxHelpers}`, + }, + }); + return; + } + + if (allowlist.length > 0 && !allowlist.includes(helperName)) { + context.report({ + node: param, + messageId: 'withHelper', + data: { + message: `Using {{unless}} in combination with other helpers should be avoided. Allowed helper${allowlist.length > 1 ? 's' : ''}: ${allowlist}`, + }, + }); + return; + } + + if (denylist.length > 0 && denylist.includes(helperName)) { + context.report({ + node: param, + messageId: 'withHelper', + data: { + message: `Using {{unless}} in combination with other helpers should be avoided. Restricted helper${denylist.length > 1 ? 's' : ''}: ${denylist}`, + }, + }); + return; + } + + if (param.params) { + nextParams.push(...param.params); + } + } + } + } while (nextParams.some((p) => p.type === 'GlimmerSubExpression')); + } + + return { + GlimmerMustacheStatement(node) { + if (node.path?.type === 'GlimmerPathExpression' && node.path.original === 'unless') { + if (node.params?.[0]?.path) { + checkWithHelper(node); + } + } + }, + + GlimmerBlockStatement(node) { + const nodeInverse = node.inverse; + + if (nodeInverse && nodeInverse.body?.length > 0) { + if (isUnless(node)) { + // Check for {{#unless}}...{{else if}} + if (nodeInverse.body[0] && isIf(nodeInverse.body[0])) { + context.report({ + node: node.program || node, + messageId: 'followingElseBlock', + }); + } else { + // {{#unless}}...{{else}} + context.report({ + node: node.program || node, + messageId: 'followingElseBlock', + }); + } + } else if (isElseUnlessBlock(nodeInverse.body[0])) { + // {{#if}}...{{else unless}} + context.report({ + node: nodeInverse.body[0], + messageId: 'asElseUnlessBlock', + }); + } + } else if (isUnless(node) && node.params?.[0]?.path) { + checkWithHelper(node); + } + }, + }; + }, +}; diff --git a/tests/lib/rules/template-simple-unless.js b/tests/lib/rules/template-simple-unless.js new file mode 100644 index 0000000000..459ccea02f --- /dev/null +++ b/tests/lib/rules/template-simple-unless.js @@ -0,0 +1,442 @@ +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/template-simple-unless'); +const RuleTester = require('eslint').RuleTester; + +const ruleTester = new RuleTester({ + parser: require.resolve('ember-eslint-parser'), + parserOptions: { ecmaVersion: 2022, sourceType: 'module' }, +}); + +ruleTester.run('template-simple-unless', rule, { + valid: [ + '', + '', + + "", + '', + '', + '', + '', + '', + '', + '', + { + code: '', + options: [{ maxHelpers: 2 }], + }, + '', + ], + invalid: [ + { + code: '', + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: '', + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + + { + code: "", + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: "", + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: '', + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: '', + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + ], +}); + +const hbsRuleTester = new RuleTester({ + parser: require.resolve('ember-eslint-parser/hbs'), + parserOptions: { + ecmaVersion: 2022, + sourceType: 'module', + }, +}); + +hbsRuleTester.run('template-simple-unless', rule, { + valid: [ + "{{#unless isRed}}I'm blue, da ba dee da ba daa{{/unless}}", + '
', + '
', + '{{unrelated-mustache-without-params}}', + '{{#if foo}}{{else}}{{/if}}', + '{{#if foo}}{{else}}{{#unless bar}}{{/unless}}{{/if}}', + '{{#if foo}}{{else}}{{unless bar someProperty}}{{/if}}', + '{{#unless (or foo bar)}}order whiskey{{/unless}}', + { + code: '{{#unless (eq (or foo bar) baz)}}order whiskey{{/unless}}', + options: [{ maxHelpers: 2 }], + }, + '{{unless foo bar}}', + '{{unless (eq foo bar) baz}}', + '{{unless (and isBad isAwful) "notBadAndAwful"}}', + '', + '', + ['{{#unless hamburger}}', ' HOT DOG!', '{{/unless}}'].join('\n'), + // allowlist + maxHelpers + { + code: '{{unless (eq foo bar) baz}}', + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + }, + { + code: '{{#unless (eq foo bar)}}baz{{/unless}}', + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + }, + { + code: '{{unless (eq (not foo) bar) baz}}', + options: [{ allowlist: [], maxHelpers: 2 }], + }, + { + code: '{{#unless (eq (not foo) bar)}}baz{{/unless}}', + options: [{ allowlist: [], maxHelpers: 2 }], + }, + { + code: '{{#unless (eq (not foo) bar)}}baz{{/unless}}', + options: [{ maxHelpers: 2 }], + }, + { + code: '{{#unless (eq (not foo) bar)}}baz{{/unless}}', + options: [{ maxHelpers: -1 }], + }, + { + code: '{{#unless (eq (not foo) bar)}}baz{{/unless}}', + options: [{ maxHelpers: -1, denylist: [] }], + }, + { + code: '{{#unless (eq (not foo) bar)}}baz{{/unless}}', + options: [{ maxHelpers: -1, denylist: ['or'] }], + }, + // valid with ETL-style allowlist config: helpers in allowlist, count within maxHelpers + { + code: '{{#unless (or foo bar)}}order whiskey{{/unless}}', + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + }, + { + code: '{{#unless (eq (or foo bar) baz)}}order whiskey{{/unless}}', + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + }, + ], + invalid: [ + // inline unless with nested helpers (exceeds default maxHelpers=1) + { + code: "{{unless (if (or true)) 'Please no'}}", + output: null, + errors: [{ messageId: 'withHelper' }], + }, + // inline unless with helper — maxHelpers: 0 + { + code: "{{unless (if true) 'Please no'}}", + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: "{{unless (and isBad isAwful) 'notBadAndAwful'}}", + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: '', + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: '', + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + // {{else}} block with {{unless}} + { + code: [ + '{{#unless bandwagoner}}', + ' Go Niners!', + '{{else}}', + ' Go Seahawks!', + '{{/unless}}', + ].join('\n'), + output: null, + errors: [{ messageId: 'followingElseBlock' }], + }, + { + code: ['{{#unless bandwagoner}}', 'Test1', '{{else}}', 'Test2', '{{/unless}}'].join('\n'), + output: null, + errors: [{ messageId: 'followingElseBlock' }], + }, + { + code: [ + '{{#unless bandwagoner}}', + '{{else}}', + ' {{#my-component}}', + ' {{/my-component}}', + '{{/unless}}', + ].join('\n'), + output: null, + errors: [{ messageId: 'followingElseBlock' }], + }, + // {{else if}} with {{unless}} + { + code: [ + '{{#unless bandwagoner}}', + ' Go Niners!', + '{{else if goHawks}}', + ' Go Seahawks!', + '{{/unless}}', + ].join('\n'), + output: null, + errors: [{ messageId: 'followingElseBlock' }], + }, + { + code: [ + '{{#unless bandwagoner}}', + ' Go Niners!', + '{{else if goPats}}', + ' Tom Brady is GOAT', + '{{else if goHawks}}', + ' Go Seahawks!', + '{{/unless}}', + ].join('\n'), + output: null, + errors: [{ messageId: 'followingElseBlock' }], + }, + { + code: [ + '{{#unless bandwagoner}}', + ' Go Niners!', + '{{else if goBengals}}', + ' Ouch, sorry', + '{{else}}', + ' Go Seahawks!', + '{{/unless}}', + ].join('\n'), + output: null, + errors: [{ messageId: 'followingElseBlock' }], + }, + // {{else unless}} + { + code: ['{{#if dog}}', ' Ruff Ruff!', '{{else unless cat}}', ' not cat', '{{/if}}'].join( + '\n' + ), + output: null, + errors: [{ messageId: 'asElseUnlessBlock' }], + }, + // block unless with disallowed helper (via allowlist) + { + code: ['{{#unless (and isFruit isYellow)}}', ' I am a green celery!', '{{/unless}}'].join( + '\n' + ), + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: [ + '{{#unless (not isBrown isSticky)}}', + ' I think I am a brown stick', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: ['{{#unless (not isBrown)}}', ' I think I am a brown', '{{/unless}}'].join('\n'), + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: [ + '{{#unless isSticky}}', + ' I think I am a brown stick', + '{{else}}', + ' Not a brown stick', + '{{/unless}}', + ].join('\n'), + output: null, + errors: [{ messageId: 'followingElseBlock' }], + }, + // maxHelpers exceeded (top-level params only) + { + code: [ + '{{#unless (or foo bar) (eq baz beer) (not-eq x y)}}', + ' MUCH HELPERS, VERY BAD', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + // maxHelpers: 0 + { + code: [ + '{{#unless (concat "blue" "red")}}', + ' I think I am a brown stick', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ maxHelpers: 0 }], + errors: [{ messageId: 'withHelper' }], + }, + // allowlist — helper not in allowlist + { + code: ['{{#unless (one foo bar)}}', ' I think I am a brown stick', '{{/unless}}'].join('\n'), + output: null, + options: [{ allowlist: ['test'], maxHelpers: 1 }], + errors: [{ messageId: 'withHelper' }], + }, + // maxHelpers exceeded with multiple top-level subexpressions + { + code: [ + '{{#unless (one foo) (two bar) (three baz)}}', + ' I think I am a brown stick', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + // denylist — helper in denylist + { + code: ['{{#unless (two three)}}', ' I think I am a brown stick', '{{/unless}}'].join('\n'), + output: null, + options: [{ denylist: ['two'], maxHelpers: -1 }], + errors: [{ messageId: 'withHelper' }], + }, + { + code: ['{{#unless (two three)}}', ' I think I am a brown stick', '{{/unless}}'].join('\n'), + output: null, + options: [{ denylist: ['two', 'four'], maxHelpers: -1 }], + errors: [{ messageId: 'withHelper' }], + }, + // --- ETL-equivalent invalid cases with allowlist config --- + // allowlist violation: `if` not in allowlist (ETL bad case: {{unless (if (or true)) ...}}) + { + code: "{{unless (if (or true)) 'Please no'}}", + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + // allowlist violation: `if` not in allowlist (ETL bad case: {{unless (if true) ...}}) + { + code: "{{unless (if true) 'Please no'}}", + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + // allowlist violation: `and` not in allowlist (ETL bad case: {{unless (and isBad isAwful) ...}}) + { + code: "{{unless (and isBad isAwful) 'notBadAndAwful'}}", + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + // allowlist violation: `not` not in allowlist (ETL bad case: inline unless with not) + { + code: '', + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + // allowlist violation: `one` not in allowlist (ETL bad case: inline unless with one) + { + code: '', + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + // maxHelpers exceeded with nested allowed helpers (ETL bad case: 3 helpers > maxHelpers 2) + { + code: [ + '{{#unless (or (eq foo bar) (not-eq baz "beer"))}}', + ' MUCH HELPERS, VERY BAD', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ allowlist: ['or', 'eq', 'not-eq'], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + // allowlist violation with nested helpers (ETL bad case: allowlist=['test'], one not in allowlist) + { + code: [ + '{{#unless (one (test power) two)}}', + ' I think I am a brown stick', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ allowlist: ['test'], maxHelpers: 1 }], + errors: [{ messageId: 'withHelper' }], + }, + // maxHelpers exceeded with empty allowlist and nested helpers (ETL bad case: 3 helpers > maxHelpers 2) + { + code: [ + '{{#unless (one (two three) (four five))}}', + ' I think I am a brown stick', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ allowlist: [], maxHelpers: 2 }], + errors: [{ messageId: 'withHelper' }], + }, + // denylist with nested helpers (ETL bad case: denylist=['two'], two is in nested position) + { + code: [ + '{{#unless (one (two three) (four five))}}', + ' I think I am a brown stick', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ denylist: ['two'], maxHelpers: -1 }], + errors: [{ messageId: 'withHelper' }], + }, + // multi-item denylist with nested helpers (ETL bad case: denylist=['two','four']) + { + code: [ + '{{#unless (one (two three) (four five))}}', + ' I think I am a brown stick', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ denylist: ['two', 'four'], maxHelpers: -1 }], + errors: [{ messageId: 'withHelper' }], + }, + // denylist violation: `one` in denylist (ETL-style with denylist=['one']) + { + code: [ + '{{#unless (one (two three) (four five))}}', + ' I think I am a brown stick', + '{{/unless}}', + ].join('\n'), + output: null, + options: [{ denylist: ['one'], maxHelpers: -1 }], + errors: [{ messageId: 'withHelper' }], + }, + ], +});