Skip to content

Commit

Permalink
no-for-loop: Respect declaration kind (#878)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fisker
fisker committed Oct 20, 2020
1 parent d98d867 commit df3f7bd
Show file tree
Hide file tree
Showing 4 changed files with 221 additions and 15 deletions.
36 changes: 26 additions & 10 deletions rules/no-for-loop.js
View file
Expand Up @@ -361,21 +361,37 @@ const create = context => {
let declarationElement = element;
let declarationType = 'const';
let removeDeclaration = true;
if (
elementNode &&
(elementNode.id.type === 'ObjectPattern' || elementNode.id.type === 'ArrayPattern')
) {
removeDeclaration = arrayReferences.length === 1;
let typeAnnotation;

if (elementNode) {
if (elementNode.id.type === 'ObjectPattern' || elementNode.id.type === 'ArrayPattern') {
removeDeclaration = arrayReferences.length === 1;
}

if (removeDeclaration) {
declarationType = elementNode.parent.kind;
declarationElement = sourceCode.getText(elementNode.id);
declarationType = element.type === 'VariableDeclarator' ? elementNode.kind : elementNode.parent.kind;
if (elementNode.id.typeAnnotation && shouldGenerateIndex) {
declarationElement = sourceCode.text.slice(elementNode.id.range[0], elementNode.id.typeAnnotation.range[0]);
typeAnnotation = sourceCode.getText(elementNode.id.typeAnnotation, -1).trim();
} else {
declarationElement = sourceCode.getText(elementNode.id);
}
}
}

const replacement = shouldGenerateIndex ?
`${declarationType} [${index}, ${declarationElement}] of ${array}.entries()` :
`${declarationType} ${declarationElement} of ${array}`;
const parts = [declarationType];
if (shouldGenerateIndex) {
parts.push(` [${index}, ${declarationElement}]`);
if (typeAnnotation) {
parts.push(`: [number, ${typeAnnotation}]`);
}

parts.push(` of ${array}.entries()`);
} else {
parts.push(` ${declarationElement} of ${array}`);
}

const replacement = parts.join('');

yield fixer.replaceTextRange([
node.init.range[0],
Expand Down
72 changes: 67 additions & 5 deletions test/no-for-loop.js
View file
Expand Up @@ -203,7 +203,7 @@ ruleTester.run('no-for-loop', rule, {
console.log(i, el);
}
`, outdent`
for (const [i, el] of arr.entries()) {
for (let [i, el] of arr.entries()) {
console.log(i, el);
}
`),
Expand Down Expand Up @@ -247,7 +247,7 @@ ruleTester.run('no-for-loop', rule, {
var x = xs[j];console.log(j, x);
}
`, outdent`
for (const [j, x] of xs.entries()) {
for (var [j, x] of xs.entries()) {
console.log(j, x);
}
`),
Expand Down Expand Up @@ -277,7 +277,7 @@ ruleTester.run('no-for-loop', rule, {
console.log(j, x, y);
}
`, outdent`
for (const [j, x] of xs.entries()) {
for (var [j, x] of xs.entries()) {
var y = ys[j];
console.log(j, x, y);
}
Expand All @@ -289,7 +289,7 @@ ruleTester.run('no-for-loop', rule, {
console.log(j, x, y);
}
`, outdent`
for (const [j, x] of xs.entries()) {
for (var [j, x] of xs.entries()) {
var y = ys[j];
console.log(j, x, y);
}
Expand All @@ -301,7 +301,7 @@ ruleTester.run('no-for-loop', rule, {
console.log(j, x, y);
}
`, outdent`
for (const [j, x] of xs.entries()) {
for (var [j, x] of xs.entries()) {
var y = ys[j], i = 10;
console.log(j, x, y);
}
Expand Down Expand Up @@ -725,10 +725,72 @@ ruleTesterEs5.run('no-for-loop', rule, {
invalid: []
});

runTest.typescript({
valid: [],
invalid: [
{
// https://github.com/microsoft/vscode/blob/cf9ac85214c3f1d3d0b80cc503ff7498f2b3ea2f/src/vs/workbench/api/common/extHostLanguageFeatures.ts#L1207
code: outdent`
for (let i = 0; i < positions.length; i++) {
let last: vscode.Position | vscode.Range = positions[i];
let selectionRange = allProviderRanges[i];
}
`,
output: outdent`
for (let [i, last]: [number, vscode.Position | vscode.Range] of positions.entries()) {
let selectionRange = allProviderRanges[i];
}
`,
errors: 1
},
{
code: outdent`
for (let i = 0; i < positions.length; i++) {
let last: vscode.Position | vscode.Range = positions[i];
}
`,
output: outdent`
for (let last: vscode.Position | vscode.Range of positions) {
}
`,
errors: 1
}
]
});

runTest.visualize([
outdent`
for (let i = 0; i < arr.length; i += 1) {
console.log(arr[i])
}
`,
// #742
outdent`
for (let i = 0; i < plugins.length; i++) {
let plugin = plugins[i];
plugin = calculateSomeNewValue();
// ...
}
`,
outdent`
for (let i = 0; i < array.length; i++) {
var foo = array[i];
foo = bar();
}
`,
outdent`
for (let i = 0; i < array.length; i++) {
let foo = array[i];
}
`,
outdent`
for (let i = 0; i < array.length; i++) {
const foo = array[i];
}
`,
outdent`
for (let i = 0; i < array.length; i++) {
var foo = array[i], bar = 1;
}
`
]);
128 changes: 128 additions & 0 deletions test/snapshots/no-for-loop.js.md
View file
Expand Up @@ -27,3 +27,131 @@ Generated by [AVA](https://avajs.dev).
> 3 | }␊
| ^^ Use a `for-of` loop instead of this `for` loop.␊
`

## no-for-loop - #2

> Snapshot 1
`␊
Input:␊
1 | for (let i = 0; i < plugins.length; i++) {␊
2 | let plugin = plugins[i];␊
3 | plugin = calculateSomeNewValue();␊
4 | // ...␊
5 | }␊
␊
Output:␊
1 | for (let plugin of plugins) {␊
2 | plugin = calculateSomeNewValue();␊
3 | // ...␊
4 | }␊
␊
Error 1/1:␊
> 1 | for (let i = 0; i < plugins.length; i++) {␊
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^␊
> 2 | let plugin = plugins[i];␊
| ^^^^^^^^^^^^^^^^^^^^^^^^^␊
> 3 | plugin = calculateSomeNewValue();␊
| ^^^^^^^^^^^^^^^^^^^^^^^^^␊
> 4 | // ...␊
| ^^^^^^^^^^^^^^^^^^^^^^^^^␊
> 5 | }␊
| ^^ Use a `for-of` loop instead of this `for` loop.␊
`

## no-for-loop - #3

> Snapshot 1
`␊
Input:␊
1 | for (let i = 0; i < array.length; i++) {␊
2 | var foo = array[i];␊
3 | foo = bar();␊
4 | }␊
␊
Output:␊
1 | for (var foo of array) {␊
2 | foo = bar();␊
3 | }␊
␊
Error 1/1:␊
> 1 | for (let i = 0; i < array.length; i++) {␊
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^␊
> 2 | var foo = array[i];␊
| ^^^^^^^^^^^^^^^^^^^^␊
> 3 | foo = bar();␊
| ^^^^^^^^^^^^^^^^^^^^␊
> 4 | }␊
| ^^ Use a `for-of` loop instead of this `for` loop.␊
`

## no-for-loop - #4

> Snapshot 1
`␊
Input:␊
1 | for (let i = 0; i < array.length; i++) {␊
2 | let foo = array[i];␊
3 | }␊
␊
Output:␊
1 | for (let foo of array) {␊
2 | }␊
␊
Error 1/1:␊
> 1 | for (let i = 0; i < array.length; i++) {␊
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^␊
> 2 | let foo = array[i];␊
| ^^^^^^^^^^^^^^^^^^^^␊
> 3 | }␊
| ^^ Use a `for-of` loop instead of this `for` loop.␊
`

## no-for-loop - #5

> Snapshot 1
`␊
Input:␊
1 | for (let i = 0; i < array.length; i++) {␊
2 | const foo = array[i];␊
3 | }␊
␊
Output:␊
1 | for (const foo of array) {␊
2 | }␊
␊
Error 1/1:␊
> 1 | for (let i = 0; i < array.length; i++) {␊
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^␊
> 2 | const foo = array[i];␊
| ^^^^^^^^^^^^^^^^^^^^^^␊
> 3 | }␊
| ^^ Use a `for-of` loop instead of this `for` loop.␊
`

## no-for-loop - #6

> Snapshot 1
`␊
Input:␊
1 | for (let i = 0; i < array.length; i++) {␊
2 | var foo = array[i], bar = 1;␊
3 | }␊
␊
Output:␊
1 | for (var foo of array) {␊
2 | var bar = 1;␊
3 | }␊
␊
Error 1/1:␊
> 1 | for (let i = 0; i < array.length; i++) {␊
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^␊
> 2 | var foo = array[i], bar = 1;␊
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^␊
> 3 | }␊
| ^^ Use a `for-of` loop instead of this `for` loop.␊
`
Binary file modified test/snapshots/no-for-loop.js.snap
View file
Binary file not shown.

0 comments on commit df3f7bd

Please sign in to comment.