Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
WIP: Fix regression where readonly-array is reporting error at assign…
…ment site (#105)

* Add test

* Fixed
  • Loading branch information
jonaskello committed Dec 14, 2018
1 parent d3ab2cf commit 636a6b2
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 37 deletions.
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -39,7 +39,7 @@
"build": "rimraf rules && yarn compile",
"lint": "tslint './src/**/*.ts{,x}'",
"test": "tslint --test test/rules/**/*",
"test:work": "yarn build && tslint --test test/rules/readonly-array/*",
"test:work": "yarn build && tslint --test test/rules/readonly-array/work",
"verify": "yarn build && yarn lint && yarn coverage",
"coverage": "rimraf coverage .nyc_output && nyc yarn test",
"report-coverage": "codecov -f coverage/*.json",
Expand Down
45 changes: 17 additions & 28 deletions src/readonlyArrayRule.ts
Expand Up @@ -10,7 +10,7 @@ import {
} from "./shared/check-node";
import {
isFunctionLikeDeclaration,
isVariableLikeDeclaration
isVariableOrParameterOrPropertyDeclaration
} from "./shared/typeguard";

type Options = Ignore.IgnoreLocalOption &
Expand All @@ -32,7 +32,7 @@ function checkNode(
invalidNodes: [
...checkArrayType(node, ctx),
...checkTypeReference(node, ctx),
...checkVariableLikeImplicitType(node, ctx)
...checkImplicitType(node, ctx)
]
};
}
Expand Down Expand Up @@ -106,35 +106,22 @@ function checkTypeReference(
return [];
}

function checkVariableLikeImplicitType(
export function checkImplicitType(
node: ts.Node,
ctx: Lint.WalkContext<Options>
): ReadonlyArray<InvalidNode> {
// The initializer is used to set and implicit type
if (Ignore.shouldIgnorePrefix(node, ctx.options, ctx.sourceFile)) {
return [];
}
// Check if the initializer is used to set an implicit array type
if (
isVariableLikeDeclaration(node) &&
isVariableOrParameterOrPropertyDeclaration(node) &&
isUntypedAndHasArrayLiteralExpressionInitializer(node)
) {
const length = node.name.getWidth(ctx.sourceFile);
const nameText = node.name.getText(ctx.sourceFile);
let typeArgument = "any";
// Not sure it is a good idea to guess what the element types are...
// if (node.initializer.elements.length > 0) {
// const element = node.initializer.elements[0];
// if (utils.isNumericLiteral(element)) {
// typeArgument = "number";
// } else if (utils.isStringLiteral(element)) {
// typeArgument = "string";
// } else if (
// element.kind === ts.SyntaxKind.TrueKeyword ||
// element.kind === ts.SyntaxKind.FalseKeyword
// ) {
// typeArgument = "boolean";
// }
// }

return [
createInvalidNode(node.name, [
new Lint.Replacement(
Expand All @@ -157,15 +144,17 @@ function checkIsReturnType(node: ts.Node): boolean {
}

function isUntypedAndHasArrayLiteralExpressionInitializer(
node: ts.VariableLikeDeclaration
): node is ts.VariableLikeDeclaration & {
initializer: ts.ArrayLiteralExpression;
} {
// tslint:disable:no-any
node:
| ts.VariableDeclaration
| ts.ParameterDeclaration
| ts.PropertyDeclaration
): node is
| ts.VariableDeclaration
| ts.ParameterDeclaration & {
initializer: ts.ArrayLiteralExpression;
} {
return Boolean(
!(node as any).type &&
(node as any).initializer &&
utils.isArrayLiteralExpression((node as any).initializer)
!node.type &&
(node.initializer && utils.isArrayLiteralExpression(node.initializer))
);
// tslint:enable:no-any
}
13 changes: 13 additions & 0 deletions src/shared/typeguard.ts
Expand Up @@ -44,3 +44,16 @@ export function isVariableLikeDeclaration(
utils.isVariableDeclaration(node)
);
}

export function isVariableOrParameterOrPropertyDeclaration(
node: ts.Node
): node is
| ts.VariableDeclaration
| ts.ParameterDeclaration
| ts.PropertyDeclaration {
return (
utils.isVariableDeclaration(node) ||
utils.isParameterDeclaration(node) ||
utils.isPropertyDeclaration(node)
);
}
8 changes: 8 additions & 0 deletions test/rules/readonly-array/default/interface.ts.fix
Expand Up @@ -55,3 +55,11 @@ const foo = function (): string {
let bar: ReadonlyArray<string>;
};

// Assignment to overridden array
interface SomeType {
array: Array<string>; // tslint:disable-line:readonly-array
}
const o: SomeType = {
array: [""],
}

8 changes: 8 additions & 0 deletions test/rules/readonly-array/default/interface.ts.lint
Expand Up @@ -64,4 +64,12 @@ const foo = function (): string {
~~~~~~~~~~~~~ [failure]
};

// Assignment to overridden array
interface SomeType {
array: Array<string>; // tslint:disable-line:readonly-array
}
const o: SomeType = {
array: [""],
}

[failure]: Only ReadonlyArray allowed.
10 changes: 2 additions & 8 deletions test/rules/readonly-array/work/variable.ts.lint
@@ -1,13 +1,7 @@

class Foo {

foo(bar: Array<string>, zoo: ReadonlyArray<string>, boo = [1, 2, 3]) {
~~~~~~~~~~~~~ [failure]
~~~ [failure]

const foo: Array<string> = [];
}

const foo = [1, 2, 3]
~~~ [failure]
}

[failure]: Only ReadonlyArray allowed.

0 comments on commit 636a6b2

Please sign in to comment.