add refactor of convert private field to getter and setter #22143

Kingwl · 2018-02-23T09:43:24Z

DanielRosenwasser · 2018-02-23T18:25:43Z

src/compiler/diagnosticMessages.json

@@ -3965,5 +3965,9 @@
    "Convert to ES6 module": {
        "category": "Message",
        "code": 95017
+    },
+    "Convert to getter and setter": {


"Generate 'get' and 'set' accessors."

You can potentially keep it as "getter" and "setter", but I don't know how well that localizes. I think the key thing I have in mind is that you should have the text "generate" in there somehow.

DanielRosenwasser · 2018-02-23T18:27:10Z

src/services/refactors/ConvertToGetterAndSetter.ts

+    function createGetter (fieldName: string, name: string, type: TypeNode) {
+        return createGetAccessor(
+            /*decorators*/ undefined,
+            [createToken(SyntaxKind.PublicKeyword)],


It'd be nice if we could detect whether other public members use modifiers instead of injecting one.

DanielRosenwasser · 2018-02-23T18:29:35Z

src/services/refactors/ConvertToGetterAndSetter.ts

+    function getConvertibleFieldAtPosition(file: SourceFile, startPosition: number): Info | undefined {
+        const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false);
+        const propertyDeclaration = findAncestor(node.parent, isPropertyDeclaration);
+        if (!(propertyDeclaration && propertyDeclaration.name.getText().charAt(0) === "_" && hasModifier(propertyDeclaration, ModifierFlags.Private))) return undefined;


Give a short comment explaining why you want to do this check.

Also, use charCodeAt(0) === CharacterCodes._.

Also, break this into at least two lines and use braces please.

Also, does this work for computed properties? You probably want to make sure that your propertyDeclaration.name is an identifier. That way you could also use propertyDeclaration.name.text after you use a type-guard.

I don't like that you have to write private _foo: number; to get this refactor. Wouldn't it make more sense to trigger on public foo: number; and convert it to private _foo: number; public get foo() {} public set foo() {}?

DanielRosenwasser · 2018-02-23T18:31:37Z

src/services/refactors/ConvertToGetterAndSetter.ts

+    function getConvertibleFieldAtPosition(file: SourceFile, startPosition: number): Info | undefined {
+        const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false);
+        const propertyDeclaration = findAncestor(node.parent, isPropertyDeclaration);
+        if (!(propertyDeclaration && propertyDeclaration.name.getText().charAt(0) === "_" && hasModifier(propertyDeclaration, ModifierFlags.Private))) return undefined;


Also, does this work for computed properties? You probably want to make sure that your propertyDeclaration.name is an identifier. That way you could also use propertyDeclaration.name.text after you use a type-guard.

DanielRosenwasser · 2018-02-23T18:32:02Z

src/services/refactors/ConvertToGetterAndSetter.ts

+
+        return {
+            fieldName: propertyDeclaration.name.getText(),
+            name: propertyDeclaration.name.getText().substring(1),


You will actually need to make sure that you create a unique name that doesn't exist in the current class. For simplicity, I would just bail out if the property with this name already exists.

DanielRosenwasser · 2018-02-23T18:32:39Z

src/services/refactors/ConvertToGetterAndSetter.ts

+        if (!(propertyDeclaration && propertyDeclaration.name.getText().charAt(0) === "_" && hasModifier(propertyDeclaration, ModifierFlags.Private))) return undefined;
+
+        return {
+            fieldName: propertyDeclaration.name.getText(),


Same deal with propertyDeclaration.name.text here and below.

alfaproject · 2018-02-24T14:29:06Z

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess2.ts

+    actionDescription: "Generate 'get' and 'set' accessors",
+    newContent: `class A {
+    protected _a: string;
+    public get a(): string {


If the initial property access is protected then this should be protected. The inner variable should be private

DanielRosenwasser · 2018-02-25T00:43:20Z

src/services/refactors/refactors.ts

@@ -4,3 +4,4 @@
 /// <reference path="extractSymbol.ts" />
 /// <reference path="installTypesForPackage.ts" />
 /// <reference path="useDefaultImport.ts" />
+/// <reference path="ConvertToGetterAndSetter.ts" />


Nit: please keep file casing consistent by renaming the file and this reference to it. (first letter should be lower-case.

DanielRosenwasser

Getting close! Make sure you also have tests for a

static member (maybe don't provide any quickfix here)
readonly member (maybe only generate a get-accessor, not clear if you want to keep the readonly modifier around)

DanielRosenwasser · 2018-02-25T00:46:14Z

src/services/refactors/ConvertToGetterAndSetter.ts

+        );
+    }
+
+    function updateOriginPropertyDeclaration (propertyDeclaration: PropertyDeclaration, fieldName: string, modifiers: ModifiersArray) {


updateoriginalPropertyDeclaration

DanielRosenwasser · 2018-02-25T00:47:21Z

src/services/refactors/ConvertToGetterAndSetter.ts

+    function updateOriginPropertyDeclaration (propertyDeclaration: PropertyDeclaration, fieldName: string, modifiers: ModifiersArray) {
+        return updateProperty(
+            propertyDeclaration,
+            /*decorators*/ undefined,


Seems problematic to drop decorators here.

DanielRosenwasser · 2018-02-25T00:47:34Z

src/services/refactors/ConvertToGetterAndSetter.ts

+            /*decorators*/ undefined,
+            modifiers,
+            fieldName,
+            /*questionOrExclamationToken*/ undefined,


Also strange to drop the token.

DanielRosenwasser · 2018-02-25T00:48:07Z

src/services/refactors/ConvertToGetterAndSetter.ts

+        };
+    }
+
+    interface Info { originName: string; fieldName: string; accessorName: string; propertyDeclaration: PropertyDeclaration; needUpdateName: boolean; hasModifiers: boolean; needUpdateModifiers: boolean; }


originName to originalName

Put this on multiple lines.

DanielRosenwasser · 2018-02-25T00:52:08Z

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess1.ts

+////     /*a*/public a: string;/*b*/
+//// }
+
+goTo.select("a", "b");


I believe we have a range API instead, but I haven't ever written one of these tests yet, so this might be fine?

i'm not sure...
I copied it from the refactor Convert Es6 Module

DanielRosenwasser · 2018-02-25T00:56:37Z

src/services/refactors/ConvertToGetterAndSetter.ts

+
+        if (find(members, member => needUpdateName ? member.name.getText() === fieldName : member.name.getText() === accessorName)) return undefined;
+
+        const hasModifiers = !!find(members, member => !!member.modifiers);


Hmm, I think I initially meant whether a field had a public modifier explicitly

what if the field is Protected

Kingwl · 2018-02-26T08:11:08Z

After thinking, I tend to not support readonly, because of the following two case:

1. Initializer is not a Literal (may be side effects)

public readonly a : TT = new TT;

it cannot be convert as Literal

public get a () {
  return "Literal";
}

2.

before:

public readonly a : TT = new TT;

after:

private _a: TT = new TT;

public get a () {
  return this._a
}

it can mod _a in the context of same class
this does not match the readonly semantics

or:

private readonly _a: TT = new TT;

public get a () {
  return this._a
}

it seems do nothing

mhegazy · 2018-03-29T23:18:01Z

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts

+//// }
+
+goTo.select("a", "b");
+verify.not.refactorAvailable("Generate 'get' and 'set' accessors");


i do not see why we would not allow this. you just need to come up with a temporary name that starts with _a, set the rename location to it, and let the user decide what they want to call it. you can just call createUniqueName to create you an identifier with a unique name and use it instead.

See https://github.com/Microsoft/TypeScript/blob/1ed30c6d4d78aeb7de0c81604dbf38120d82e82b/src/services/refactors/extractSymbol.ts#L1137-L1161

mhegazy · 2018-03-29T23:19:09Z

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess7.ts

+    actionDescription: "Generate 'get' and 'set' accessors",
+    newContent: `class A {
+    _a: string;
+    get a(): string {


why? i would expect this to be get _a

mhegazy · 2018-03-29T23:26:06Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        if (!fieldInfo) return undefined;
+
+        const changeTracker = textChanges.ChangeTracker.fromContext(context);
+        const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);


newLineCharacter should now be available on ChangeTracker.newLineCharacter , no need to recompute it,

mhegazy · 2018-03-29T23:27:28Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const propertyDeclaration = findAncestor(node.parent, isPropertyDeclaration);
+
+        if (!propertyDeclaration || propertyDeclaration.name.kind !== SyntaxKind.Identifier) return undefined;
+        // make sure propertyDeclaration have only AccessibilityModifier


what about static? you can have static accessors

mhegazy · 2018-03-29T23:28:03Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+
+    function getConvertibleFieldAtPosition(file: SourceFile, startPosition: number): Info | undefined {
+        const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false);
+        const propertyDeclaration = findAncestor(node.parent, isPropertyDeclaration);


we also need to handle parameter properties. so this can be a property or a parameter declaration.

mhegazy · 2018-03-29T23:29:40Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        // make sure propertyDeclaration have only AccessibilityModifier
+        if ((getModifierFlags(propertyDeclaration) | ModifierFlags.AccessibilityModifier) !== ModifierFlags.AccessibilityModifier) return undefined;
+
+        const containerClass = getContainingClass(propertyDeclaration);


instead of these two lines, you can just check that isClassLike(propertyDeclaration.parent) or isClassLike(parameterDeclaration.parent.parent)

mhegazy · 2018-03-29T23:31:09Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const members = getMembersOfDeclaration(containerClass);
+        if (!members) return undefined;
+
+        const needUpdateName = propertyDeclaration.name.text.charCodeAt(0) !== CharacterCodes._;


do not think you need that. as i noted in my other comment, we should pick a unique name that has a seed of "_" + <property name>, then set the rename location on this name. the user will get the chance to rename it right there and then.

so in short the name should always be updated.

mhegazy · 2018-03-29T23:33:37Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const accessorName = needUpdateName ? propertyDeclaration.name.text : propertyDeclaration.name.text.substring(1);
+        const fieldName = `_${accessorName}`;
+
+        if (find(members, member => needUpdateName ? member.name.getText() === fieldName : member.name.getText() === accessorName)) return undefined;


do not think you need these either.

mhegazy · 2018-03-29T23:35:17Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+
+        const hasModifiers = !!find(members, member => !!member.modifiers);
+        const needUpdateModifiers = hasModifiers && (!propertyDeclaration.modifiers || !hasModifier(propertyDeclaration, ModifierFlags.Private));
+        const accessorType = propertyDeclaration.questionToken ? mergeTypeNodeToUnion(propertyDeclaration.type, createKeywordTypeNode(SyntaxKind.UndefinedKeyword)) : propertyDeclaration.type;


i would say you do not need to create this type until you need to execute the refactoring. this function is called in both getAvailableActions and getEditsForAction, so ideally we would defer that until we are computing the edits.

mhegazy · 2018-03-29T23:39:32Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const fieldInfo = getConvertibleFieldAtPosition(file, startPosition);
+        if (!fieldInfo) return undefined;
+
+        return [


nit, keep the [ and { on the same line.

mhegazy · 2018-03-29T23:46:19Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const hasModifiers = !!find(members, member => !!member.modifiers);
+        const needUpdateModifiers = hasModifiers && (!propertyDeclaration.modifiers || !hasModifier(propertyDeclaration, ModifierFlags.Private));
+        const accessorType = propertyDeclaration.questionToken ? mergeTypeNodeToUnion(propertyDeclaration.type, createKeywordTypeNode(SyntaxKind.UndefinedKeyword)) : propertyDeclaration.type;
+


also keep in mind that propertyDeclaration.type may not be there. if this is the case, you want to not include a type, nor do you want to merge it with undefined.

also you should add a test for that.

mhegazy

Looks good, a few comments. also please add a test for applying the refactoring on a .js file

ghost · 2018-03-29T23:23:23Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+    function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
+        const { file, startPosition } = context;
+
+        const fieldInfo = getConvertibleFieldAtPosition(file, startPosition);


Variable can be inlined

ghost · 2018-03-29T23:24:48Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const accessorName = needUpdateName ? propertyDeclaration.name.text : propertyDeclaration.name.text.substring(1);
+        const fieldName = `_${accessorName}`;
+
+        if (find(members, member => needUpdateName ? member.name.getText() === fieldName : member.name.getText() === accessorName)) return undefined;


Prefer members.some(...) over find when just using as a boolean

Also, .getText() on every member might be slow since getText() has to do scanning. Might want to do something like function getDeclarationName from services.ts to just get Identifier and StringLiteral-named properties.

Also, getMemberName(member.name) === (needUpdateName ? fieldName : accessorName)

Actually, can avoid looping over members and instead check for checker.getPropertyOfType(checker.getTypeAtLocation(containerClass.name), needUpdateName ? fieldName : accessorName) which avoids conflicts with supertypes too.

ghost · 2018-03-29T23:30:11Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const containerClass = getContainingClass(propertyDeclaration);
+        if (!containerClass) return undefined;
+
+        const members = getMembersOfDeclaration(containerClass);


Isn't this just containerClass.members?

ghost · 2018-03-29T23:32:38Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
+
+        const { fieldName, accessorName, accessorType, propertyDeclaration, needUpdateName, hasModifiers, needUpdateModifiers } = fieldInfo;
+        const accessorModifiers = hasModifiers ? (


hasModifiers currently is true if any member on the class has modifiers. Instead this could just preserve whatever modifier was there before on the particular member without looking at other members. I.e., public x: number becomes public get x() { ... } public set x() { ... } and x: number becomes get x() { ... } set x() { ... }, making hasModifiers unnecessary.

ghost · 2018-03-29T23:33:26Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const changeTracker = textChanges.ChangeTracker.fromContext(context);
+        const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
+
+        const { fieldName, accessorName, accessorType, propertyDeclaration, needUpdateName, hasModifiers, needUpdateModifiers } = fieldInfo;


needUpdateName and needUpdateModifiers aren't used separately, so I would combine them to one property.

ghost · 2018-03-29T23:34:01Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const fieldInfo = getConvertibleFieldAtPosition(file, startPosition);
+        if (!fieldInfo) return undefined;
+
+        const changeTracker = textChanges.ChangeTracker.fromContext(context);


Nit: would move this closer to its use.

ghost · 2018-03-29T23:34:58Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const modifiers = hasModifiers ? createNodeArray([createToken(SyntaxKind.PrivateKeyword)]) : undefined;
+        if (needUpdateName || needUpdateModifiers) {
+            changeTracker.replaceNode(file, propertyDeclaration, updateoriginalPropertyDeclaration(propertyDeclaration, fieldName, modifiers), {
+                suffix: newLineCharacter


With the latest changes to ChangeTracker it shouldn't be necessary to explicitly specify options. If it is, file an issue and we can fix it later after this PR is merged.

ghost · 2018-03-29T23:36:06Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+            name,
+            [createParameter(
+                /*decorators*/ undefined,
+                /*modifies*/ undefined,


ghost · 2018-03-29T23:41:30Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+        const accessorType = propertyDeclaration.questionToken ? mergeTypeNodeToUnion(propertyDeclaration.type, createKeywordTypeNode(SyntaxKind.UndefinedKeyword)) : propertyDeclaration.type;
+
+        return {
+            originalName: propertyDeclaration.name.text,


Might want to make const originalName early since it's used in 3 other places.

Kingwl · 2018-04-03T02:13:27Z

@mhegazy
PropertyDeclaration is not existed in ClassLikeContainer in JS(probably) 😂

mhegazy · 2018-04-03T17:32:47Z

@mhegazy
PropertyDeclaration is not existed in ClassLikeContainer in JS(probably) 😂

we do allow all "standard-track" features in JS:

mhegazy · 2018-04-03T17:38:14Z

@Kingwl i think we should support readonly as well.. no reason not to..

class C { 
    readonly x = 0;
}

should convert to

class C { 
    private _x = 0;
    get x() { 
        return this._x;
    }
}

I think we should do that in a separate PR though, since there is some complications with constructor access here.. you want to replace all references to the readonly property in the constructor with the new one that was created and not the getter/setter. i.e.

class C { 
    readonly x: number;
    constructor() { 
        this.x = 0;
    }
}

should become:

class C { 
    private _x: number;
    get x() { 
        return this._x;
    }
    constructor () { 
        this._x = 0; // _x and not x here
    }
}

mhegazy · 2018-04-03T18:04:25Z

gave it a quick test, found two issues..

JavaScript files, we should not add accessor modifiers if this is a JS file. should be a simple check in getFieldModifiers and in getAccessorModifiers. Also please add a test for this
comments. we are not removing the comments from the nodes correctly before using them, you probably want to use suppressLeadingAndTrailingTrivia on the name of the field before using it, e.g.

/** class comment */
class C {
    // Field commment
    x = 22;
}

as i said before, we should look into the readonly case, but i would do that in a separate PR. if you can get these two issues fixed, we should be good to go on this PR.

Kingwl · 2018-04-03T23:47:47Z

Sure, but it might be late. I'm enjoying my holiday in Disney(three days)😁

mhegazy · 2018-04-03T23:58:18Z

no worries. take your time. and have a great vacation °o°

Kingwl · 2018-04-06T07:32:15Z

ahhhh, could refactor apply withPropertyAssign ?

mhegazy · 2018-04-06T16:35:05Z

ahhhh, could refactor apply withPropertyAssign ?

i do not see why not. good point.

Kingwl · 2018-04-07T10:50:39Z

src/services/textChanges.ts

@@ -462,6 +468,9 @@ namespace ts.textChanges {
            else if (isVariableDeclaration(node)) {
                return { prefix: ", " };
            }
+            else if (isPropertyAssignment(node)) {
+                return { suffix: "," + this.newLineCharacter }


@mhegazy some thing need help
in this case
how should i add the , after every props
if i update the ObjectLiteralExpression, the original PropertyAssignment will overship with this change

Kingwl · 2018-04-07T10:51:36Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+    function getPropertyAssignmentDeclarationInfo(propertyAssignment: PropertyAssignment): DeclarationInfo | undefined {
+        return {
+            isStatic: false,
+            type: undefined,


what should i do with this type declaration,
should i get the type with typeChecker?

no, you are fine. the type will be inferred from the return type of the getter.

mhegazy · 2018-04-09T21:44:37Z

src/services/refactors/generateGetAccessorAndSetAccessor.ts

@@ -72,16 +89,14 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
        return declaration.modifiers;
    }

-    function getFieldModifiers(isStatic: boolean): NodeArray<Modifier> {
+    function getFieldModifiers(isJS: boolean, isStatic: boolean, isClassLike: boolean): NodeArray<Modifier> | undefined {
+        if (isJS || !isClassLike) return undefined;


you still want to handel the static modifier in the case of js. i would jsut make it

return createNodeArray( append<Modifier>(isJS ? [] : [createToken(SyntaxKind.PrivateKeyword)], isStatic ? createToken(SyntaxKind.StaticKeyword) : undefined));

mhegazy · 2018-04-10T18:11:33Z

@andy-ms any more comments?

mhegazy · 2018-04-10T20:47:25Z

thanks @Kingwl!. wanna get the readonly support in next?

Kingwl · 2018-04-11T01:09:16Z

sure 😅

Kingwl · 2018-04-11T05:34:21Z

wait for pr been merged which @andy-ms has send and thanks for the review 😬

Kingwl · 2018-05-08T02:21:47Z

thanks all for @mhegazy @andy-ms 's review and improve
i'd like to add readonly support right now

mhegazy · 2018-05-08T02:32:20Z

Go for it.

Kingwl force-pushed the add-getter-and-setter branch from 38f3153 to c632a1c Compare February 23, 2018 10:02

DanielRosenwasser reviewed Feb 23, 2018

View reviewed changes

DanielRosenwasser requested changes Feb 23, 2018

View reviewed changes

Kingwl force-pushed the add-getter-and-setter branch from c632a1c to 95c3ca5 Compare February 24, 2018 10:02

alfaproject suggested changes Feb 24, 2018

View reviewed changes

DanielRosenwasser reviewed Feb 25, 2018

View reviewed changes

DanielRosenwasser requested changes Feb 25, 2018

View reviewed changes

DanielRosenwasser reviewed Feb 25, 2018

View reviewed changes

Kingwl force-pushed the add-getter-and-setter branch from 95c3ca5 to 11637de Compare February 26, 2018 07:48

Kingwl force-pushed the add-getter-and-setter branch 2 times, most recently from ba0f71b to e39a889 Compare February 26, 2018 08:29

Kingwl force-pushed the add-getter-and-setter branch from dcdb450 to 9acfc4c Compare March 12, 2018 07:59

mhegazy reviewed Mar 29, 2018

View reviewed changes

ghost reviewed Mar 29, 2018

View reviewed changes

Kingwl added 3 commits April 2, 2018 18:15

add refactor of convert private field to getter and setter

bed084f

fix refactor

0e4d513

stash

507e804

add support of PropertyAssignment and StringLiteral

e5daa8c

Kingwl commented Apr 7, 2018

View reviewed changes

add support for js file

fafdcf5

mhegazy reviewed Apr 9, 2018

View reviewed changes

allow static modifier in js file

0da98bd

mhegazy approved these changes Apr 10, 2018

View reviewed changes

ghost approved these changes Apr 10, 2018

View reviewed changes

ghost merged commit 9c0671d into microsoft:master Apr 10, 2018

This was referenced Apr 10, 2018

generateGetAccessorAndSetAccessor: Fix typos and use type predicate #23310

Merged

generateGetAccessorAndSetAccessor: Share modifier-creating code #23311

Merged

Simplify getConvertibleFieldAtPosition #23315

Merged

ghost mentioned this pull request Apr 10, 2018

generateGetAccessorAndSetAccessor: Preserve a parameter property declaration #23318

Merged

Kingwl deleted the add-getter-and-setter branch April 11, 2018 01:09

Kingwl mentioned this pull request May 8, 2018

add support for readonly modifier #23954

Merged

microsoft locked and limited conversation to collaborators Jul 31, 2018

This pull request was closed.


		if (find(members, member => needUpdateName ? member.name.getText() === fieldName : member.name.getText() === accessorName)) return undefined;

		const hasModifiers = !!find(members, member => !!member.modifiers);

add refactor of convert private field to getter and setter #22143

add refactor of convert private field to getter and setter #22143

Conversation

Kingwl commented Feb 23, 2018

DanielRosenwasser Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielRosenwasser Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielRosenwasser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kingwl Feb 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kingwl commented Feb 26, 2018 • edited Loading

1. Initializer is not a Literal (may be side effects)

2.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhegazy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Mar 29, 2018 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Mar 29, 2018 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kingwl commented Apr 3, 2018 • edited Loading

mhegazy commented Apr 3, 2018

mhegazy commented Apr 3, 2018

mhegazy commented Apr 3, 2018

Kingwl commented Apr 3, 2018

mhegazy commented Apr 3, 2018

Kingwl commented Apr 6, 2018

mhegazy commented Apr 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhegazy commented Apr 10, 2018

mhegazy commented Apr 10, 2018

Kingwl commented Apr 11, 2018

Kingwl commented Apr 11, 2018

Kingwl commented May 8, 2018 • edited Loading

mhegazy commented May 8, 2018

DanielRosenwasser Feb 23, 2018 •

edited

Loading

DanielRosenwasser Feb 23, 2018 •

edited

Loading

Kingwl Feb 26, 2018 •

edited

Loading

Kingwl commented Feb 26, 2018 •

edited

Loading

ghost Mar 29, 2018 •

edited by ghost

Loading

ghost Mar 29, 2018 •

edited by ghost

Loading

Kingwl commented Apr 3, 2018 •

edited

Loading

Kingwl commented May 8, 2018 •

edited

Loading