Skip to content

Commit

Permalink
Add suggestion for 'no-collection-size-mischeck' (#340)
Browse files Browse the repository at this point in the history
  • Loading branch information
yassin-kammoun-sonarsource committed Mar 24, 2022
1 parent aac249c commit b980329
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 1 deletion.
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -27,7 +27,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
* "if ... else if" constructs should end with "else" clauses ([`elseif-without-else`]) (*disabled*)
* "switch" statements should not have too many "case" clauses ([`max-switch-cases`])
* Collapsible "if" statements should be merged ([`no-collapsible-if`])
* Collection sizes and array length comparisons should make sense ([`no-collection-size-mischeck`]) (*uses-types*)
* Collection sizes and array length comparisons should make sense ([`no-collection-size-mischeck`]) (:wrench: *fixable*, *uses-types*)
* String literals should not be duplicated ([`no-duplicate-string`])
* Two branches in a conditional structure should not have exactly the same implementation ([`no-duplicated-branches`])
* Boolean expressions should not be gratuitous ([`no-gratuitous-expressions`])
Expand Down
2 changes: 2 additions & 0 deletions docs/rules/no-collection-size-mischeck.md
@@ -1,5 +1,7 @@
# no-collection-size-mischeck

:wrench: *fixable*

The size of a collection and the length of an array are always greater than or equal to zero. So testing that a size or length is greater than or equal to zero doesn't make sense, since the result is always `true`. Similarly testing that it is less than zero will always return `false`. Perhaps the intent was to check the non-emptiness of the collection or array instead.

## Noncompliant Code Example
Expand Down
25 changes: 25 additions & 0 deletions src/rules/no-collection-size-mischeck.ts
Expand Up @@ -31,9 +31,11 @@ const rule: TSESLint.RuleModule<string, string[]> = {
messages: {
fixCollectionSizeCheck:
'Fix this expression; {{propertyName}} of "{{objectName}}" is always greater or equal to zero.',
suggestFixedSizeCheck: 'Use "{{operator}}" for {{operation}} check',
},
schema: [],
type: 'problem',
hasSuggestions: true,
docs: {
description: 'Collection sizes and array length comparisons should make sense',
recommended: 'error',
Expand Down Expand Up @@ -63,6 +65,7 @@ const rule: TSESLint.RuleModule<string, string[]> = {
objectName: context.getSourceCode().getText(object),
},
node,
suggest: getSuggestion(expr, property.name, context),
});
}
}
Expand All @@ -82,4 +85,26 @@ function isCollection(node: TSESTree.Node, services: RequiredParserServices) {
return !!tp.symbol && CollectionLike.includes(tp.symbol.name);
}

function getSuggestion(
expr: TSESTree.BinaryExpression,
operation: string,
context: TSESLint.RuleContext<string, string[]>,
): TSESLint.ReportSuggestionArray<string> {
const { left, operator } = expr;
const operatorToken = context
.getSourceCode()
.getTokenAfter(left, token => token.value === operator)!;
const fixedOperator = operator === '<' ? '==' : '>';
return [
{
messageId: 'suggestFixedSizeCheck',
data: {
operation,
operator: fixedOperator,
},
fix: fixer => fixer.replaceText(operatorToken, fixedOperator),
},
];
}

export = rule;
20 changes: 20 additions & 0 deletions tests/rules/no-collection-size-mischeck.test.ts
Expand Up @@ -54,6 +54,16 @@ ruleTester.run('Collection sizes and array length comparisons should make sense'
endLine: 1,
column: 5,
endColumn: 24,
suggestions: [
{
messageId: 'suggestFixedSizeCheck',
data: {
operation: 'size',
operator: '==',
},
output: `if (collection.size == 0) {}`,
},
],
},
],
},
Expand Down Expand Up @@ -86,6 +96,16 @@ ruleTester.run('Collection sizes and array length comparisons should make sense'
endLine: 1,
column: 5,
endColumn: 27,
suggestions: [
{
messageId: 'suggestFixedSizeCheck',
data: {
operation: 'length',
operator: '>',
},
output: `if (collection.length > 0) {}`,
},
],
},
],
},
Expand Down

0 comments on commit b980329

Please sign in to comment.