This repository has been archived by the owner on May 24, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 52
Add --comment option #68
Closed
+549
−7
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if both |
||
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 = { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably also add the new test cases to |
||
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 -); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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/*:: <T> */ { | ||
get() /*: T */ { | ||
return this.value; | ||
} | ||
|
||
map/*:: <M> */() /*: Wrapper<M> */ { | ||
// do something | ||
} | ||
} | ||
|
||
// Extends Parametric class | ||
class StringWrapper extends Wrapper/*:: <string> */ { | ||
// ... | ||
} | ||
|
||
// 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/*:: <T> */ extends SomeClass /*:: implements */ /*:: SomeInterface */ { | ||
|
||
/*:: value: T */ | ||
|
||
constructor(value /*: T */ ) { | ||
this.value = value | ||
} | ||
|
||
get() /*: T */ { | ||
return this.value | ||
} | ||
|
||
} | ||
|
||
// Test async/await functions | ||
async function asyncFunction/*:: <T> */(input /*: T */ ) /*: Promise<T> */ { | ||
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() {} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably resolve this TODO. I wouldn't want to merge this feature if it messes up Windows line endings.