Lint: Allow omitting return type in callback arrow functions (#874)

Author: emillaine

NOTICE

@@ -15,6 +15,7 @@ under the licensing terms detailed in LICENSE:
 * Aaron Turner <[email protected]>
 * Willem Wyndham <[email protected]>
 * Bowen Wang <[email protected]>
+* Emil Laine <[email protected]>
 
 Portions of this software are derived from third-party works licensed under
 the following terms:

README.md

@@ -55,10 +55,10 @@ $> git clone https://github.com/AssemblyScript/assemblyscript.git
 $> cd assemblyscript
 $> npm install
 $> npm link
-$> npm clean
+$> npm run clean
 ```
 
-Note that a fresh clone of the compiler will use the distribution files in `dist/`, but after an `npm clean` it will run [the sources](./src) directly through ts-node, which is useful in development. This condition can also be checked by running `asc -v` (it is running the sources if it states `-dev`). Also please see our [contribution guidelines](./CONTRIBUTING.md) before making your first pull request.
+Note that a fresh clone of the compiler will use the distribution files in `dist/`, but after an `npm run clean` it will run [the sources](./src) directly through ts-node, which is useful in development. This condition can also be checked by running `asc -v` (it is running the sources if it states `-dev`). Also please see our [contribution guidelines](./CONTRIBUTING.md) before making your first pull request.
 
 Building
 --------

lib/lint/formatters/asFormatter.js

@@ -41,10 +41,10 @@ class Formatter extends abstractFormatter_1.AbstractFormatter {
         });
     }
 }
+exports.Formatter = Formatter;
 Formatter.metadata = {
     formatterName: "as",
     description: "AssemblyScript's TSLint formatter.",
     sample: "Similar to ASC's output.",
     consumer: "human",
 };
-exports.Formatter = Formatter;

lib/lint/rules/asTypesRule.js

@@ -7,9 +7,10 @@ class Rule extends Lint.Rules.AbstractRule {
         return this.applyWithWalker(new DiagnosticsWalker(sourceFile, this.getOptions()));
     }
 }
+exports.Rule = Rule;
 Rule.MISSING_TYPE_OR_INITIALIZER = "Missing type or initializer.";
 Rule.MISSING_RETURN_TYPE = "Missing return type.";
-exports.Rule = Rule;
+Rule.UNNECESSARY_RETURN_TYPE = "Unnecessary return type.";
 class DiagnosticsWalker extends Lint.RuleWalker {
     visitVariableDeclaration(node) {
         var list = node.parent;
@@ -39,7 +40,12 @@ class DiagnosticsWalker extends Lint.RuleWalker {
         super.visitFunctionDeclaration(node);
     }
     visitArrowFunction(node) {
-        this.checkFunctionReturnType(node);
+        if (requiresReturnType(node)) {
+            this.checkFunctionReturnType(node);
+        }
+        else if (node.type) {
+            this.addFailureAtNode(node.type, Rule.UNNECESSARY_RETURN_TYPE);
+        }
         super.visitArrowFunction(node);
     }
     visitMethodDeclaration(node) {
@@ -56,3 +62,10 @@ class DiagnosticsWalker extends Lint.RuleWalker {
         }
     }
 }
+function requiresReturnType(node) {
+    if (ts.isCallExpression(node.parent) && ts.isIdentifier(node.parent.expression)
+        && ["lengthof", "nameof"].includes(node.parent.expression.text)) {
+        return true;
+    }
+    return !ts.isCallLikeExpression(node.parent);
+}

lib/lint/rules/asVariablesRule.js

@@ -7,9 +7,9 @@ class Rule extends Lint.Rules.AbstractRule {
         return this.applyWithWalker(new VariablesWalker(sourceFile, this.getOptions()));
     }
 }
+exports.Rule = Rule;
 Rule.TOP_LEVEL_VAR = "Top-level variable should be 'var' (distinct local or global).";
 Rule.BLOCK_LEVEL_LET = "Block-level variable should be 'let' (shared local).";
-exports.Rule = Rule;
 class VariablesWalker extends Lint.RuleWalker {
     visitVariableDeclarationList(node) {
         if (tsutils_1.isVariableStatement(node.parent)) {

lib/lint/rules/internal/asInternalCaseRule.js

@@ -8,8 +8,8 @@ class Rule extends Lint.Rules.AbstractRule {
         return this.applyWithWalker(new CaseWalker(sourceFile, this.getOptions()));
     }
 }
-Rule.NOT_BRACED = "Multi-line case clauses should be braced.";
 exports.Rule = Rule;
+Rule.NOT_BRACED = "Multi-line case clauses should be braced.";
 class CaseWalker extends Lint.RuleWalker {
     visitDefaultClause(node) {
         this.checkDefaultOrCaseClause(node);

lib/lint/rules/internal/asInternalDiagnosticsRule.js

@@ -8,8 +8,8 @@ class Rule extends Lint.Rules.AbstractRule {
         return this.applyWithWalker(new DiagnosticsWalker(sourceFile, this.getOptions()));
     }
 }
-Rule.NOT_ON_SEPARATE_LINE = "Diagnostic message not on a separate line.";
 exports.Rule = Rule;
+Rule.NOT_ON_SEPARATE_LINE = "Diagnostic message not on a separate line.";
 class DiagnosticsWalker extends Lint.RuleWalker {
     visitPropertyAccessExpression(node) {
         if (node.expression.kind === ts.SyntaxKind.Identifier) {

lib/lint/src/rules/asTypesRule.ts

@@ -5,6 +5,7 @@ export class Rule extends Lint.Rules.AbstractRule {
 
   static MISSING_TYPE_OR_INITIALIZER = "Missing type or initializer.";
   static MISSING_RETURN_TYPE = "Missing return type.";
+  static UNNECESSARY_RETURN_TYPE = "Unnecessary return type.";
 
   apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
     return this.applyWithWalker(new DiagnosticsWalker(sourceFile, this.getOptions()));
@@ -46,7 +47,11 @@ class DiagnosticsWalker extends Lint.RuleWalker {
   }
 
   visitArrowFunction(node: ts.ArrowFunction) {
-    this.checkFunctionReturnType(node);
+    if (requiresReturnType(node)) {
+      this.checkFunctionReturnType(node);
+    } else if (node.type) {
+      this.addFailureAtNode(node.type, Rule.UNNECESSARY_RETURN_TYPE);
+    }
     super.visitArrowFunction(node);
   }
 
@@ -66,3 +71,11 @@ class DiagnosticsWalker extends Lint.RuleWalker {
     }
   }
 }
+
+function requiresReturnType(node: ts.ArrowFunction): boolean {
+  if (ts.isCallExpression(node.parent) && ts.isIdentifier(node.parent.expression)
+    && ["lengthof", "nameof"].includes(node.parent.expression.text)) {
+    return true;
+  }
+  return !ts.isCallLikeExpression(node.parent);
+}

tests/compiler/std/array.ts

@@ -396,25 +396,25 @@ var i: i32;
   arr[2] = 2;
   arr[3] = 3;
 
-  i = arr.findIndex((value: i32, index: i32, array: Array<i32>): bool => value == 0);
+  i = arr.findIndex((value: i32, index: i32, array: Array<i32>) => value == 0);
 
   assert(i == 0);
 
-  i = arr.findIndex((value: i32, index: i32, array: Array<i32>): bool => value == 1);
+  i = arr.findIndex((value: i32, index: i32, array: Array<i32>) => value == 1);
   assert(i == 1);
 
-  i = arr.findIndex((value: i32, index: i32, array: Array<i32>): bool => value == 100);
+  i = arr.findIndex((value: i32, index: i32, array: Array<i32>) => value == 100);
   assert(i == -1);
 
   // Test side effect push
-  i = arr.findIndex((value: i32, index: i32, array: Array<i32>): bool => {
+  i = arr.findIndex((value: i32, index: i32, array: Array<i32>) => {
     array.push(100); // push side effect should not affect this method by spec
     return value == 100;
   });
   // array should be changed, but this method result should be calculated for old array length
   assert(i == -1);
   assert(arr.length == 8);
-  i = arr.findIndex((value: i32, index: i32, array: Array<i32>): bool => value == 100);
+  i = arr.findIndex((value: i32, index: i32, array: Array<i32>) => value == 100);
   assert(i != -1);
 
   arr.pop();
@@ -423,7 +423,7 @@ var i: i32;
   arr.pop();
 
   // Test side effect pop
-  i = arr.findIndex((value: i32, index: i32, array: Array<i32>): bool => {
+  i = arr.findIndex((value: i32, index: i32, array: Array<i32>) => {
     array.pop(); // popped items shouldn't be looked up, and we shouldn't go out of bounds
     return value == 100;
   });
@@ -438,21 +438,21 @@ var i: i32;
 // Array#every /////////////////////////////////////////////////////////////////////////////////////
 
 {
-  let every = arr.every((value: i32, index: i32, array: Array<i32>): bool => value >= 0);
+  let every = arr.every((value: i32, index: i32, array: Array<i32>) => value >= 0);
   assert(every == true);
 
-  every = arr.every((value: i32, index: i32, array: Array<i32>): bool => value <= 0);
+  every = arr.every((value: i32, index: i32, array: Array<i32>) => value <= 0);
   assert(every == false);
 
   // Test side effect push
-  every = arr.every((value: i32, index: i32, array: Array<i32>): bool => {
+  every = arr.every((value: i32, index: i32, array: Array<i32>) => {
     array.push(100); // push side effect should not affect this method by spec
     return value < 10;
   });
   // array should be changed, but this method result should be calculated for old array length
   assert(every == true);
   assert(arr.length == 8);
-  every = arr.every((value: i32, index: i32, array: Array<i32>): bool => value < 10);
+  every = arr.every((value: i32, index: i32, array: Array<i32>) => value < 10);
   assert(every == false);
 
   arr.pop();
@@ -461,7 +461,7 @@ var i: i32;
   arr.pop();
 
   // Test side effect pop
-  every = arr.every((value: i32, index: i32, array: Array<i32>): bool => {
+  every = arr.every((value: i32, index: i32, array: Array<i32>) => {
     array.pop(); //poped items shouldn't be looked up, and we shouldn't go out of bounds
     return value < 3;
   });
@@ -476,21 +476,21 @@ var i: i32;
 // Array#some //////////////////////////////////////////////////////////////////////////////////////
 
 {
-  let some = arr.some((value: i32, index: i32, array: Array<i32>): bool => value >= 3);
+  let some = arr.some((value: i32, index: i32, array: Array<i32>) => value >= 3);
   assert(some == true);
 
-  some = arr.some((value: i32, index: i32, array: Array<i32>): bool => value <= -1);
+  some = arr.some((value: i32, index: i32, array: Array<i32>) => value <= -1);
   assert(some == false);
 
   // Test side effect push
-  some = arr.some((value: i32, index: i32, array: Array<i32>): bool => {
+  some = arr.some((value: i32, index: i32, array: Array<i32>) => {
     array.push(100); // push side effect should not affect this method by spec
     return value > 10;
   });
   // array should be changed, but this method result should be calculated for old array length
   assert(some == false);
   assert(arr.length == 8);
-  some = arr.some((value: i32, index: i32, array: Array<i32>): bool => value > 10);
+  some = arr.some((value: i32, index: i32, array: Array<i32>) => value > 10);
   assert(some == true);
 
   arr.pop();
@@ -499,7 +499,7 @@ var i: i32;
   arr.pop();
 
   // Test side effect pop
-  some = arr.some((value: i32, index: i32, array: Array<i32>): bool => {
+  some = arr.some((value: i32, index: i32, array: Array<i32>) => {
     array.pop(); // poped items shouldn't be looked up, and we shouldn't go out of bounds
     return value > 3;
   });
@@ -515,20 +515,20 @@ var i: i32;
 
 {
   i = 0;
-  arr.forEach((value: i32, index: i32, array: Array<i32>): void => { i += value; });
+  arr.forEach((value: i32, index: i32, array: Array<i32>) => { i += value; });
   assert(i == 6);
 
   // Test side effect push
   i = 0;
-  arr.forEach((value: i32, index: i32, array: Array<i32>): void => {
+  arr.forEach((value: i32, index: i32, array: Array<i32>) => {
     array.push(100); //push side effect should not affect this method by spec
     i += value;
   });
   // array should be changed, but this method result should be calculated for old array length
   assert(i == 6);
   assert(arr.length == 8);
   i = 0;
-  arr.forEach((value: i32, index: i32, array: Array<i32>): void => { i += value; });
+  arr.forEach((value: i32, index: i32, array: Array<i32>) => { i += value; });
   assert(i == 406);
 
   arr.pop();
@@ -538,7 +538,7 @@ var i: i32;
 
   // Test side effect pop
   i = 0;
-  arr.forEach((value: i32, index: i32, array: Array<i32>): void => {
+  arr.forEach((value: i32, index: i32, array: Array<i32>) => {
     array.pop(); //poped items shouldn't be looked up, and we shouldn't go out of bounds
     i += value;
   });
@@ -550,7 +550,7 @@ var i: i32;
   arr.push(3);
 
   // Test rehash action effec
-  arr.forEach((value: i32, index: i32, array: Array<i32>): void => {
+  arr.forEach((value: i32, index: i32, array: Array<i32>) => {
     if (index == 0) {
       for (let i = 0; i < 4; i++) {
         array.pop();
@@ -582,13 +582,13 @@ var i: i32;
 // Array#map ///////////////////////////////////////////////////////////////////////////////////////
 
 {
-  let newArr: f32[] = arr.map<f32>((value: i32, index: i32, array: Array<i32>): f32 => <f32>value);
+  let newArr: f32[] = arr.map<f32>((value: i32, index: i32, array: Array<i32>) => <f32>value);
   assert(newArr.length == 4);
   assert(newArr[0] == <f32>arr[0]);
 
   // Test side effect push
   i = 0;
-  arr.map<i32>((value: i32, index: i32, array: Array<i32>): i32 => {
+  arr.map<i32>((value: i32, index: i32, array: Array<i32>) => {
     array.push(100); //push side effect should not affect this method by spec
     i += value;
     return value;
@@ -597,7 +597,7 @@ var i: i32;
   assert(arr.length == 8);
 
   i = 0;
-  arr.map<i32>((value: i32, index: i32, array: Array<i32>): i32 => {
+  arr.map<i32>((value: i32, index: i32, array: Array<i32>) => {
     i += value;
     return value;
   });
@@ -610,7 +610,7 @@ var i: i32;
 
   // Test side effect pop
   i = 0;
-  arr.map<i32>((value: i32, index: i32, array: Array<i32>): i32 => {
+  arr.map<i32>((value: i32, index: i32, array: Array<i32>) => {
     array.pop(); //poped items shouldn't be looked up, and we shouldn't go out of bounds
     i += value;
     return value;
@@ -626,12 +626,12 @@ var i: i32;
 // Array#filter ////////////////////////////////////////////////////////////////////////////////////
 
 {
-  let filteredArr: i32[] = arr.filter((value: i32, index: i32, array: Array<i32>): bool => value >= 2);
+  let filteredArr: i32[] = arr.filter((value: i32, index: i32, array: Array<i32>) => value >= 2);
   assert(filteredArr.length == 2);
 
   // Test side effect push
   i = 0;
-  arr.filter((value: i32, index: i32, array: Array<i32>): bool => {
+  arr.filter((value: i32, index: i32, array: Array<i32>) => {
     array.push(100); //push side effect should not affect this method by spec
     i += value;
     return value >= 2;
@@ -640,7 +640,7 @@ var i: i32;
   assert(arr.length == 8);
 
   i = 0;
-  arr.filter((value: i32, index: i32, array: Array<i32>): bool => {
+  arr.filter((value: i32, index: i32, array: Array<i32>) => {
     i += value;
     return value >= 2;
   });
@@ -653,7 +653,7 @@ var i: i32;
 
   // Test side effect pop
   i = 0;
-  arr.filter((value: i32, index: i32, array: Array<i32>): bool => {
+  arr.filter((value: i32, index: i32, array: Array<i32>) => {
     array.pop(); //poped items shouldn't be looked up, and we shouldn't go out of bounds
     i += value;
     return value >= 2;
@@ -900,23 +900,23 @@ function assertSortedDefault<T>(arr: Array<T>): void {
   let randomized64  = createRandomOrderedArray(64);
   let randomized257 = createRandomOrderedArray(257);
 
-  assertSorted<i32>(randomized64, (a: i32, b: i32): i32 => a - b);
-  assertSorted<i32>(randomized64, (a: i32, b: i32): i32 => b - a);
+  assertSorted<i32>(randomized64, (a: i32, b: i32) => a - b);
+  assertSorted<i32>(randomized64, (a: i32, b: i32) => b - a);
 
-  assertSorted<i32>(randomized257, (a: i32, b: i32): i32 => a - b);
-  assertSorted<i32>(randomized257, (a: i32, b: i32): i32 => b - a);
+  assertSorted<i32>(randomized257, (a: i32, b: i32) => a - b);
+  assertSorted<i32>(randomized257, (a: i32, b: i32) => b - a);
 }
 
 // Test sorting complex objects
 {
   let reversedNested512 = createReverseOrderedNestedArray(2);
-  assertSorted<i32[]>(reversedNested512, (a: i32[], b: i32[]): i32 => a[0] - b[0]);
+  assertSorted<i32[]>(reversedNested512, (a: i32[], b: i32[]) => a[0] - b[0]);
 }
 
 // Test sorting reference elements
 {
   let reversedElements512 = createReverseOrderedElementsArray(512);
-  assertSorted<Proxy<i32>>(reversedElements512, (a: Proxy<i32>, b: Proxy<i32>): i32 => a.x - b.x);
+  assertSorted<Proxy<i32>>(reversedElements512, (a: Proxy<i32>, b: Proxy<i32>) => a.x - b.x);
 }
 
 // Test sorting strings