maxmustermann/TypeScript
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fix import addition location (#17327)

Browse source 
* Add test with bug

* Fix for import placement

* Consolidate comment recognition functions into utilities

* Add another test with all 3 kinds

* Recognize path directives as part of triple slash directives

* Also handle no-default-lib triple-slash comments

* Test for all the triple-slash kinds

* Keep import-placement logic in the quickfix, since its not really a node start; accept new baselines

* Work in not-ES6, use a real no-lib comment

* Remove no default lib triple slash comment, it disables checking and thereby quick fixes

* Copy regex rather than have a regex copy
This commit is contained in:
Wesley Wigham 2017-08-09 14:03:37 -07:00 committed by GitHub
parent 445cc8df79
commit 6221d7089e
9 changed files with 118 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
src/compiler/comments.ts
View file

@ -415,17 +415,7 @@ namespace ts {
* @return true if the comment is a triple-slash comment else false
*/
function isTripleSlashComment(commentPos: number, commentEnd: number) {
// Verify this is /// comment, but do the regexp match only when we first can find /// in the comment text
// so that we don't end up computing comment string and doing match for all // comments
if (currentText.charCodeAt(commentPos + 1) === CharacterCodes.slash &&
commentPos + 2 < commentEnd &&
currentText.charCodeAt(commentPos + 2) === CharacterCodes.slash) {
const textSubStr = currentText.substring(commentPos, commentEnd);
return textSubStr.match(fullTripleSlashReferencePathRegEx) ||
textSubStr.match(fullTripleSlashAMDReferencePathRegEx) ?
true : false;
}
return false;
return isRecognizedTripleSlashComment(currentText, commentPos, commentEnd);
}
}
}

36
src/compiler/utilities.ts
View file

