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

Codefix for implementing interfaces #11547

Merged
merged 101 commits into from
Dec 30, 2016
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
101 commits
Select commit Hold shift + click to select a range
1438f9a
Codefixes for implement interface and change extends to implements.
Oct 11, 2016
7a5cb5f
Merge branch 'pvb/codeaction/api' into interfaceFixes
Oct 12, 2016
bf3e487
Merge branch 'master' into interfaceFixes
Oct 13, 2016
6b4f6b5
Remove Type Assignability QuickFix
aozgaa Oct 25, 2016
151f940
Refactor classDeclarations
aozgaa Oct 25, 2016
d1cea73
Refactor Loop
aozgaa Oct 25, 2016
c2feab6
Rename Tests
aozgaa Oct 25, 2016
f74872d
temp
aozgaa Oct 26, 2016
2abf906
Merge branch 'master' into interfaceFixes
aozgaa Oct 26, 2016
132b746
Covnert to Doc Comment
aozgaa Oct 26, 2016
ecc029f
more temp
aozgaa Oct 26, 2016
a66b0ae
Split CodeFixes in Separate Files
aozgaa Oct 26, 2016
5d9a4e3
Move codeFixProvider.ts
aozgaa Oct 27, 2016
3ac9ffa
Nerf Extends to implements For Now
aozgaa Oct 27, 2016
d24236b
Simplify getCodeFixChanges
aozgaa Oct 27, 2016
7758e6d
Rename Most Tests
aozgaa Oct 28, 2016
7272833
Rename and Add a Test
aozgaa Oct 28, 2016
834245c
Add Failing Tests
aozgaa Oct 28, 2016
c89b97b
Add removeAbstractModifier Fix
aozgaa Oct 29, 2016
16dc834
extends->implements w/ implements keyword present
aozgaa Oct 29, 2016
cbaea99
Cleanup Extends -> Interface Change
aozgaa Oct 31, 2016
bc21346
Refactor fourslash testing for codeFixes
aozgaa Nov 1, 2016
99ae5d9
Remove unused tests
aozgaa Nov 1, 2016
aa6ecd4
Fix linting errors
aozgaa Nov 1, 2016
3240000
Merge branch 'master' into interfaceFixes
aozgaa Nov 1, 2016
0380f3f
Recognize modifiers
aozgaa Nov 2, 2016
1b60a97
Remove unused
aozgaa Nov 2, 2016
e5279fd
Rename and simplify fourslash interface
aozgaa Nov 2, 2016
04968ab
fix references to codefixes?
aozgaa Nov 2, 2016
d02eb6c
fix jakefile1
aozgaa Nov 2, 2016
3b0b696
broken
aozgaa Nov 3, 2016
36c5bef
Add tests and simplify existing ones
aozgaa Nov 4, 2016
1b8486d
Still re-writing missing member grabber
aozgaa Nov 4, 2016
b7b30aa
Add straggling Test
aozgaa Nov 4, 2016
71d1744
Test Fixes
aozgaa Nov 8, 2016
8c35185
Expose More TypeChecker
aozgaa Nov 8, 2016
bc1bb0e
Make test failure more readable
aozgaa Nov 8, 2016
0ce53f0
rename tests
aozgaa Nov 8, 2016
b26ba83
Expose signatureToString, addSupressAnyReturn Flag
aozgaa Nov 8, 2016
11cea6a
Use TypeChecker to Get Types, Print
aozgaa Nov 8, 2016
7141a2a
Better error message in fourslash
aozgaa Nov 8, 2016
55bf3e3
Use new engine for interface fixes
aozgaa Nov 8, 2016
4441380
tests: edit expected type params
aozgaa Nov 8, 2016
1ec234a
Edit error handling tests
aozgaa Nov 8, 2016
6bd35fb
Fix Type Param method Tests
aozgaa Nov 8, 2016
4973852
fix linter errors
aozgaa Nov 8, 2016
1d6ef6a
cleanup
aozgaa Nov 9, 2016
d842a6f
Merge branch 'master' into interfaceFixes
aozgaa Nov 9, 2016
b1e97b3
Get one fix per interface
aozgaa Nov 10, 2016
c650c33
Update tests
aozgaa Nov 10, 2016
0591e1b
Simplify Testing
aozgaa Nov 10, 2016
263734f
Add a test
aozgaa Nov 14, 2016
4b202ab
Initialize result to empy array
aozgaa Nov 14, 2016
357ed7e
Remove Inner Loop
aozgaa Nov 14, 2016
f6fc320
Get Ancestor instead of manual walk
aozgaa Nov 14, 2016
efd16c7
Spell out names fully
aozgaa Nov 14, 2016
bba96da
remove multiple implements TODO
aozgaa Nov 14, 2016
cfe50d1
Fix extends -> implements for decorators/modifiers
aozgaa Nov 14, 2016
ad3035d
remove generated file from PR
aozgaa Nov 14, 2016
d8b359f
Fix typo and capitalization
aozgaa Nov 14, 2016
6400d53
Fix handling of default class
aozgaa Nov 14, 2016
c010a0e
Use getTypeOfSymbolAtLocation
aozgaa Nov 15, 2016
395d736
Remove getSymbolOfNode from TypeChecker interface
aozgaa Nov 15, 2016
d7d4bf6
Handle class expressions
aozgaa Nov 15, 2016
a94d955
Aggregate changes before building result
aozgaa Nov 15, 2016
43afb80
remove fix
aozgaa Nov 15, 2016
6ed8d18
unexpose resolveStructuredTypeMembers
aozgaa Nov 15, 2016
389959a
Merge branch 'master' into interfaceFixes
aozgaa Nov 15, 2016
69118cd
Merge branch 'master' into interfaceFixes
Nov 17, 2016
4af0e2a
Merge branch 'master' into interfaceFixes
Nov 28, 2016
680af0f
use getStart()
Nov 28, 2016
16b146f
Use array instead of map
Nov 28, 2016
f37640a
Add args to diagnostic message
Nov 28, 2016
5d6a714
move helpers under codefix dir
Nov 28, 2016
bf48564
FIx typo in method stub.
Nov 29, 2016
ba80ce6
stubbing extra completions
Nov 29, 2016
8134d64
Merge branch 'master' into interfaceFixes
Dec 5, 2016
0c1772b
Merge branch 'interfaceFixes' of https://github.com/Microsoft/TypeScr…
Dec 5, 2016
f0c7713
implement getters/setters as property
Dec 5, 2016
c511aea
Add Support for multiple signatures
Dec 7, 2016
5cd0ea3
handle well-known computed property/method names
Dec 8, 2016
c22e47d
add test for computed literals
Dec 8, 2016
c1a41b9
Expose indexSignaturePrinting
Dec 8, 2016
2f51b36
add missing index signature support
Dec 9, 2016
97b3d7a
make index signature fix work with generics
Dec 9, 2016
819a654
Add tests and fix rest parameters
Dec 9, 2016
5e48e33
cleanup
Dec 9, 2016
1338b94
Simplify rest parameter handling
Dec 9, 2016
b9ae36c
Simplify index signature generation
Dec 9, 2016
d724517
Merge branch 'master' into interfaceFixes
Dec 9, 2016
469745b
Synthetic signature uses existing parameter names
Dec 12, 2016
1ba6c86
Merge branch 'master' into interfaceFixes
Dec 12, 2016
ad01110
Consolidate Tests
Dec 12, 2016
3cfac08
abstract class expr instantiation and abstract fix works with class expr
Dec 14, 2016
4673812
refactor fix
Dec 14, 2016
4b02099
add tests
Dec 14, 2016
8be8819
simplify and inline methods
Dec 14, 2016
d75d548
implicit any for synthesized signature
Dec 14, 2016
4af3937
Merge branch 'master' into interfaceFixes
Dec 14, 2016
97f18c9
Cleanup
Dec 14, 2016
3fc94bb
Rename Tests
Dec 14, 2016
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
135 changes: 135 additions & 0 deletions scripts/buildProtocol.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

