Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ rules in templates can be disabled with eslint directives with mustache or html
| [template-no-multiple-empty-lines](docs/rules/template-no-multiple-empty-lines.md) | disallow multiple consecutive empty lines in templates | | 🔧 | |
| [template-no-mut-helper](docs/rules/template-no-mut-helper.md) | disallow usage of (mut) helper | | | |
| [template-no-negated-comparison](docs/rules/template-no-negated-comparison.md) | disallow negated comparisons in templates | | | |
| [template-no-negated-condition](docs/rules/template-no-negated-condition.md) | disallow negated conditions in if/unless | | 🔧 | |
| [template-no-nested-splattributes](docs/rules/template-no-nested-splattributes.md) | disallow nested ...attributes usage | | | |
| [template-no-obscure-array-access](docs/rules/template-no-obscure-array-access.md) | disallow obscure array access patterns like `objectPath.@each.property` | | 🔧 | |
| [template-no-obsolete-elements](docs/rules/template-no-obsolete-elements.md) | disallow obsolete HTML elements | | | |
Expand Down
53 changes: 53 additions & 0 deletions docs/rules/template-no-negated-condition.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# ember/template-no-negated-condition

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->

Disallow negated conditions in `{{#if}}` blocks. Use `{{#unless}}` instead or rewrite the condition.

## Rule Details

This rule discourages the use of `{{#if (not condition)}}` in favor of `{{#unless condition}}` for better readability.

## Examples

Examples of **incorrect** code for this rule:

```gjs
<template>
{{#if (not isValid)}}
Invalid
{{/if}}
</template>
```

Examples of **correct** code for this rule:

```gjs
<template>
{{#unless isValid}}
Invalid
{{/unless}}
</template>
<template>
{{#if isValid}}
Valid
{{/if}}
</template>
```

## Options

| Name | Type | Default | Description |
| ----------------- | --------- | ------- | ----------------------------------------------------------------------------------------------------------------------- |
| `simplifyHelpers` | `boolean` | `true` | When `true`, also reports negated comparison helpers (e.g. `(not (eq ...))`) and suggests using `(not-eq ...)` instead. |

## Related Rules

- [simple-unless](template-simple-unless.md)

## References

- [no-negated-condition](https://eslint.org/docs/rules/no-negated-condition) from eslint
295 changes: 295 additions & 0 deletions lib/rules/template-no-negated-condition.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,295 @@
const ERROR_MESSAGE_FLIP_IF =
'Change `{{if (not condition)}} ... {{else}} ... {{/if}}` to `{{if condition}} ... {{else}} ... {{/if}}`.';
const ERROR_MESSAGE_USE_IF = 'Change `unless (not condition)` to `if condition`.';
const ERROR_MESSAGE_USE_UNLESS = 'Change `if (not condition)` to `unless condition`.';
const ERROR_MESSAGE_NEGATED_HELPER = 'Simplify unnecessary negation of helper.';

const INVERTIBLE_HELPERS = new Set(['not', 'eq', 'not-eq', 'gt', 'gte', 'lt', 'lte']);

const HELPER_INVERSIONS = {
not: null, // special case
eq: 'not-eq',
'not-eq': 'eq',
gt: 'lte',
gte: 'lt',
lt: 'gte',
lte: 'gt',
};

function isIf(node) {
return node.path?.type === 'GlimmerPathExpression' && node.path.original === 'if';
}

function isUnless(node) {
return node.path?.type === 'GlimmerPathExpression' && node.path.original === 'unless';
}

function hasNotHelper(node) {
return (
node.params?.length > 0 &&
node.params[0].type === 'GlimmerSubExpression' &&
node.params[0].path?.type === 'GlimmerPathExpression' &&
node.params[0].path.original === 'not'
);
}

function hasNestedFixableHelper(node) {
const inner = node.params[0]?.params?.[0];
return (
inner &&
inner.path?.type === 'GlimmerPathExpression' &&
INVERTIBLE_HELPERS.has(inner.path.original)
);
}

function escapeRegExp(string) {
return string.replaceAll(/[$()*+.?[\\\]^{|}]/g, '\\$&');
}

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'disallow negated conditions in if/unless',
category: 'Best Practices',
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-negated-condition.md',
templateMode: 'both',
},
fixable: 'code',
schema: [
{
type: 'object',
properties: {
simplifyHelpers: { type: 'boolean' },
},
additionalProperties: false,
},
],
messages: {
flipIf: ERROR_MESSAGE_FLIP_IF,
useIf: ERROR_MESSAGE_USE_IF,
useUnless: ERROR_MESSAGE_USE_UNLESS,
negatedHelper: ERROR_MESSAGE_NEGATED_HELPER,
},
originallyFrom: {
name: 'ember-template-lint',
rule: 'lib/rules/no-negated-condition.js',
docs: 'docs/rule/no-negated-condition.md',
tests: 'test/unit/rules/no-negated-condition-test.js',
},
},

