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

function types: optional and vararg arguments. #650

Merged
merged 2 commits into from
Nov 11, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
289 changes: 17 additions & 272 deletions package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"@types/source-map": "^0.5.1",
"@types/source-map-support": "^0.2.27",
"chai": "^3.5.0",
"chai-diff": "^1.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good, but FWIW in my experience it's easier to just let the thing write out new goldens and examine the output with 'git diff'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I'm interactively tweaking the emit, I find it much easier to see a good diff there than re-gen the goldens. That also runs a high risk of missing a change. But I understand workflows are different.

"clang-format": "^1.0.55",
"diff": "^3.2.0",
"glob": "^7.0.0",
Expand Down
26 changes: 22 additions & 4 deletions src/type-translator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,7 @@ export class TypeTranslator {

/** Converts a ts.Signature (function signature) to a Closure function type. */
private signatureToClosure(sig: ts.Signature): string {
// TODO(martinprobst): Consider harmonizing some overlap with emitFunctionType in tsickle.ts.
const params = this.convertParams(sig);
let typeStr = `function(${params.join(', ')})`;

Expand All @@ -601,10 +602,27 @@ export class TypeTranslator {
}

private convertParams(sig: ts.Signature): string[] {
return sig.parameters.map(param => {
const paramType = this.typeChecker.getTypeOfSymbolAtLocation(param, this.node);
return this.translate(paramType);
});
const paramTypes: string[] = [];
// The Signature itself does not include information on optional and var arg parameters.
// Use its declaration to recover that information.
const decl = sig.declaration;
for (let i = 0; i < sig.parameters.length; i++) {
const param = sig.parameters[i];

const paramDecl = decl.parameters[i];
const optional = !!paramDecl.questionToken;
const varArgs = !!paramDecl.dotDotDotToken;
let paramType = this.typeChecker.getTypeOfSymbolAtLocation(param, this.node);
if (varArgs) {
const typeRef = paramType as ts.TypeReference;
paramType = typeRef.typeArguments![0];
}
let typeStr = this.translate(paramType);
if (varArgs) typeStr = '...' + typeStr;
if (optional) typeStr = typeStr + '=';
paramTypes.push(typeStr);
}
return paramTypes;
}

warn(msg: string) {
Expand Down
1 change: 1 addition & 0 deletions test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ ts_library(
name = "golden_test_lib",
srcs = [
"golden_tsickle_test.ts",
"chai-diff.d.ts",
],
tsconfig = "//:tsconfig.json",
deps = [
Expand Down
24 changes: 24 additions & 0 deletions test/chai-diff.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

/**
* @fileoverview Minimal type declarations for the chai-diff library.
* @see https://www.npmjs.com/package/chai-diff
*/

declare namespace Chai {
interface Assertion {
differentFrom(val: string, opts?: {}): void;
}
}

declare module 'chai-diff' {
// tslint:disable-next-line:no-any This is the type defined by chai.use.
function chaiDiff(chai: any, utils: any): void;
export = chaiDiff;
}
8 changes: 6 additions & 2 deletions test/golden_tsickle_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {expect} from 'chai';
import {expect, use as chaiUse} from 'chai';
// tslint:disable-next-line:no-require-imports chai-diff needs a CommonJS import.
Copy link
Contributor

Choose a reason for hiding this comment

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

How come? You can
import * as chaiDiff from 'chai-diff';
and then use it as a function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a wontfix TypeScript issue about it:
microsoft/TypeScript#5073

import chaiDiff = require('chai-diff');
import * as fs from 'fs';
import * as path from 'path';
import * as ts from 'typescript';
Expand All @@ -16,6 +18,8 @@ import {normalizeLineEndings} from '../src/util';

import * as testSupport from './test_support';

chaiUse(chaiDiff);

// Set TEST_FILTER=foo to only run tests from the foo package.
// Set TEST_FILTER=foo/bar to also filter for the '/bar' file.
const TEST_FILTER = (() => {
Expand Down Expand Up @@ -92,7 +96,7 @@ function compareAgainstGolden(
}
}
} else {
expect(output).to.equal(golden, `${goldenPath}`);
expect(output).not.differentFrom(golden!, {});
}
}

Expand Down
2 changes: 2 additions & 0 deletions test_files/type/type.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ let /** @type {{optional: (undefined|string|boolean)}} */ typeOptionalUnion = {}
let /** @type {function(): void} */ typeFunc = function () { };
let /** @type {function(number, ?): string} */ typeFunc2 = function (a, b) { return ''; };
let /** @type {function(number, function(number): string): string} */ typeFunc3 = function (x, cb) { return ''; };
let /** @type {function(number, (undefined|!Object)=): string} */ typeFuncOptionalArg;
let /** @type {function(number, ...number): void} */ typeFuncVarArgs;
/**
* @param {function(number): number} callback
* @return {void}
Expand Down
5 changes: 3 additions & 2 deletions test_files/type/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ let typeOptionalUnion: {optional?: string|boolean} = {};

let typeFunc: () => void = function() {};
let typeFunc2: (a: number, b: any) => string = function(a, b) { return ''; };
let typeFunc3: (x: number, callback: (x: number) => string) => string = function(x, cb) { return ''; }
// TODO: let typeFunc4: (a: number, ...args: number[]) => void;
let typeFunc3: (x: number, callback: (x: number) => string) => string = function(x, cb) { return ''; };
let typeFuncOptionalArg: (a: number, b?: {}) => string;
let typeFuncVarArgs: (a: number, ...args: number[]) => void;

function typeCallback(callback: (val: number) => number) { }
typeCallback(val => val + 1);
Expand Down
8 changes: 7 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,12 @@ camelcase@^2.0.1:
version "2.1.1"
resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-2.1.1.tgz#7c1d16d679a1bbe59ca02cacecfb011e201f5a1f"

chai-diff@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/chai-diff/-/chai-diff-1.0.1.tgz#6c66894700d80cd90350ab4e4403625d4f53a1c1"
dependencies:
diff "^2.2.1"

chai@^3.5.0:
version "3.5.0"
resolved "https://registry.yarnpkg.com/chai/-/chai-3.5.0.tgz#4d02637b067fe958bdbfdd3a40ec56fef7373247"
Expand Down Expand Up @@ -306,7 +312,7 @@ diff@3.2.0:
version "3.2.0"
resolved "https://registry.yarnpkg.com/diff/-/diff-3.2.0.tgz#c9ce393a4b7cbd0b058a725c93df299027868ff9"

diff@^2.0.2:
diff@^2.0.2, diff@^2.2.1:
version "2.2.3"
resolved "https://registry.yarnpkg.com/diff/-/diff-2.2.3.tgz#60eafd0d28ee906e4e8ff0a52c1229521033bf99"

Expand Down