102 changes: 30 additions & 72 deletions src/services/codefixes/interfaceFixes.ts
Original file line number Diff line number Diff line change
@@ -1,46 +1,5 @@
/* @internal */
namespace ts.codefix {
registerCodeFix({
errorCodes: [Diagnostics.Type_0_is_not_assignable_to_type_1.code],
getCodeActions: (context: CodeFixContext) => {
const sourceFile = context.sourceFile;
const start = context.span.start;
const token = getTokenAtPosition(sourceFile, start);
const checker = context.program.getTypeChecker();
let textChanges: TextChange[] = [];

if (token.kind === SyntaxKind.Identifier && token.parent.kind === SyntaxKind.VariableDeclaration) {
const variableDeclaration = <VariableDeclaration>token.parent;
const membersAndStartPosObject = getMembersAndStartPosFromReference(variableDeclaration);
const variableMembers = membersAndStartPosObject.members;
const trackingAddedMembers: string[] = [];
const startPos: number = membersAndStartPosObject.startPos;

if (variableDeclaration.type.kind === SyntaxKind.TypeReference) {
textChanges = textChanges.concat(getChanges(variableDeclaration.type, variableMembers, startPos, checker, /*reference*/ true, trackingAddedMembers, context.newLineCharacter));
}
else if (variableDeclaration.type.kind === SyntaxKind.UnionType) {
const types = (<UnionTypeNode>variableDeclaration.type).types;
ts.forEach(types, t => {
textChanges = textChanges.concat(getChanges(t, variableMembers, startPos, checker, /*reference*/ true, trackingAddedMembers, context.newLineCharacter));
});
}
}

if (textChanges.length > 0) {
return [{
description: getLocaleSpecificMessage(Diagnostics.Implement_interface_on_reference),
changes: [{
fileName: sourceFile.fileName,
textChanges: textChanges
}]
}];
}

return undefined;
}
});

registerCodeFix({
Copy link
Contributor

Choose a reason for hiding this comment

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

please split these into different files. let's keep each code fix in its own file.

Copy link
Contributor

Choose a reason for hiding this comment

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

you can still share the helpers between files if you need to.

Copy link
Contributor

Choose a reason for hiding this comment

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

Split, and helpers moved to src/services/utilities.ts

errorCodes: [Diagnostics.Class_0_incorrectly_implements_interface_1.code],
getCodeActions: (context: CodeFixContext) => {
Expand All @@ -49,28 +8,29 @@ namespace ts.codefix {
const token = getTokenAtPosition(sourceFile, start);
const checker = context.program.getTypeChecker();

let textChanges: TextChange[] = [];

if (token.kind === SyntaxKind.Identifier && token.parent.kind === SyntaxKind.ClassDeclaration) {
if (token.kind === SyntaxKind.Identifier && isClassLike(token.parent)) {
const classDeclaration = <ClassDeclaration>token.parent;
const startPos: number = classDeclaration.members.pos;
const classMembers = getClassMembers(classDeclaration);
const classMembers = ts.map(getNamedClassMemberDeclarations(classDeclaration), member => member.name.getText());
const trackingAddedMembers: string[] = [];
const interfaceClauses = ts.getClassImplementsHeritageClauseElements(classDeclaration);

let textChanges: TextChange[] = undefined;

for (let i = 0; interfaceClauses && i < interfaceClauses.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what if the class does not have curlies? can add tests?

textChanges = textChanges.concat(getChanges(interfaceClauses[i], classMembers, startPos, checker, /*reference*/ false, trackingAddedMembers, context.newLineCharacter));
let newChanges = getChanges(interfaceClauses[i], classMembers, startPos, checker, /*reference*/ false, trackingAddedMembers, context.newLineCharacter);
textChanges = textChanges ? textChanges.concat(newChanges) : newChanges;
}
}

if (textChanges.length > 0) {
return [{
description: getLocaleSpecificMessage(Diagnostics.Implement_interface_on_class),
changes: [{
fileName: sourceFile.fileName,
textChanges: textChanges
}]
}];
if (textChanges && textChanges.length > 0) {
return [{
description: getLocaleSpecificMessage(Diagnostics.Implement_interface_on_class),
changes: [{
fileName: sourceFile.fileName,
textChanges: textChanges
}]
}];
}
}

return undefined;
Expand All @@ -87,13 +47,13 @@ namespace ts.codefix {

let textChanges: TextChange[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the point of this initialization, you can just override it below. consider just leaving this as undefined, and checking for undefined below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Refactored


if (token.kind === SyntaxKind.Identifier && token.parent.kind === SyntaxKind.ClassDeclaration) {
if (token.kind === SyntaxKind.Identifier && isClassLike(token.parent)) {
const classDeclaration = <ClassDeclaration>token.parent;
const startPos = classDeclaration.members.pos;
const classMembers = getClassMembers(classDeclaration);
const abstractClassMembers = ts.map(getNamedClassAbstractMemberDeclarations(classDeclaration), member => member.name.getText());
const trackingAddedMembers: string[] = [];
const extendsClause = ts.getClassExtendsHeritageClauseElement(classDeclaration);
textChanges = textChanges.concat(getChanges(extendsClause, classMembers, startPos, checker, /*reference*/ false, trackingAddedMembers, context.newLineCharacter));
textChanges = textChanges.concat(getChanges(extendsClause, abstractClassMembers, startPos, checker, /*reference*/ false, trackingAddedMembers, context.newLineCharacter));
}

if (textChanges.length > 0) {
Expand All @@ -116,10 +76,10 @@ namespace ts.codefix {

if (type && type.symbol && type.symbol.declarations) {
const interfaceMembers = getMembers(<InterfaceDeclaration>type.symbol.declarations[0], checker);
Copy link
Contributor

Choose a reason for hiding this comment

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

what about inherited members? e.g.:

interface Base { a: string }
interface Derived extends Base { b: number}

class C implements Derived {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This works, see e.g unImplementedInterface30.ts

for (let j = 0; interfaceMembers && j < interfaceMembers.length; j++) {
if (interfaceMembers[j].name && existingMembers.indexOf(interfaceMembers[j].name.getText()) === -1) {
if (interfaceMembers[j].kind === SyntaxKind.PropertySignature) {
const interfaceProperty = <PropertySignature>interfaceMembers[j];
for(let interfaceMember of interfaceMembers){
if (interfaceMember.name && existingMembers.indexOf(interfaceMember.name.getText()) === -1) {
if (interfaceMember.kind === SyntaxKind.PropertySignature) {
const interfaceProperty = <PropertySignature>interfaceMember;
if (trackingAddedMembers.indexOf(interfaceProperty.name.getText()) === -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 does not handle computed properties correctly. e.g. [Symbol.Iterator]. please add tests.

let propertyText = "";
if (reference) {
Expand All @@ -134,8 +94,8 @@ namespace ts.codefix {
trackingAddedMembers.push(interfaceProperty.name.getText());
}
}
else if (interfaceMembers[j].kind === SyntaxKind.MethodSignature || interfaceMembers[j].kind === SyntaxKind.MethodDeclaration) {
const interfaceMethod = <MethodSignature>interfaceMembers[j];
else if (interfaceMember.kind === SyntaxKind.MethodSignature || interfaceMember.kind === SyntaxKind.MethodDeclaration) {
const interfaceMethod = <MethodSignature>interfaceMember;
handleMethods(interfaceMethod, startPos, reference, trackingAddedMembers, changesArray, newLineCharacter);
}
}
Expand Down Expand Up @@ -168,14 +128,12 @@ namespace ts.codefix {
return result;
}

function getClassMembers(classDeclaration: ClassDeclaration): string[] {
const classMembers: string[] = [];
for (let i = 0; classDeclaration.members && i < classDeclaration.members.length; i++) {
if (classDeclaration.members[i].name) {
classMembers.push(classDeclaration.members[i].name.getText());
}
}
return classMembers;
function getNamedClassMemberDeclarations(classDeclaration: ClassDeclaration): ClassElement[] {
return classDeclaration.members.filter(member => member.name);
}

function getNamedClassAbstractMemberDeclarations(classDeclaration: ClassDeclaration): ClassElement[] {
return getNamedClassMemberDeclarations(classDeclaration).filter(member => getModifierFlags(member) & ModifierFlags.Abstract);
}

function getMembersAndStartPosFromReference(variableDeclaration: VariableDeclaration): { startPos: number, members: string[] } {
Expand Down
9 changes: 5 additions & 4 deletions tests/cases/fourslash/unImplementedInterface14.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
//// |]f2() {}
//// }

verify.codeFixAtPosition(`f1(){
throw new Error('Method not Implemented');
},
`);
verify.not.codeFixAvailable();
// verify.codeFixAtPosition(`f1(){
// throw new Error('Method not Implemented');
// },
// `);
11 changes: 6 additions & 5 deletions tests/cases/fourslash/unImplementedInterface15.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
////
//// |]}

verify.codeFixAtPosition(`
f1(){
throw new Error('Method not Implemented');
}
`);
verify.not.codeFixAvailable();
// verify.codeFixAtPosition(`
Copy link
Contributor

Choose a reason for hiding this comment

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

why are these still here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed.

// f1(){
// throw new Error('Method not Implemented');
// }
// `);
5 changes: 3 additions & 2 deletions tests/cases/fourslash/unImplementedInterface16.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
//// var x: I1 ={[|
//// |]}

verify.codeFixAtPosition(`x : ""
`);
verify.not.codeFixAvailable();
// verify.codeFixAtPosition(`x : ""
// `);
5 changes: 3 additions & 2 deletions tests/cases/fourslash/unImplementedInterface17.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
//// var x: I1 ={[|
//// |]}

verify.codeFixAtPosition(`x : 0
`);
verify.not.codeFixAvailable();
// verify.codeFixAtPosition(`x : 0
// `);
5 changes: 3 additions & 2 deletions tests/cases/fourslash/unImplementedInterface18.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
////
//// |]}

verify.codeFixAtPosition(`x : false
`);
verify.not.codeFixAvailable();
// verify.codeFixAtPosition(`x : false
// `);
5 changes: 3 additions & 2 deletions tests/cases/fourslash/unImplementedInterface19.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@
//// |]f1(){}
//// }

verify.codeFixAtPosition(`x : "",
`);
verify.not.codeFixAvailable();
// verify.codeFixAtPosition(`x : "",
// `);
5 changes: 3 additions & 2 deletions tests/cases/fourslash/unImplementedInterface20.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@
//// |]f1(){}
//// }

verify.codeFixAtPosition(`x : 0,
`);
verify.not.codeFixAvailable();
// verify.codeFixAtPosition(`x : 0,
// `);
5 changes: 3 additions & 2 deletions tests/cases/fourslash/unImplementedInterface21.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@
//// |]f1(){}
//// }

verify.codeFixAtPosition(`x : false,
`);
verify.not.codeFixAvailable();
// verify.codeFixAtPosition(`x : false,
// `);
5 changes: 3 additions & 2 deletions tests/cases/fourslash/unImplementedInterface22.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
//// var x: I1 ={[|
//// |]}

verify.codeFixAtPosition(`x : null
`);
verify.not.codeFixAvailable();
// verify.codeFixAtPosition(`x : null
// `);
Loading