create(context) {
const options = context.options[0] || {};
const simplifyHelpers = options.simplifyHelpers === undefined ? true : options.simplifyHelpers;
const sourceCode = context.sourceCode;

/**
* Get the source text for the inner condition of a (not ...) sub-expression,
* with the nested helper optionally inverted.
*/
function getUnwrappedConditionText(notExpr, invertHelper) {
const inner = notExpr.params[0];
if (invertHelper && inner.path?.type === 'GlimmerPathExpression') {
const helperName = inner.path.original;
const inverted = HELPER_INVERSIONS[helperName];
if (inverted !== undefined) {
if (inverted === null) {
if (inner.params.length > 1) {
// (not (not c1 c2)) -> (or c1 c2)
const paramsText = inner.params.map((p) => sourceCode.getText(p)).join(' ');
return `(or ${paramsText})`;
}
// (not (not x)) -> just x
return sourceCode.getText(inner.params[0]);
}
// (not (eq a b)) -> (not-eq a b)
const innerText = sourceCode.getText(inner);
return innerText.replace(`(${helperName} `, `(${inverted} `);
}
}
return sourceCode.getText(inner);
}

/**
* Build a fix function for block statements.
*/
function buildBlockFix(node, messageId) {
return function fix(fixer) {
const fullText = sourceCode.getText(node);
const keyword = node.path.original;
const notExpr = node.params[0];

if (messageId === 'negatedHelper') {
const conditionText = getUnwrappedConditionText(notExpr, true);
const newText = fullText.replace(sourceCode.getText(notExpr), conditionText);
return fixer.replaceText(node, newText);
}

if (messageId === 'flipIf') {
// {{#if (not x)}}A{{else}}B{{/if}} -> {{#if x}}B{{else}}A{{/if}}
const conditionText = getUnwrappedConditionText(notExpr, false);
const programBody = node.program.body.map((n) => sourceCode.getText(n)).join('');
const inverseBody = node.inverse.body.map((n) => sourceCode.getText(n)).join('');

return fixer.replaceText(
node,
`{{#${keyword} ${conditionText}}}${inverseBody}{{else}}${programBody}{{/${keyword}}}`
);
}

if (messageId === 'useIf' || messageId === 'useUnless') {
const newKeyword = keyword === 'unless' ? 'if' : 'unless';
const conditionText = getUnwrappedConditionText(notExpr, false);
const notExprText = escapeRegExp(sourceCode.getText(notExpr));
const newText = fullText
.replace(
new RegExp(`^\\{\\{#${keyword} ${notExprText}`),
`{{#${newKeyword} ${conditionText}`
)
.replace(new RegExp(`\\{\\{/${keyword}\\}\\}$`), `{{/${newKeyword}}}`);
return fixer.replaceText(node, newText);
}

return null;
};
}

/**
* Build a fix function for inline (mustache/subexpression) statements.
*/
function buildInlineFix(node, messageId) {
return function fix(fixer) {
const fullText = sourceCode.getText(node);
const keyword = node.path.original;
const notExpr = node.params[0];

if (messageId === 'negatedHelper') {
const conditionText = getUnwrappedConditionText(notExpr, true);
const newText = fullText.replace(sourceCode.getText(notExpr), conditionText);
return fixer.replaceText(node, newText);
}

if (messageId === 'flipIf') {
const conditionText = getUnwrappedConditionText(notExpr, false);
const param1Text = sourceCode.getText(node.params[1]);
const param2Text = sourceCode.getText(node.params[2]);
const isSubExpr = node.type === 'GlimmerSubExpression';
const open = isSubExpr ? '(' : '{{';
const close = isSubExpr ? ')' : '}}';
return fixer.replaceText(
node,
`${open}${keyword} ${conditionText} ${param2Text} ${param1Text}${close}`
);
}

if (messageId === 'useIf' || messageId === 'useUnless') {
const newKeyword = keyword === 'unless' ? 'if' : 'unless';
const conditionText = getUnwrappedConditionText(notExpr, false);
const isSubExpr = node.type === 'GlimmerSubExpression';
const open = isSubExpr ? '(' : '{{';
const close = isSubExpr ? ')' : '}}';
const remainingParams = node.params
.slice(1)
.map((p) => sourceCode.getText(p))
.join(' ');
return fixer.replaceText(
node,
`${open}${newKeyword} ${conditionText} ${remainingParams}${close}`
);
}

return null;
};
}

// eslint-disable-next-line complexity
function checkNode(node) {
const nodeIsIf = isIf(node);
const nodeIsUnless = isUnless(node);

if (!nodeIsIf && !nodeIsUnless) {
return;
}

// Skip `{{else if ...}}` / `{{else unless ...}}` chains for the outer check,
// unless they have a fixable negated helper inside
if (node.type === 'GlimmerBlockStatement') {
const text = sourceCode.getText(node);
if (text.startsWith('{{else ')) {
if (!simplifyHelpers || !hasNotHelper(node) || !hasNestedFixableHelper(node)) {
return;
}
context.report({
node: node.params[0],
messageId: 'negatedHelper',
fix: buildBlockFix(node, 'negatedHelper'),
});
return;
}

// Ignore `if ... else if ...` (chained) to avoid forcing negation
if (
node.inverse?.body?.length > 0 &&
node.inverse.body[0].type === 'GlimmerBlockStatement' &&
nodeIsIf &&
isIf(node.inverse.body[0])
) {
return;
}
}

if (!hasNotHelper(node)) {
return;
}

const notExpr = node.params[0];
const hasFixableHelper = hasNestedFixableHelper(node);

// If it's `if (not (someHelper ...))` and we can't simplify the helper,
// don't suggest converting to `unless` (simple-unless rule would reject it)
if (
nodeIsIf &&
notExpr.params?.[0]?.type === 'GlimmerSubExpression' &&
(!simplifyHelpers || !hasFixableHelper)
) {
return;
}

// (not a b c) with multiple params — can't simply remove negation
if (notExpr.params?.length > 1) {
return;
}

// Determine message
const isIfElseBlock = node.type === 'GlimmerBlockStatement' && node.inverse?.body?.length > 0;
const isIfElseInline = node.type !== 'GlimmerBlockStatement' && node.params?.length === 3;
const shouldFlip = isIfElseBlock || isIfElseInline;

let messageId;
if (hasFixableHelper && simplifyHelpers) {
messageId = 'negatedHelper';
} else if (shouldFlip && nodeIsIf) {
messageId = 'flipIf';
} else if (nodeIsUnless) {
messageId = 'useIf';
} else {
messageId = 'useUnless';
}

const isBlock = node.type === 'GlimmerBlockStatement';
context.report({
node: notExpr,
messageId,
fix: isBlock ? buildBlockFix(node, messageId) : buildInlineFix(node, messageId),
});
}

return {
GlimmerBlockStatement: checkNode,
GlimmerMustacheStatement: checkNode,
GlimmerSubExpression: checkNode,
};
},
};
Loading
Loading