Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Fix regression where readonly-array is reporting error at assignment site #105

Merged
merged 5 commits into from Dec 14, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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.