@ -262,6 +262,32 @@ namespace ts {
return !nodeIsMissing(node);
}
/**
* Determine if the given comment is a triple-slash
*
* @return true if the comment is a triple-slash comment else false
*/
export function isRecognizedTripleSlashComment(text: string, commentPos: number, commentEnd: number) {
// Verify this is /// comment, but do the regexp match only when we first can find /// in the comment text
// so that we don't end up computing comment string and doing match for all // comments
if (text.charCodeAt(commentPos + 1) === CharacterCodes.slash &&
commentPos + 2 < commentEnd &&
text.charCodeAt(commentPos + 2) === CharacterCodes.slash) {
const textSubStr = text.substring(commentPos, commentEnd);
return textSubStr.match(fullTripleSlashReferencePathRegEx) ||
textSubStr.match(fullTripleSlashAMDReferencePathRegEx) ||
textSubStr.match(fullTripleSlashReferenceTypeReferenceDirectiveRegEx) ||
textSubStr.match(defaultLibReferenceRegEx) ?
true : false;
}
return false;
}
export function isPinnedComment(text: string, comment: CommentRange) {
return text.charCodeAt(comment.pos + 1) === CharacterCodes.asterisk &&
text.charCodeAt(comment.pos + 2) === CharacterCodes.exclamation;
}
export function getTokenPosOfNode(node: Node, sourceFile?: SourceFileLike, includeJsDoc?: boolean): number {
// With nodes that have no width (i.e. 'Missing' nodes), we actually *don't*
// want to skip trivia because this will launch us forward to the next token.
@ -660,6 +686,7 @@ namespace ts {
export let fullTripleSlashReferencePathRegEx = /^(\/\/\/\s*<reference\s+path\s*=\s*)('|")(.+?)\2.*?\/>/;
export let fullTripleSlashReferenceTypeReferenceDirectiveRegEx = /^(\/\/\/\s*<reference\s+types\s*=\s*)('|")(.+?)\2.*?\/>/;
export let fullTripleSlashAMDReferencePathRegEx = /^(\/\/\/\s*<amd-dependency\s+path\s*=\s*)('|")(.+?)\2.*?\/>/;
export let defaultLibReferenceRegEx = /^(\/\/\/\s*<reference\s+no-default-lib\s*=\s*)('|")(.+?)\2\s*\/>/;
export function isPartOfTypeNode(node: Node): boolean {
if (SyntaxKind.FirstTypeNode <= node.kind && node.kind <= SyntaxKind.LastTypeNode) {
@ -1846,7 +1873,7 @@ namespace ts {
export function getFileReferenceFromReferencePath(comment: string, commentRange: CommentRange): ReferencePathMatchResult {
const simpleReferenceRegEx = /^\/\/\/\s*<reference\s+/gim;
const isNoDefaultLibRegEx = /^(\/\/\/\s*<reference\s+no-default-lib\s*=\s*)('|")(.+?)\2\s*\/>/gim;
const isNoDefaultLibRegEx = new RegExp(defaultLibReferenceRegEx.source, "gim");
if (simpleReferenceRegEx.test(comment)) {
if (isNoDefaultLibRegEx.test(comment)) {
return { isNoDefaultLib: true };
@ -2823,7 +2850,7 @@ namespace ts {
//
// var x = 10;
if (node.pos === 0) {
leadingComments = filter(getLeadingCommentRanges(text, node.pos), isPinnedComment);
leadingComments = filter(getLeadingCommentRanges(text, node.pos), isPinnedCommentLocal);
}
}
else {
@ -2869,9 +2896,8 @@ namespace ts {
return currentDetachedCommentInfo;
function isPinnedComment(comment: CommentRange) {
return text.charCodeAt(comment.pos + 1) === CharacterCodes.asterisk &&
text.charCodeAt(comment.pos + 2) === CharacterCodes.exclamation;
function isPinnedCommentLocal(comment: CommentRange) {
return isPinnedComment(text, comment);
}
}

24
src/services/codefixes/importFixes.ts
View file

@ -396,7 +396,7 @@ namespace ts.codefix {
: createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName))]));
const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, createLiteral(moduleSpecifierWithoutQuotes));
if (!lastImportDeclaration) {
changeTracker.insertNodeAt(sourceFile, sourceFile.getStart(), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` });
changeTracker.insertNodeAt(sourceFile, getSourceFileImportLocation(sourceFile), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` });
}
else {
changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: context.newLineCharacter });
@ -413,6 +413,28 @@ namespace ts.codefix {
moduleSpecifierWithoutQuotes
);
function getSourceFileImportLocation(node: SourceFile) {
// For a source file, it is possible there are detached comments we should not skip
const text = node.text;
let ranges = getLeadingCommentRanges(text, 0);
if (!ranges) return 0;
let position = 0;
// However we should still skip a pinned comment at the top
if (ranges.length && ranges[0].kind === SyntaxKind.MultiLineCommentTrivia && isPinnedComment(text, ranges[0])) {
position = ranges[0].end + 1;
ranges = ranges.slice(1);
}
// As well as any triple slash references
for (const range of ranges) {
if (range.kind === SyntaxKind.SingleLineCommentTrivia && isRecognizedTripleSlashComment(node.text, range.pos, range.end)) {
position = range.end + 1;
continue;
}
break;
}
return position;
}
function getModuleSpecifierForNewImport() {
const fileName = sourceFile.fileName;
const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName;

1
tests/baselines/reference/1.0lib-noErrors.js
View file

@ -1158,3 +1158,4 @@ MERCHANTABLITY OR NON-INFRINGEMENT.
See the Apache Version 2.0 License for specific language governing permissions
and limitations under the License.
***************************************************************************** */
/// <reference no-default-lib="true"/>

1
tests/baselines/reference/typeReferenceDirectives1.js
View file

@ -10,6 +10,7 @@ interface A {
}
//// [app.js]
/// <reference types="lib"/>
//// [app.d.ts]

1
tests/baselines/reference/typeReferenceDirectives3.js
View file

@ -16,6 +16,7 @@ interface A {
}
//// [app.js]
/// <reference types="lib"/>
/// <reference path="ref.d.ts" />

4
tests/cases/fourslash/importNameCodeFixNewImportAmbient2.ts
View file

@ -1,6 +1,6 @@
/// <reference path="fourslash.ts" />
////[|/*
////[|/*!
//// * I'm a license or something
//// */
////f1/*0*/();|]
@ -12,7 +12,7 @@
//// }
verify.importFixAtPosition([
`/*
`/*!
* I'm a license or something
*/
import { f1 } from "ambient-module";

35
tests/cases/fourslash/importNameCodeFixNewImportFileAllComments.ts Normal file
View file

@ -0,0 +1,35 @@
/// <reference path="fourslash.ts" />
//// [|/*!
//// * This is a license or something
//// */
//// /// <reference types="node" />
//// /// <reference path="./a.ts" />
//// /// <amd-dependency path="./b.ts" />
//// /**
//// * This is a comment intended to be attached to this interface
//// */
//// export interface SomeInterface {
//// }
//// f1/*0*/();|]
// @Filename: module.ts
//// export function f1() {}
//// export var v1 = 5;
verify.importFixAtPosition([
`/*!
* This is a license or something
*/
/// <reference types="node" />
/// <reference path="./a.ts" />
/// <amd-dependency path="./b.ts" />
import { f1 } from "./module";
/**
* This is a comment intended to be attached to this interface
*/
export interface SomeInterface {
}
f1();`
]);

23
tests/cases/fourslash/importNameCodeFixNewImportFileDetachedComments.ts Normal file
View file

@ -0,0 +1,23 @@
/// <reference path="fourslash.ts" />
//// [|/**
//// * This is a comment intended to be attached to this interface
//// */
//// export interface SomeInterface {
//// }
//// f1/*0*/();|]
// @Filename: module.ts
//// export function f1() {}
//// export var v1 = 5;
verify.importFixAtPosition([
`import { f1 } from "./module";
/**
* This is a comment intended to be attached to this interface
*/
export interface SomeInterface {
}
f1();`
]);