Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add no-sparse-arrays rule #2407

Merged
merged 3 commits into from Apr 5, 2017

Conversation

andy-hanson
Copy link
Contributor

@andy-hanson andy-hanson commented Mar 25, 2017

PR checklist

Overview of change:

Added the no-sparse-arrays rule, which warns on [1, , 3].

CHANGELOG.md entry:

[new-rule] no-sparse-arrays

@adidahiya
Copy link
Contributor

why not name this the same as eslint and tslint-microsoft-contrib? no-sparse-arrays

for (const element of node.elements) {
if (utils.isOmittedExpression(element)) {
// Node has an empty range, so just use range starting at `element.pos`.
ctx.addFailureAt(element.pos, 1, Rule.FAILURE_STRING);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it's not that easy. You need to exclude array destructuring.
So you basically need test cases like:

[foo, , bar] = [];
[({foo: [, bar]})] = [];
[foo = [, 1]] = []; // should fail

Have a look at the tests for prefer-const, which does almost the same.

@@ -0,0 +1,5 @@
[1, , 3];
~ [Array has a missing element.]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test with trailing comma and one with multiple trailing commas

~ [Array has a missing element.]

// Destructuring allowed.
const [x, , z] = arr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's only destructuring in declarations (ArrayBindingPattern). Destructuring in expression uses ArrayLiteralExpression, see my other comment above.


/** Traverse the LHS of an `=` expression, calling `cb` embedded default value, but ignoring binding patterns. */
function traverseExpressionsInLHS(node: ts.Node, cb: (node: ts.Expression) => void): void {
switch (node.kind) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to add a case for ts.SyntaxKind.ParethesizedExpression


[foo, , bar] = [];
[{foo: [, bar]}] = [];
[foo = [, 1]] = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add another test case like:

[foo = bar([,])] = [];
            ~[0]

I know this works with your current implementation. Just to make sure we won't break it in the future.

@adidahiya adidahiya changed the title Add no-array-literal-hole rule Add no-sparse-arrays rule Mar 28, 2017
@andy-hanson andy-hanson mentioned this pull request Apr 5, 2017
@adidahiya adidahiya merged commit 3e5066f into palantir:master Apr 5, 2017
@andy-hanson andy-hanson deleted the no-array-literal-hole branch April 5, 2017 04:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants