Skip to content

Commit

Permalink
feat: don't require $ReadOnlyArray if variable is initialized with em…
Browse files Browse the repository at this point in the history
…pty array (#337)

* feat: Don't require $ReadOnlyArray if variable is initialized with empty array

* docs: phrasing
  • Loading branch information
pnevyk authored and gajus committed Jun 1, 2018
1 parent ff4f857 commit 7c57cea
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 10 deletions.
2 changes: 2 additions & 0 deletions .README/rules/no-mutable-array.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ General reasons for using immutable data structures:
* They always have failure atomicity
* They are much easier to cache

Note that initialization of a variable with an empty array is considered valid (e.g., `const values: Array<string> = [];`). This behavior resembles the behavior of Flow's [unsealed objects](https://flow.org/en/docs/types/objects/#toc-unsealed-objects), as it is assumed that empty array is intended to be mutated.

<!-- assertions noMutableArray -->
49 changes: 39 additions & 10 deletions src/rules/noMutableArray.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,49 @@
import _ from 'lodash';

const schema = [];

// const x = [];
const isEmptyArrayLiteral = (node) => {
return _.get(node, 'init.type') === 'ArrayExpression' && _.get(node, 'init.elements.length') === 0;
};

// const x = new Array(); const y = Array();
const isEmptyArrayInstance = (node) => {
if (_.get(node, 'init.type') === 'NewExpression' || _.get(node, 'init.type') === 'CallExpression') {
return _.get(node, 'init.callee.name') === 'Array' && _.get(node, 'init.arguments.length') === 0;
} else {
return false;
}
};

const isAnnotationOfEmptyArrayInit = (node) => {
if (_.has(node, 'parent.parent.parent')) {
const parent = _.get(node, 'parent.parent.parent');
const isVariableDeclaration = _.get(parent, 'type') === 'VariableDeclarator';

return isVariableDeclaration && (isEmptyArrayLiteral(parent) || isEmptyArrayInstance(parent));
} else {
return false;
}
};

const create = (context) => {
return {
ArrayTypeAnnotation (node) {
context.report({
fix (fixer) {
const rawElementType = context.getSourceCode().getText(node.elementType);

return fixer.replaceText(node, '$ReadOnlyArray<' + rawElementType + '>');
},
message: 'Use "$ReadOnlyArray" instead of array shorthand notation',
node
});
if (!isAnnotationOfEmptyArrayInit(node)) {
context.report({
fix (fixer) {
const rawElementType = context.getSourceCode().getText(node.elementType);

return fixer.replaceText(node, '$ReadOnlyArray<' + rawElementType + '>');
},
message: 'Use "$ReadOnlyArray" instead of array shorthand notation',
node
});
}
},
GenericTypeAnnotation (node) {
if (node.id.name === 'Array') {
if (node.id.name === 'Array' && !isAnnotationOfEmptyArrayInit(node)) {
context.report({
fix (fixer) {
return fixer.replaceText(node.id, '$ReadOnlyArray');
Expand Down
25 changes: 25 additions & 0 deletions tests/rules/assertions/noMutableArray.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,36 @@ export default {
code: 'type X = string[]',
errors: [{message: 'Use "$ReadOnlyArray" instead of array shorthand notation'}],
output: 'type X = $ReadOnlyArray<string>'
},
{
code: 'const values: Array<Array<string>> = [];',
errors: [{message: 'Use "$ReadOnlyArray" instead of "Array"'}],
output: 'const values: Array<$ReadOnlyArray<string>> = [];'
},
{
code: 'let values: Array<Array<string>>;',
errors: [
{message: 'Use "$ReadOnlyArray" instead of "Array"'},
{message: 'Use "$ReadOnlyArray" instead of "Array"'}
],
output: 'let values: $ReadOnlyArray<$ReadOnlyArray<string>>;'
}
],
valid: [
{
code: 'type X = $ReadOnlyArray<string>'
},
{
code: 'const values: Array<$ReadOnlyArray<string>> = [];'
},
{
code: 'const values: $ReadOnlyArray<string>[] = [];'
},
{
code: 'const values: Array<$ReadOnlyArray<string>> = new Array();'
},
{
code: 'const values: Array<$ReadOnlyArray<string>> = Array();'
}
]
};

0 comments on commit 7c57cea

Please sign in to comment.