Skip to content

Commit

Permalink
Closes #18.
Browse files Browse the repository at this point in the history
  • Loading branch information
jonaskello committed Apr 9, 2017
1 parent 0179f35 commit a4a5cdf
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 97 deletions.
27 changes: 26 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,28 @@ type person = {

Yes, variable names like `mutableAge` are ugly, but then again mutation is an ugly business :-).

## Recommended built-in rules

# [no-var-keyword](https://palantir.github.io/tslint/rules/no-var-keyword/)

Without this rule, it is still possible to create `var` variables that are mutable.

# [typedef](https://palantir.github.io/tslint/rules/typedef/) with call-signature option

For performance reasons, tslint-immutable does not check implicit return types. So for example this function will return an mutable array but will not be detected (see [#18]()https://github.com/jonaskello/tslint-immutable/issues/18 for more info):

```javascript
function foo() {
return [1, 2, 3];
}
```

To avoid this situation you can enable the built in typedef rule like this:

`"typedef": [true, "call-signature"]`

Now the above function is forced to declare the return type becomes this and will be detected.

## Sample Configuration File

Here's a sample TSLint configuration file (tslint.json) that activates all the rules:
Expand All @@ -264,12 +286,15 @@ Here's a sample TSLint configuration file (tslint.json) that activates all the r
"rulesDirectory": ["./node_modules/tslint-immutable/rules"],
"rules": {

// Recommended built-in rules
"no-var-keyword": true,
"typedef": [true, "call-signature"],

// Immutability rules
"readonly-interface": true,
"readonly-indexer": true,
"readonly-array": true,
"no-let": true,
"no-var-keyword": true, // built-in tslint rule

// Functional style rules
"no-this": true,
Expand Down
37 changes: 0 additions & 37 deletions rules/readonlyArrayRule.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,27 +72,6 @@ function checkFunctionNode(node, ctx) {
if (node.type) {
checkArrayTypeReference(node.type, ctx);
}
else if (node.body) {
// No explicit return type, check the body for return statements
checkFunctionBody(node.body, ctx);
}
}
function checkFunctionBody(node, ctx) {
return ts.forEachChild(node, cb);
function cb(node) {
// Check for invalid return type
checkReturnStatement(node, ctx);
// Use return becuase performance hints docs say it optimizes the function using tail-call recursion
return ts.forEachChild(node, cb);
}
}
function checkReturnStatement(node, ctx) {
if (node.kind === ts.SyntaxKind.ReturnStatement) {
var returnNode = node;
if (returnNode.expression && returnNode.expression.kind === ts.SyntaxKind.ArrayLiteralExpression) {
ctx.addFailureAtNode(returnNode, Rule.FAILURE_STRING);
}
}
}
function checkArrayTypeReference(node, ctx) {
if (node.kind === ts.SyntaxKind.TypeReference) {
Expand Down Expand Up @@ -128,19 +107,3 @@ function checkArrayLiteralExpression(node, ctx) {
return;
}
}
// function isInvalidArrayLiteralExpression(node: ts.ArrayLiteralExpression, ctx: Lint.WalkContext<Options>): boolean {
// // If the array literal is used in a variable declaration, the variable
// // must have a type spcecified, otherwise it will implicitly be of mutable Array type
// // It could also be a function parameter that has an array literal as default value
// if (node.parent && (node.parent.kind === ts.SyntaxKind.VariableDeclaration || node.parent.kind === ts.SyntaxKind.Parameter)) {
// const parent = node.parent as ts.VariableDeclaration | ts.ParameterDeclaration;
// if (!parent.type) {
// if (ctx.options.ignorePrefix &&
// parent.name.getText(ctx.sourceFile).substr(0, ctx.options.ignorePrefix.length) === ctx.options.ignorePrefix) {
// return false;
// }
// return true;
// }
// }
// return false;
// }
23 changes: 0 additions & 23 deletions src/readonlyArrayRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,30 +67,7 @@ function checkFunctionNode(node: ts.FunctionDeclaration | ts.ArrowFunction, ctx:
if (node.type) {
checkArrayTypeReference(node.type, ctx);
}
else if (node.body) {
// No explicit return type, check the body for return statements
checkFunctionBody(node.body, ctx);
}

}

function checkFunctionBody(node: ts.Node, ctx: Lint.WalkContext<Options>): void {
return ts.forEachChild(node, cb);
function cb(node: ts.Node): void {
// Check for invalid return type
checkReturnStatement(node, ctx);
// Use return becuase performance hints docs say it optimizes the function using tail-call recursion
return ts.forEachChild(node, cb);
}
}

function checkReturnStatement(node: ts.Node, ctx: Lint.WalkContext<Options>) {
if (node.kind === ts.SyntaxKind.ReturnStatement) {
const returnNode = node as ts.ReturnStatement;
if (returnNode.expression && returnNode.expression.kind === ts.SyntaxKind.ArrayLiteralExpression) {
ctx.addFailureAtNode(returnNode, Rule.FAILURE_STRING);
}
}
}

function checkArrayTypeReference(node: ts.Node, ctx: Lint.WalkContext<Options>) {
Expand Down
16 changes: 0 additions & 16 deletions test/rules/readonly-array/default/interface.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,6 @@ const foo = (): Array<string> => {

}

function foo() {

// Should fail on implicit Array return type
return [1, 2, 3];
~~~~~~~~~~~~~~~~~ [failure]

}

const foo = () => {

// Should fail on implicit Array return type
return [1, 2, 3];
~~~~~~~~~~~~~~~~~ [failure]

}

function foo(): ReadonlyArray<number> {

// Should not fail on explicit ReadonlyArray return type
Expand Down
4 changes: 4 additions & 0 deletions test/rules/readonly-array/default/variable.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const foo: Array<string> = [];
const foo = [1, 2, 3]
~~~ [failure]

// Should fail on Array type being used as template param
let x: Foo<Array<string>>;
~~~~~~~~~~~~~ [failure]

// -- Local variable declarations

// Should fail on Array type in variable declaration as function parameter (TypeReferenceNode)
Expand Down
16 changes: 0 additions & 16 deletions test/rules/readonly-array/ignore-local/interface.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,6 @@ const foo = (): Array<string> => {

}

function foo() {

// Should fail on implicit Array return type
return [1, 2, 3];
~~~~~~~~~~~~~~~~~ [failure]

}

const foo = () => {

// Should fail on implicit Array return type
return [1, 2, 3];
~~~~~~~~~~~~~~~~~ [failure]

}

function foo(): ReadonlyArray<number> {

// Should not fail on explicit ReadonlyArray return type
Expand Down
7 changes: 3 additions & 4 deletions test/rules/readonly-array/work/variable.ts.lint
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
function foo() {
return [1, 2, 3];
~~~~~~~~~~~~~~~~~ [failure]
}
let x: Foo<Array<string>>;
~~~~~~~~~~~~~ [failure]


[failure]: Only ReadonlyArray allowed.

0 comments on commit a4a5cdf

Please sign in to comment.