Improve prefer-array-literal rule (#862)

* Use only tslint --test for prefer-array-literal rule

* Restrict only global objects with Array (fixes #856)

* Add allow-single-argument option and check argument types (fixes #822)

* Update docs and metadata for prefer-array-literal rule

* Apply suggestions related to wording in README.md

Co-Authored-By: Josh Goldberg <joshuakgoldberg@outlook.com>

* Change new option name to allow-size-argument

* Use Set for restricted namespaces. Add `self` to restrictions

* Fix error message for incorrect size argument

* Handle array spread in argument position

* Drop node text from all error messages

* Handle potential exceptions from TS

* Add comment about additional check for SpreadElement
This commit is contained in:
Andrii Dieiev 2019-05-18 02:47:02 +03:00 коммит произвёл GitHub
Родитель 7ac0019a53
Коммит f85d5f6dbc
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
12 изменённых файлов: 407 добавлений и 138 удалений

Просмотреть файл

@ -828,15 +828,19 @@ We recommend you specify exact versions of lint libraries, including `tslint-mic
<td>
Use array literal syntax when declaring or instantiating array types.
For example, prefer the Javascript form of <code>string[]</code> to the TypeScript form <code>Array&lt;string&gt;</code>.
Prefer <code>[]' to <code>new Array()</code>.
Prefer <code>[4, 5]' to <code>new Array(4, 5)</code>.
Prefer <code>[undefined, undefined]' to <code>new Array(2)</code>.
Since 2.0.10, this rule can be configured to allow Array type parameters.
To ignore type parameters, configure the rule with the values: <code>[ true, { 'allow-type-parameters': true } ]</code>.
Prefer <code>[]</code> over <code>new Array()</code>.
Prefer <code>[4, 5]</code> over <code>new Array(4, 5)</code>.
Prefer <code>[undefined, undefined]</code> over <code>new Array(2)</code>.
<br />
Since 2.0.10, this rule can be configured to allow <code>Array</code> type parameters.
To ignore type parameters, configure the rule with the values: <code>[true, {"allow-type-parameters": true}]</code>.
<br />
Since @next, you can lift restriction on <code>Array</code> constructor calls with a single argument (to create empty array of a given length). If type information is available - rule will allow only a single argument of <code>number</code> type.
To allow empty array creation, configure the rule with the values: <code>[true, {"allow-size-argument": true}]</code>.
<br />
This rule has some overlap with the <a href="https://palantir.github.io/tslint/rules/array-type">TSLint array-type rule</a>; however, the version here catches more instances.
</td>
<td>1.0, 2.0.10</td>
<td>1.0, 2.0.10, @next</td>
</tr>
<tr>
<td>

Просмотреть файл

@ -6,16 +6,44 @@ import { AstUtils } from './utils/AstUtils';
import { ExtendedMetadata } from './utils/ExtendedMetadata';
import { isObject } from './utils/TypeGuard';
// undefined for case when function/constructor is called directly without namespace
const RESTRICTED_NAMESPACES: Set<string | undefined> = new Set([undefined, 'window', 'self', 'global', 'globalThis']);
function inRestrictedNamespace(node: ts.NewExpression | ts.CallExpression): boolean {
const functionTarget = AstUtils.getFunctionTarget(node);
return RESTRICTED_NAMESPACES.has(functionTarget);
}
type InvocationType = 'constructor' | 'function';
interface Options {
allowSizeArgument: boolean;
allowTypeParameters: boolean;
}
export class Rule extends Lint.Rules.AbstractRule {
export class Rule extends Lint.Rules.OptionallyTypedRule {
public static metadata: ExtendedMetadata = {
ruleName: 'prefer-array-literal',
type: 'maintainability',
description: 'Use array literal syntax when declaring or instantiating array types.',
options: null, // tslint:disable-line:no-null-keyword
optionsDescription: '',
options: {
type: 'object',
properties: {
'allow-size-argument': {
type: 'boolean'
},
'allow-type-parameters': {
type: 'boolean'
}
},
additionalProperties: false
},
optionsDescription: Lint.Utils.dedent`
Rule accepts object with next boolean options:
- "allow-size-argument" - allows calls to Array constructor with a single element (to create empty array of a given length).
- "allow-type-parameters" - allow Array type parameters.
`,
typescriptOnly: true,
issueClass: 'Non-SDL',
issueType: 'Warning',
@ -25,16 +53,26 @@ export class Rule extends Lint.Rules.AbstractRule {
commonWeaknessEnumeration: '398, 710'
};
public static GENERICS_FAILURE_STRING: string = 'Replace generic-typed Array with array literal: ';
public static CONSTRUCTOR_FAILURE_STRING: string = 'Replace Array constructor with an array literal: ';
public static FUNCTION_FAILURE_STRING: string = 'Replace Array function with an array literal: ';
public static GENERICS_FAILURE_STRING: string = 'Replace generic-typed Array with array literal.';
public static getReplaceFailureString = (type: InvocationType) => `Replace Array ${type} with an array literal.`;
public static getSizeParamFailureString = (type: InvocationType) =>
`To create an array of a given length you should use non-negative integer. Otherwise replace Array ${type} with an array literal.`;
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk, this.parseOptions(this.getOptions()));
return this.applyWithProgram(sourceFile, /* program */ undefined);
}
public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program | undefined): Lint.RuleFailure[] {
return this.applyWithFunction(
sourceFile,
walk,
this.parseOptions(this.getOptions()),
program ? program.getTypeChecker() : undefined
);
}
private parseOptions(options: Lint.IOptions): Options {
let value: boolean = false;
let allowSizeArgument: boolean = false;
let allowTypeParameters: boolean = false;
let ruleOptions: any[] = [];
if (options.ruleArguments instanceof Array) {
@ -47,43 +85,67 @@ export class Rule extends Lint.Rules.AbstractRule {
ruleOptions.forEach((opt: unknown) => {
if (isObject(opt)) {
value = opt['allow-type-parameters'] === true;
allowSizeArgument = opt['allow-size-argument'] === true;
allowTypeParameters = opt['allow-type-parameters'] === true;
}
});
return {
allowTypeParameters: value
allowSizeArgument,
allowTypeParameters
};
}
}
function walk(ctx: Lint.WalkContext<Options>) {
const { allowTypeParameters } = ctx.options;
function walk(ctx: Lint.WalkContext<Options>, checker: ts.TypeChecker | undefined) {
const { allowTypeParameters, allowSizeArgument } = ctx.options;
function checkExpression(type: InvocationType, node: ts.CallExpression | ts.NewExpression): void {
const functionName = AstUtils.getFunctionName(node);
if (functionName === 'Array' && inRestrictedNamespace(node)) {
const callArguments = node.arguments;
if (!allowSizeArgument || !callArguments || callArguments.length !== 1) {
const failureString = Rule.getReplaceFailureString(type);
ctx.addFailureAt(node.getStart(), node.getWidth(), failureString);
} else {
// When typechecker is not available - allow any call with single expression
if (checker) {
try {
// TS might throw exceptions in non-standard conditions (like .vue files)
// Use try...catch blocks to fallback to the same behavior as when checker is not available
// See https://github.com/microsoft/tslint-microsoft-contrib/issues/859
const argument = callArguments[0];
const argumentType = checker.getTypeAtLocation(argument);
// Looks like TS returns type or array elements when SpreadElement is passed to getTypeAtLocation.
// Additional check for SpreadElement is required to catch case when array has type number[] and its element will pass assignability check.
// SpreadElement shouldn't be allowed because result of `Array(...arr)` call will dependepend on array lenght and might produce unexpected results.
if (!tsutils.isTypeAssignableToNumber(checker, argumentType) || argument.kind === ts.SyntaxKind.SpreadElement) {
const failureString = Rule.getSizeParamFailureString(type);
ctx.addFailureAt(node.getStart(), node.getWidth(), failureString);
}
} catch {
// No error to use same behavior as when typeChecker is not available
}
}
}
}
}
function cb(node: ts.Node): void {
if (tsutils.isTypeReferenceNode(node)) {
if (!allowTypeParameters) {
if ((<ts.Identifier>node.typeName).text === 'Array') {
const failureString = Rule.GENERICS_FAILURE_STRING + node.getText();
const failureString = Rule.GENERICS_FAILURE_STRING;
ctx.addFailureAt(node.getStart(), node.getWidth(), failureString);
}
}
}
if (tsutils.isNewExpression(node)) {
const functionName = AstUtils.getFunctionName(node);
if (functionName === 'Array') {
const failureString = Rule.CONSTRUCTOR_FAILURE_STRING + node.getText();
ctx.addFailureAt(node.getStart(), node.getWidth(), failureString);
}
checkExpression('constructor', node);
}
if (tsutils.isCallExpression(node)) {
const expr = node.expression;
if (expr.getText() === 'Array') {
const failureString = Rule.FUNCTION_FAILURE_STRING + node.getText();
ctx.addFailureAt(node.getStart(), node.getWidth(), failureString);
}
checkExpression('function', node);
}
return ts.forEachChild(node, cb);

Просмотреть файл

@ -1,93 +0,0 @@
import { Utils } from '../utils/Utils';
import { TestHelper } from './TestHelper';
describe('preferArrayLiteralRule', (): void => {
const ruleName: string = 'prefer-array-literal';
it('should allow string[] as variable type', (): void => {
const inputScript: string = 'var x : string[];';
TestHelper.assertViolations(ruleName, inputScript, []);
});
it('should allow Array type parameters when options say to ignore type params', (): void => {
const inputScript: string = `
let myArray: Array<string> = [];
interface MyInterface {
myArray: Array<string>;
} `;
TestHelper.assertViolationsWithOptions(ruleName, [true, { 'allow-type-parameters': true }], inputScript, []);
});
it('should ban Array<string> as variable type', (): void => {
const inputScript: string = 'var x : Array<string>;';
TestHelper.assertViolations(ruleName, inputScript, [
{
failure: 'Replace generic-typed Array with array literal: Array<string>',
name: Utils.absolutePath('file.ts'),
ruleName: 'prefer-array-literal',
startPosition: { character: 9, line: 1 }
}
]);
});
it('should ban Array<string> as parameter type', (): void => {
const inputScript: string = 'function (parm: Array<number>) {}';
TestHelper.assertViolations(ruleName, inputScript, [
{
failure: 'Replace generic-typed Array with array literal: Array<number>',
name: Utils.absolutePath('file.ts'),
ruleName: 'prefer-array-literal',
startPosition: { character: 17, line: 1 }
}
]);
});
it('should ban new Array() constructor', (): void => {
const inputScript: string = 'new Array()';
TestHelper.assertViolationsWithOptions(ruleName, [true, { 'allow-type-parameters': true }], inputScript, [
{
failure: 'Replace Array constructor with an array literal: new Array()',
name: Utils.absolutePath('file.ts'),
ruleName: 'prefer-array-literal',
startPosition: { character: 1, line: 1 }
}
]);
});
it('should ban new Array(4, 5) constructor', (): void => {
const inputScript: string = 'new Array(4, 5)';
TestHelper.assertViolations(ruleName, inputScript, [
{
failure: 'Replace Array constructor with an array literal: new Array(4, 5)',
name: Utils.absolutePath('file.ts'),
ruleName: 'prefer-array-literal',
startPosition: { character: 1, line: 1 }
}
]);
});
it('should ban new Array(4) constructor', (): void => {
const inputScript: string = 'new Array(4)';
TestHelper.assertViolations(ruleName, inputScript, [
{
failure: 'Replace Array constructor with an array literal: new Array(4)',
name: Utils.absolutePath('file.ts'),
ruleName: 'prefer-array-literal',
startPosition: { character: 1, line: 1 }
}
]);
});
it('should ban Array function', (): void => {
const inputScript: string = 'Array(2)';
TestHelper.assertViolations(ruleName, inputScript, [
{
failure: 'Replace Array function with an array literal: Array(2)',
name: Utils.absolutePath('file.ts'),
ruleName: 'prefer-array-literal',
startPosition: { character: 1, line: 1 }
}
]);
});
});

Просмотреть файл

@ -25,7 +25,7 @@ export namespace AstUtils {
return '';
}
export function getFunctionTarget(expression: ts.CallExpression): string | undefined {
export function getFunctionTarget(expression: ts.CallExpression | ts.NewExpression): string | undefined {
if (expression.expression.kind === ts.SyntaxKind.PropertyAccessExpression) {
const propExp: ts.PropertyAccessExpression = <ts.PropertyAccessExpression>expression.expression;
return propExp.expression.getText();

Просмотреть файл

@ -0,0 +1,82 @@
let a: string[];
let b: Array<string> = [];
~~~~~~~~~~~~~ [type]
interface C {
myArray: Array<string>;
~~~~~~~~~~~~~ [type]
}
var d: Array<string>;
~~~~~~~~~~~~~ [type]
function e(param: Array<number>) { }
~~~~~~~~~~~~~ [type]
var f = new Array();
~~~~~~~~~~~ [constructor]
var g = new Array(4, 5);
~~~~~~~~~~~~~~~ [constructor]
var h = new Array(4);
var i = Array(2);
var j = new Array;
~~~~~~~~~ [constructor]
// calls to Array function/constructor on global objects is forbidden
var nc1 = window.Array(1);
var nc2 = global.Array(1, 2);
~~~~~~~~~~~~~~~~~~ [function]
var nc3 = globalThis.Array('3');
~~~~~~~~~~~~~~~~~~~~~ [size-argument-function]
var nn1 = new window.Array(1);
var nn2 = new global.Array(1, 2);
~~~~~~~~~~~~~~~~~~~~~~ [constructor]
var nn3 = new globalThis.Array('3');
~~~~~~~~~~~~~~~~~~~~~~~~~ [size-argument-constructor]
// calls to Array function/constructor from namespaces are valid
import { Types } from 'mongoose';
export const foo: Types.Array<number> = new Types.Array();
declare var num: number;
declare var str: string;
declare var unionA: number | Array<number>;
~~~~~~~~~~~~~ [type]
declare var unionF: number | (() => number);
const t1 = Array(num);
const t2 = Array(str);
~~~~~~~~~~ [size-argument-function]
const t3 = Array(unionA);
~~~~~~~~~~~~~ [size-argument-function]
const t3 = Array(unionF);
~~~~~~~~~~~~~ [size-argument-function]
const t4 = Array(num + 1);
const t5 = Array(str + 1);
~~~~~~~~~~~~~~ [size-argument-function]
const t6 = Array(10 + 1);
const t7 = Array(10 + '1');
~~~~~~~~~~~~~~~ [size-argument-function]
const t8 = Array(1.5); // no error - limitation of typed rule
const t9 = Array(-1); // no error - limitation of typed rule
const t10 = Array(-num); // no error - limitation of typed rule
declare var arrS: number[];
declare var arrN: number[];
const t11 = Array(...arrS);
~~~~~~~~~~~~~~ [size-argument-function]
const t12 = Array(...arrN);
~~~~~~~~~~~~~~ [size-argument-function]
[type]: Replace generic-typed Array with array literal.
[constructor]: Replace Array constructor with an array literal.
[function]: Replace Array function with an array literal.
[size-argument-constructor]: To create an array of a given length you should use non-negative integer. Otherwise replace Array constructor with an array literal.
[size-argument-function]: To create an array of a given length you should use non-negative integer. Otherwise replace Array function with an array literal.

Просмотреть файл

@ -0,0 +1,5 @@
{
"compilerOptions": {
"target": "es5"
}
}

Просмотреть файл

@ -0,0 +1,5 @@
{
"rules": {
"prefer-array-literal": [true, {"allow-size-argument": true}]
}
}

Просмотреть файл

@ -0,0 +1,73 @@
let a: string[];
let b: Array<string> = [];
~~~~~~~~~~~~~ [type]
interface C {
myArray: Array<string>;
~~~~~~~~~~~~~ [type]
}
var d: Array<string>;
~~~~~~~~~~~~~ [type]
function e(param: Array<number>) { }
~~~~~~~~~~~~~ [type]
var f = new Array();
~~~~~~~~~~~ [constructor]
var g = new Array(4, 5);
~~~~~~~~~~~~~~~ [constructor]
var h = new Array(4);
var i = Array(2);
var j = new Array;
~~~~~~~~~ [constructor]
// calls to Array function/constructor on global objects is forbidden
var nc1 = window.Array(1);
var nc2 = global.Array(1, 2);
~~~~~~~~~~~~~~~~~~ [function]
var nc3 = globalThis.Array('3'); // no error - limitation of untyped rule
var nn1 = new window.Array(1);
var nn2 = new global.Array(1, 2);
~~~~~~~~~~~~~~~~~~~~~~ [constructor]
var nn3 = new globalThis.Array('3'); // no error - limitation of untyped rule
// calls to Array function/constructor from namespaces are valid
import { Types } from 'mongoose';
export const foo: Types.Array<number> = new Types.Array();
declare var num: number;
declare var str: string;
declare var unionA: number | Array<number>;
~~~~~~~~~~~~~ [type]
declare var unionF: number | (() => number);
// no errors below - limitation of untyped rule
const t1 = Array(num);
const t2 = Array(str);
const t3 = Array(unionA);
const t3 = Array(unionF);
const t4 = Array(num + 1);
const t5 = Array(str + 1);
const t6 = Array(10 + 1);
const t7 = Array(10 + '1');
const t8 = Array(1.5);
const t9 = Array(-1);
const t10 = Array(-num);
declare var arrS: number[];
declare var arrN: number[];
// no errors to match behavior of a regular single argument
const t11 = Array(...arrS);
const t12 = Array(...arrN);
[type]: Replace generic-typed Array with array literal.
[constructor]: Replace Array constructor with an array literal.
[function]: Replace Array function with an array literal.

Просмотреть файл

@ -0,0 +1,5 @@
{
"rules": {
"prefer-array-literal": [true, {"allow-size-argument": true}]
}
}

Просмотреть файл

@ -1,24 +1,84 @@
let a: string[];
let b: Array<string> = [];
~~~~~~~~~~~~~ [Replace generic-typed Array with array literal: Array<string>]
interface C {
myArray: Array<string>;
~~~~~~~~~~~~~ [Replace generic-typed Array with array literal: Array<string>]
}
var d: Array<string>;
~~~~~~~~~~~~~ [Replace generic-typed Array with array literal: Array<string>]
function e(param: Array<number>) { }
~~~~~~~~~~~~~ [Replace generic-typed Array with array literal: Array<number>]
var f = new Array();
~~~~~~~~~~~ [Replace Array constructor with an array literal: new Array()]
~~~~~~~~~~~ [constructor]
var g = new Array(4, 5);
~~~~~~~~~~~~~~~ [Replace Array constructor with an array literal: new Array(4, 5)]
~~~~~~~~~~~~~~~ [constructor]
var h = new Array(4);
~~~~~~~~~~~~ [Replace Array constructor with an array literal: new Array(4)]
~~~~~~~~~~~~ [constructor]
var i = Array(2);
~~~~~~~~ [function]
var j = new Array;
~~~~~~~~~ [constructor]
// calls to Array function/constructor on global objects is forbidden
var nc1 = window.Array(1);
~~~~~~~~~~~~~~~ [function]
var nc2 = global.Array(1, 2);
~~~~~~~~~~~~~~~~~~ [function]
var nc3 = globalThis.Array('3');
~~~~~~~~~~~~~~~~~~~~~ [function]
var nn1 = new window.Array(1);
~~~~~~~~~~~~~~~~~~~ [constructor]
var nn2 = new global.Array(1, 2);
~~~~~~~~~~~~~~~~~~~~~~ [constructor]
var nn3 = new globalThis.Array('3');
~~~~~~~~~~~~~~~~~~~~~~~~~ [constructor]
// calls to Array function/constructor from namespaces are valid
import { Types } from 'mongoose';
export const foo: Types.Array<number> = new Types.Array();
declare var num: number;
declare var str: string;
declare var unionA: number | Array<number>;
declare var unionF: number | (() => number);
const t1 = Array(num);
~~~~~~~~~~ [function]
const t2 = Array(str);
~~~~~~~~~~ [function]
const t3 = Array(unionA);
~~~~~~~~~~~~~ [function]
const t3 = Array(unionF);
~~~~~~~~~~~~~ [function]
const t4 = Array(num + 1);
~~~~~~~~~~~~~~ [function]
const t5 = Array(str + 1);
~~~~~~~~~~~~~~ [function]
const t6 = Array(10 + 1);
~~~~~~~~~~~~~ [function]
const t7 = Array(10 + '1');
~~~~~~~~~~~~~~~ [function]
const t8 = Array(1.5);
~~~~~~~~~~ [function]
const t9 = Array(-1);
~~~~~~~~~ [function]
const t10 = Array(-num);
~~~~~~~~~~~ [function]
declare var arrS: number[];
declare var arrN: number[];
const t11 = Array(...arrS);
~~~~~~~~~~~~~~ [function]
const t12 = Array(...arrN);
~~~~~~~~~~~~~~ [function]
[constructor]: Replace Array constructor with an array literal.
[function]: Replace Array function with an array literal.

Просмотреть файл

@ -1,5 +1,5 @@
{
"rules": {
"prefer-array-literal": [true, "allow-type-parameters"]
"prefer-array-literal": [true, {"allow-type-parameters": true}]
}
}

Просмотреть файл

@ -1,24 +1,90 @@
let a: string[];
let b: Array<string> = [];
~~~~~~~~~~~~~ [Replace generic-typed Array with array literal: Array<string>]
~~~~~~~~~~~~~ [type]
interface C {
myArray: Array<string>;
~~~~~~~~~~~~~ [Replace generic-typed Array with array literal: Array<string>]
~~~~~~~~~~~~~ [type]
}
var d: Array<string>;
~~~~~~~~~~~~~ [Replace generic-typed Array with array literal: Array<string>]
~~~~~~~~~~~~~ [type]
function e(param: Array<number>) { }
~~~~~~~~~~~~~ [Replace generic-typed Array with array literal: Array<number>]
~~~~~~~~~~~~~ [type]
var f = new Array();
~~~~~~~~~~~ [Replace Array constructor with an array literal: new Array()]
~~~~~~~~~~~ [constructor]
var g = new Array(4, 5);
~~~~~~~~~~~~~~~ [Replace Array constructor with an array literal: new Array(4, 5)]
~~~~~~~~~~~~~~~ [constructor]
var h = new Array(4);
~~~~~~~~~~~~ [Replace Array constructor with an array literal: new Array(4)]
~~~~~~~~~~~~ [constructor]
var i = Array(2);
~~~~~~~~ [function]
var j = new Array;
~~~~~~~~~ [constructor]
// calls to Array function/constructor on global objects is forbidden
var nc1 = window.Array(1);
~~~~~~~~~~~~~~~ [function]
var nc2 = global.Array(1, 2);
~~~~~~~~~~~~~~~~~~ [function]
var nc3 = globalThis.Array('3');
~~~~~~~~~~~~~~~~~~~~~ [function]
var nn1 = new window.Array(1);
~~~~~~~~~~~~~~~~~~~ [constructor]
var nn2 = new global.Array(1, 2);
~~~~~~~~~~~~~~~~~~~~~~ [constructor]
var nn3 = new globalThis.Array('3');
~~~~~~~~~~~~~~~~~~~~~~~~~ [constructor]
// calls to Array function/constructor from namespaces are valid
import { Types } from 'mongoose';
export const foo: Types.Array<number> = new Types.Array();
declare var num: number;
declare var str: string;
declare var unionA: number | Array<number>;
~~~~~~~~~~~~~ [type]
declare var unionF: number | (() => number);
const t1 = Array(num);
~~~~~~~~~~ [function]
const t2 = Array(str);
~~~~~~~~~~ [function]
const t3 = Array(unionA);
~~~~~~~~~~~~~ [function]
const t3 = Array(unionF);
~~~~~~~~~~~~~ [function]
const t4 = Array(num + 1);
~~~~~~~~~~~~~~ [function]
const t5 = Array(str + 1);
~~~~~~~~~~~~~~ [function]
const t6 = Array(10 + 1);
~~~~~~~~~~~~~ [function]
const t7 = Array(10 + '1');
~~~~~~~~~~~~~~~ [function]
const t8 = Array(1.5);
~~~~~~~~~~ [function]
const t9 = Array(-1);
~~~~~~~~~ [function]
const t10 = Array(-num);
~~~~~~~~~~~ [function]
declare var arrS: number[];
declare var arrN: number[];
const t11 = Array(...arrS);
~~~~~~~~~~~~~~ [function]
const t12 = Array(...arrN);
~~~~~~~~~~~~~~ [function]
[type]: Replace generic-typed Array with array literal.
[constructor]: Replace Array constructor with an array literal.
[function]: Replace Array function with an array literal.