From b636cfc2981b71a8da2b053e715566758f77a1f9 Mon Sep 17 00:00:00 2001 From: "clintwood (Office)" Date: Fri, 6 Jul 2018 12:15:46 +0200 Subject: [PATCH 1/2] add option -C, --comment Transform flow types to flow Comment Types in output --- flow-remove-types | 6 +- index.js | 34 +++++- test/expected-comments.js | 168 +++++++++++++++++++++++++++++ test/expected-nested-comments.js | 177 +++++++++++++++++++++++++++++++ test/source-nested-comments.js | 159 +++++++++++++++++++++++++++ 5 files changed, 538 insertions(+), 6 deletions(-) create mode 100644 test/expected-comments.js create mode 100644 test/expected-nested-comments.js create mode 100644 test/source-nested-comments.js diff --git a/flow-remove-types b/flow-remove-types index 91fb132..f3f6865 100755 --- a/flow-remove-types +++ b/flow-remove-types @@ -14,6 +14,7 @@ var usage = 'Usage: flow-remove-types [options] [sources] \n' + ' -x, --extensions File extensions to transform\n' + ' -o, --out-file The file path to write transformed file to\n' + ' -d, --out-dir The directory path to write transformed files within\n' + +' -C, --comment Transform flow types to flow Comment Types in output\n' + ' -a, --all Transform all files, not just those with a @flow comment\n' + ' -p, --pretty Remove flow types without replacing with spaces, \n' + ' producing prettier output but may require using source maps\n' + @@ -71,6 +72,7 @@ var ignore = /node_modules/; var extensions = [ '.js', '.mjs', '.jsx', '.flow', '.es6' ]; var outDir; var outFile; +var commentTypes; var all; var pretty; var sourceMaps; @@ -94,6 +96,8 @@ while (i < process.argv.length) { outFile = process.argv[i++]; } else if (arg === '-d' || arg === '--out-dir') { outDir = process.argv[i++]; + } else if (arg === '-C' || arg === '--comment') { + commentTypes = true; } else if (arg === '-a' || arg === '--all') { all = true; } else if (arg === '-p' || arg === '--pretty') { @@ -231,7 +235,7 @@ function btoa(str) { function transformSource(content, filepath) { try { - return flowRemoveTypes(content, { all: all, pretty: pretty }); + return flowRemoveTypes(content, { all: all, pretty: pretty, commentTypes: commentTypes }); } catch (error) { if (error.loc) { var line = error.loc.line - 1; diff --git a/index.js b/index.js index 84c6b12..efbdb23 100644 --- a/index.js +++ b/index.js @@ -14,6 +14,9 @@ var vlq = require('vlq'); * If true, removes types completely rather than replacing with spaces. * This may require using source maps. * + * - commentTypes: (default: false) + * If true, transforms types to Comment Types using shortform syntax. + * * Returns an object with two methods: * * - .toString() @@ -25,6 +28,7 @@ var vlq = require('vlq'); module.exports = function flowRemoveTypes(source, options) { // Options var all = Boolean(options && options.all); + var commentTypes = Boolean(options && options.commentTypes); if (options && options.checkPragma) { throw new Error( 'flow-remove-types: the "checkPragma" option has been replaced by "all".' @@ -61,8 +65,8 @@ module.exports = function flowRemoveTypes(source, options) { pretty: Boolean(options && options.pretty) }; - // Remove the flow pragma. - if (pragmaStart !== -1) { + // Remove the flow pragma. if not using Comment Types + if (!commentTypes && pragmaStart !== -1) { var pragmaIdx = findTokenIndex(ast.tokens, pragmaStart); var pragmaType = ast.tokens[pragmaIdx].type; if (pragmaType === 'CommentLine' || pragmaType === 'CommentBlock') { @@ -76,9 +80,14 @@ module.exports = function flowRemoveTypes(source, options) { return resultPrinter(options, source, removedNodes); } +var LINE_RX = /(\r\n?|\n|\u2028|\u2029)/; +// var NESTED_COMMENTS_RX = /\s*(\/\*(.|\s)*?\*\/|\s*\/\/.*\r?\n)\s*/g; +var NESTED_COMMENTS_RX = / *(\/\*.*?\*\/| *\/\/.*) */g; + function resultPrinter(options, source, removedNodes) { // Options var pretty = Boolean(options && options.pretty); + var commentTypes = Boolean(options && options.commentTypes); return { toString: function () { @@ -94,7 +103,24 @@ function resultPrinter(options, source, removedNodes) { var node = removedNodes[i]; result += source.slice(lastPos, node.start); lastPos = node.end; - if (!pretty) { + if (commentTypes) { + // Remove nested comments with a regexp replace + var toComment = source.slice(node.start, node.end).replace(NESTED_COMMENTS_RX, ' ').replace(/\s+;/, ';'); + if (!node.loc || node.loc.start.line === node.loc.end.line) { + // possibly use the shorter single ':' syntax + if (toComment && toComment[0] === ':') { + result += ' /*' + toComment + ' */ '; + } else { + result += '/*:: ' + toComment + ' */'; + } + } else { + var toCommentLines = toComment.split(LINE_RX); + // TODO: detect file line endings scheme (\n or \r\n) + toCommentLines.unshift('/*::\n'); + toCommentLines.push('\n*/'); + result += toCommentLines.join(''); + } + } else if (!pretty) { var toReplace = source.slice(node.start, node.end); if (!node.loc || node.loc.start.line === node.loc.end.line) { result += space(toReplace.length); @@ -121,8 +147,6 @@ function resultPrinter(options, source, removedNodes) { } } -var LINE_RX = /(\r\n?|\n|\u2028|\u2029)/; - // A collection of methods for each AST type names which contain Flow types to // be removed. var removeFlowVisitor = { diff --git a/test/expected-comments.js b/test/expected-comments.js new file mode 100644 index 0000000..dc2e55e --- /dev/null +++ b/test/expected-comments.js @@ -0,0 +1,168 @@ +/* @flow */ + +// Regular import +import { + Something, + /*:: type SomeType *//*:: , */ + /*:: typeof SomeOtherThing */ +} from 'some-module'; + +// Import types +/*:: import type { SomeType } from 'some-module'; */ + +// Typed function +async function test(x /*: Type */ , y /*.*/ /*:: ? */ /*.*/ , z /*.*/ /*:: ? */ /*.*/ /*: number */ = 123) /*: string */ { + // Typed expression + return await (x /*: any */ ); +} + +// Interface +/*:: +interface Foo { + prop: any; + + method(): mixed; +} +*/ + +// Exported interface +/*:: +export interface IThing { + exported: true; +} +*/ + +// Interface extends +/*:: +interface SillyFoo extends Foo { + silly: string; +} +*/ + +// Implements interface +class Bar extends Other /*:: implements */ /*.*/ /*:: Foo *//*:: , */ /*:: ISomething */ { + // Class Property with default value + answer /*: number */ = 42; + + // Class Property + /*:: prop: any; */ + + method() /*: mixed */ { + return; + } +} + +// Class expression implements interface +var SomeClass = class Baz /*:: implements */ /*:: Foo */ { + /*:: prop: any; */ + + method() /*: mixed */ { + return; + } +}; + +// Parametric class +class Wrapper/*:: */ { + get() /*: T */ { + return this.value; + } + + map/*:: */() /*: Wrapper */ { + // do something + } +} + +// Extends Parametric class +class StringWrapper extends Wrapper/*:: */ { + // ... +} + +// Declare class +/*:: +declare class Baz { + method(): mixed; +} +*/ + +// Declare funtion +/*:: declare function someFunc(): void; */ + +// Declare interface +/*:: +declare interface ISomething { + answer: number; +} +*/ + +// Declare module +/*:: +declare module 'fs' { + declare function readThing(path: string): string; +} +*/ + +// Declare type alias +/*:: +declare type Location = { + lat: number, + lon: number +}; +*/ + +// Declare variable +/*:: declare var SOME_CONST: string; */ + +// Type alias +/*:: type T = string; */ + +// Export type +/*:: export type { T }; */ + +// Regular export +export { Wrapper }; + +// Exported type alias +/*:: export type ONE = { one: number }; */ + +// Object with types within +var someObj = { + objMethod() /*: void */ { + // do nothing. + } +} + +// Example from README +import SomeClass from 'some-module' +/*:: import type { SomeInterface } from 'some-module' */ + +export class MyClass/*:: */ extends SomeClass /*:: implements */ /*:: SomeInterface */ { + + /*:: value: T */ + + constructor(value /*: T */ ) { + this.value = value + } + + get() /*: T */ { + return this.value + } + +} + +// Test async/await functions +async function asyncFunction/*:: */(input /*: T */ ) /*: Promise */ { + return await t; +} + +// Test read-only data +/*:: +export type TestReadOnly = {| + +readOnly: $ReadOnlyArray<> +|}; +*/ + +// Test covariant type variant class with constaint and default. +export class TestClassWithDefault/*:: <+T: TestReadOnly = TestReadOnly> */ { + + constructor() {} +} diff --git a/test/expected-nested-comments.js b/test/expected-nested-comments.js new file mode 100644 index 0000000..65877e3 --- /dev/null +++ b/test/expected-nested-comments.js @@ -0,0 +1,177 @@ +/* @flow */ + +// Regular import +import { + Something, + //a1 + /*:: type SomeType *//*:: , */ //a3 + /*:: typeof SomeOtherThing */ /*a5*/ +} from 'some-module'; + +// Import types +/*:: import type { SomeType } from 'some-module'; */ //b8 + +// Import types +/*:: +import type { + SomeOtherType, + SomeOtherOtherType +} from 'some-module'; +*/ //c10 + +// Typed function +async function test(x /*: Type */ , y /*.*/ /*:: ? */ /*.*/ , z /*.*/ /*:: ? */ /*.*/ /*: number */ = 123) /*: string */ { + // Typed expression + return await (x /*: any */ ); +} + +// Interface +/*:: +interface Foo { + prop: any; + + method(): mixed; +} +*/ + +// Exported interface +/*:: +export interface IThing { + exported: true; +} +*/ + +// Interface extends +/*:: +interface SillyFoo extends Foo { + silly: string; +} +*/ + +// Implements interface +class Bar extends Other /*:: implements */ /*.*/ /*:: Foo *//*:: , */ /*:: ISomething */ { + // Class Property with default value + answer /*: number */ = 42; //f1 + + // Class Property + /*:: prop: any; */ + + method() /*: mixed */ { + return; + } +} + +// Class expression implements interface +var SomeClass = class Baz /*:: implements */ /*:: Foo */ { + /*:: prop: any; */ + + method() /*: mixed */ { + return; + } +}; + +// Parametric class +class Wrapper/*:: */ { + get() /*: T */ { + return this.value; + } + + map/*:: */() /*: Wrapper */ { + // do something + } +} + +// Extends Parametric class +class StringWrapper extends Wrapper/*:: */ { + // ... +} + +// Declare class +/*:: +declare class Baz { + method() : mixed; +} +*/ + +// Declare funtion +/*:: declare function someFunc(): void; */ + +// Declare interface +/*:: +declare interface ISomething { + answer: number; +} +*/ + +// Declare module +/*:: +declare module 'fs' { + declare function readThing(path: string): string; +} +*/ + +// Declare type alias +/*:: +declare type Location = { + lat: number, + lon: number +}; +*/ + +// Declare variable +/*:: declare var SOME_CONST: string; */ + +// Type alias +/*:: type T = string; */ + +// Export type +/*:: export type { T }; */ + +// Regular export +export { Wrapper }; + +// Exported type alias +/*:: export type ONE = { one: number }; */ + +// Object with types within +var someObj = { + objMethod() /*: void */ { + // do nothing. + } +} + +// Example from README +import SomeClass from 'some-module' +/*:: import type { SomeInterface } from 'some-module' */ + +export class MyClass/*:: */ extends SomeClass /*:: implements */ /*:: SomeInterface */ { + + /*:: value: T */ + + constructor(value /*: T */ ) { + this.value = value + } + + get() /*: T */ { + return this.value + } + +} + +// Test async/await functions +async function asyncFunction/*:: */(input /*: T */ ) /*: Promise */ { + return await t; +} + +// Test read-only data +/*:: +export type TestReadOnly = {| + +readOnly: $ReadOnlyArray<> +|}; +*/ + +// Test covariant type variant class with constaint and default. +export class TestClassWithDefault/*:: <+T: TestReadOnly = TestReadOnly> */ { + + constructor() {} +} diff --git a/test/source-nested-comments.js b/test/source-nested-comments.js new file mode 100644 index 0000000..5daf8f3 --- /dev/null +++ b/test/source-nested-comments.js @@ -0,0 +1,159 @@ +/* @flow */ + +// Regular import +import { + Something, + //a1 + type /*a2*/ SomeType, //a3 + typeof /*a4*/ SomeOtherThing /*a5*/ +} from 'some-module'; + +// Import types +import /*b1*/ type /*b2*/ { /*b3*/ SomeType /*b4*/ } /*b5*/ from /*b6*/ 'some-module' /*b7*/; //b8 + +// Import types +import /*c1*/ type /*c2*/ { + /*c3*/ SomeOtherType, //c4 + /*c5*/ SomeOtherOtherType //c6 +} /*c7*/ from /*c8*/ 'some-module' /*c9*/; //c10 + +// Typed function +async function test(x: Type, y /*.*/ ? /*.*/ , z /*.*/ ? /*.*/ : /*.*/ number = 123): string { + // Typed expression + return await (x: /*d1*/ any); +} + +// Interface +interface Foo { + prop: any; //e1 + /*e2*/ + method(): /*e3*/ mixed; //e4 +} + +// Exported interface +export interface IThing { + exported: true; +} + +// Interface extends +interface SillyFoo extends Foo { + silly: string; +} + +// Implements interface +class Bar extends Other implements /*.*/ Foo, ISomething { + // Class Property with default value + answer: number = 42; //f1 + + // Class Property + prop: any; + + method(): mixed { + return; + } +} + +// Class expression implements interface +var SomeClass = class Baz implements Foo { + prop: any; + + method(): mixed { + return; + } +}; + +// Parametric class +class Wrapper { + get(): T { + return this.value; + } + + map(): Wrapper { + // do something + } +} + +// Extends Parametric class +class StringWrapper extends Wrapper { + // ... +} + +// Declare class +declare /*g1*/ class /*g2*/ Baz /*g3*/ { + method() /*g4*/ : /*g5*/ mixed; +} + +// Declare funtion +declare function someFunc(): void; + +// Declare interface +declare interface ISomething { + answer: number; +} + +// Declare module +declare module 'fs' { + declare function readThing(path: string): string; +} + +// Declare type alias +declare type Location = { + lat: number, + lon: number +}; + +// Declare variable +declare var SOME_CONST: string; + +// Type alias +type T = string; + +// Export type +export type { T }; + +// Regular export +export { Wrapper }; + +// Exported type alias +export type ONE = { one: number }; + +// Object with types within +var someObj = { + objMethod(): void { + // do nothing. + } +} + +// Example from README +import SomeClass from 'some-module' +import type { SomeInterface } from 'some-module' + +export class MyClass extends SomeClass implements SomeInterface { + + value: T + + constructor(value: T) { + this.value = value + } + + get(): T { + return this.value + } + +} + +// Test async/await functions +async function asyncFunction(input: T): Promise { + return await t; +} + +// Test read-only data +export type TestReadOnly = {| + +readOnly: $ReadOnlyArray<> +|}; + +// Test covariant type variant class with constaint and default. +export class TestClassWithDefault<+T: TestReadOnly = TestReadOnly> { + + constructor() {} +} From 88b03b1472ab1e1c39713d84489139f5b1307dbc Mon Sep 17 00:00:00 2001 From: "clintwood (Office)" Date: Fri, 6 Jul 2018 12:54:53 +0200 Subject: [PATCH 2/2] add tests for --comment / -C command line option --- test.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test.sh b/test.sh index 6d8a526..ee6a8d8 100755 --- a/test.sh +++ b/test.sh @@ -10,7 +10,7 @@ echo "Test: flow-remove-types --pretty test/source.js" DIFF=$(./flow-remove-types --pretty test/source.js | diff test/expected-pretty.js -); if [ -n "$DIFF" ]; then echo "$DIFF"; exit 1; fi; -# Test expected source maps with --pretty --sourcemaps +# Test expected source maps with --pretty --sourcemaps --out-dir echo "Test: flow-remove-types --pretty --sourcemaps test/source.js -d test/expected-with-maps" TEST_DIR=$(cd $(dirname ${BASH_SOURCE[0]}) && pwd) DIR=$(mktemp -d) @@ -29,6 +29,16 @@ echo "Test: flow-remove-types --pretty --sourcemaps inline test/source.js" DIFF=$(./flow-remove-types --pretty --sourcemaps inline test/source.js | diff test/expected-pretty-inlinemap.js -); if [ -n "$DIFF" ]; then echo "$DIFF"; exit 1; fi; +# Test expected output with --comment option +echo "Test: flow-remove-types --comment test/source.js" +DIFF=$(./flow-remove-types --comment test/source.js | diff test/expected-comments.js -); +if [ -n "$DIFF" ]; then echo "$DIFF"; exit 1; fi; + +# Test expected output with --comment option +echo "Test: flow-remove-types --comment test/source-nested-comments.js" +DIFF=$(./flow-remove-types --comment test/source-nested-comments.js | diff test/expected-nested-comments.js -); +if [ -n "$DIFF" ]; then echo "$DIFF"; exit 1; fi; + # Test expected output with @flow outside of comments echo "Test: flow-remove-types test/without-flow.js" DIFF=$(./flow-remove-types test/without-flow.js | diff test/without-flow.js -);