-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Gabritto/semicolons #46832
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
Gabritto/semicolons #46832
Changes from 5 commits
3e59096
64e0bd4
f48dfe5
1916f05
36bed8e
fdb37a4
88897e8
1197993
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -229,6 +229,7 @@ namespace ts.Completions { | |
| triggerCharacter: CompletionsTriggerCharacter | undefined, | ||
| completionKind: CompletionTriggerKind | undefined, | ||
| cancellationToken: CancellationToken, | ||
| formatContext?: formatting.FormatContext, | ||
| ): CompletionInfo | undefined { | ||
| const { previousToken } = getRelevantTokens(position, sourceFile); | ||
| if (triggerCharacter && !isInString(sourceFile, position, previousToken) && !isValidTrigger(sourceFile, triggerCharacter, previousToken, position)) { | ||
|
|
@@ -275,7 +276,7 @@ namespace ts.Completions { | |
|
|
||
| switch (completionData.kind) { | ||
| case CompletionDataKind.Data: | ||
| const response = completionInfoFromData(sourceFile, host, program, compilerOptions, log, completionData, preferences); | ||
| const response = completionInfoFromData(sourceFile, host, program, compilerOptions, log, completionData, preferences, formatContext); | ||
| if (response?.isIncomplete) { | ||
| incompleteCompletionsCache?.set(response); | ||
| } | ||
|
|
@@ -412,6 +413,7 @@ namespace ts.Completions { | |
| log: Log, | ||
| completionData: CompletionData, | ||
| preferences: UserPreferences, | ||
| formatContext: formatting.FormatContext | undefined, | ||
| ): CompletionInfo | undefined { | ||
| const { | ||
| symbols, | ||
|
|
@@ -459,6 +461,7 @@ namespace ts.Completions { | |
| completionKind, | ||
| preferences, | ||
| compilerOptions, | ||
| formatContext, | ||
| isTypeOnlyLocation, | ||
| propertyAccessToConvert, | ||
| isJsxIdentifierExpected, | ||
|
|
@@ -489,6 +492,7 @@ namespace ts.Completions { | |
| completionKind, | ||
| preferences, | ||
| compilerOptions, | ||
| formatContext, | ||
| isTypeOnlyLocation, | ||
| propertyAccessToConvert, | ||
| isJsxIdentifierExpected, | ||
|
|
@@ -638,6 +642,7 @@ namespace ts.Completions { | |
| options: CompilerOptions, | ||
| preferences: UserPreferences, | ||
| completionKind: CompletionKind, | ||
| formatContext: formatting.FormatContext | undefined, | ||
| ): CompletionEntry | undefined { | ||
| let insertText: string | undefined; | ||
| let replacementSpan = getReplacementSpanForContextToken(replacementToken); | ||
|
|
@@ -706,7 +711,7 @@ namespace ts.Completions { | |
| completionKind === CompletionKind.MemberLike && | ||
| isClassLikeMemberCompletion(symbol, location)) { | ||
| let importAdder; | ||
| ({ insertText, isSnippet, importAdder } = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, contextToken)); | ||
| ({ insertText, isSnippet, importAdder } = getEntryForMemberCompletion(host, program, options, preferences, name, symbol, location, contextToken, formatContext)); | ||
| if (importAdder?.hasFixes()) { | ||
| hasAction = true; | ||
| source = CompletionSource.ClassMemberSnippet; | ||
|
|
@@ -832,6 +837,7 @@ namespace ts.Completions { | |
| symbol: Symbol, | ||
| location: Node, | ||
| contextToken: Node | undefined, | ||
| formatContext: formatting.FormatContext | undefined, | ||
| ): { insertText: string, isSnippet?: true, importAdder?: codefix.ImportAdder } { | ||
| const classLikeDeclaration = findAncestor(location, isClassLike); | ||
| if (!classLikeDeclaration) { | ||
|
|
@@ -852,15 +858,16 @@ namespace ts.Completions { | |
| }); | ||
| const importAdder = codefix.createImportAdder(sourceFile, program, preferences, host); | ||
|
|
||
| // Create empty body for possible method implementation. | ||
| let body; | ||
| if (preferences.includeCompletionsWithSnippetText) { | ||
| isSnippet = true; | ||
| // We are adding a tabstop (i.e. `$0`) in the body of the suggested member, | ||
| // if it has one, so that the cursor ends up in the body once the completion is inserted. | ||
| // Note: this assumes we won't have more than one body in the completion nodes, which should be the case. | ||
| const emptyStatement = factory.createExpressionStatement(factory.createIdentifier("")); | ||
| setSnippetElement(emptyStatement, { kind: SnippetKind.TabStop, order: 0 }); | ||
| body = factory.createBlock([emptyStatement], /* multiline */ true); | ||
| const emptyStmt = factory.createEmptyStatement(); | ||
|
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. conceptually, it only makes sense for a tab stop snippet to be attached to a node that doesn't cause anything to be emitted. using an empty identifier, as I was before, was causing some troubles when setting positions for synthetic nodes, because we can't assume the identifier will be empty. an empty statement seemed more appropriate then. 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. I can’t tell if you already got this, but an empty statement is only parsable when terminated by a semicolon, so in the reverse, it’s impossible to emit an empty statement without emitting a semicolon. Is the formatter removing this semicolon or did you somehow get this to emit without one? 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. I hadn't thought of that, I think. The semicolon doesn't exist in this case because the empty statement is never emitted -- when we call 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. Got it. I don’t think it’s a problem, it’s just a little weird to think about. 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. it's definitely weird. I tried to go for the least weird option :/ |
||
| body = factory.createBlock([emptyStmt], /* multiline */ true); | ||
| setSnippetElement(emptyStmt, { kind: SnippetKind.TabStop, order: 0 }); | ||
| } | ||
| else { | ||
| body = factory.createBlock([], /* multiline */ true); | ||
|
|
@@ -911,18 +918,45 @@ namespace ts.Completions { | |
| modifiers = node.modifierFlagsCache | requiredModifiers | presentModifiers; | ||
| } | ||
| node = factory.updateModifiers(node, modifiers & (~presentModifiers)); | ||
|
|
||
| completionNodes.push(node); | ||
| }, | ||
| body, | ||
| codefix.PreserveOptionalFlags.Property, | ||
| isAbstract); | ||
|
|
||
| if (completionNodes.length) { | ||
| insertText = printer.printSnippetList( | ||
| ListFormat.MultiLine | ListFormat.NoTrailingNewLine, | ||
| factory.createNodeArray(completionNodes), | ||
| sourceFile); | ||
| // If we have access to formatting settings, we print the nodes using the emitter, | ||
| // and then format the printed text. | ||
| if (formatContext) { | ||
| const syntheticFile = { | ||
| text: printer.printSnippetList( | ||
| ListFormat.MultiLine | ListFormat.NoTrailingNewLine, | ||
| factory.createNodeArray(completionNodes), | ||
| sourceFile), | ||
| getLineAndCharacterOfPosition(pos: number) { | ||
| return getLineAndCharacterOfPosition(this, pos); | ||
| }, | ||
| }; | ||
|
|
||
| const formatOptions = getFormatCodeSettingsForWriting(formatContext, sourceFile); | ||
| const changes = flatMap(completionNodes, node => { | ||
| const nodeWithPos = textChanges.assignPositionsToNode(node); | ||
| return formatting.formatNodeGivenIndentation( | ||
| nodeWithPos, | ||
| syntheticFile, | ||
| sourceFile.languageVariant, | ||
| /* indentation */ 0, | ||
| /* delta */ 0, | ||
| { ...formatContext, options: formatOptions }); | ||
| }); | ||
| insertText = textChanges.applyChanges(syntheticFile.text, changes); | ||
| } | ||
| else { // Otherwise, just use emitter to print the new nodes. | ||
| insertText = printer.printSnippetList( | ||
| ListFormat.MultiLine | ListFormat.NoTrailingNewLine, | ||
| factory.createNodeArray(completionNodes), | ||
| sourceFile); | ||
| } | ||
| } | ||
|
|
||
| return { insertText, isSnippet, importAdder }; | ||
|
|
@@ -972,8 +1006,8 @@ namespace ts.Completions { | |
| function createSnippetPrinter( | ||
| printerOptions: PrinterOptions, | ||
| ) { | ||
| const printer = createPrinter(printerOptions); | ||
| const baseWriter = createTextWriter(getNewLineCharacter(printerOptions)); | ||
| const baseWriter = textChanges.createWriter(getNewLineCharacter(printerOptions)); | ||
| const printer = createPrinter(printerOptions, baseWriter); | ||
| const writer: EmitTextWriter = { | ||
| ...baseWriter, | ||
| write: s => baseWriter.write(escapeSnippetText(s)), | ||
|
|
@@ -1117,6 +1151,7 @@ namespace ts.Completions { | |
| kind: CompletionKind, | ||
| preferences: UserPreferences, | ||
| compilerOptions: CompilerOptions, | ||
| formatContext: formatting.FormatContext | undefined, | ||
| isTypeOnlyLocation?: boolean, | ||
| propertyAccessToConvert?: PropertyAccessExpression, | ||
| jsxIdentifierExpected?: boolean, | ||
|
|
@@ -1166,6 +1201,7 @@ namespace ts.Completions { | |
| compilerOptions, | ||
| preferences, | ||
| kind, | ||
| formatContext, | ||
| ); | ||
| if (!entry) { | ||
| continue; | ||
|
|
@@ -1444,7 +1480,8 @@ namespace ts.Completions { | |
| name, | ||
| symbol, | ||
| location, | ||
| contextToken); | ||
| contextToken, | ||
| formatContext); | ||
| if (importAdder) { | ||
| const changes = textChanges.ChangeTracker.with( | ||
| { host, formatContext, preferences }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1052,15 +1052,6 @@ namespace ts.textChanges { | |
| ? "" : options.suffix); | ||
| } | ||
|
|
||
| function getFormatCodeSettingsForWriting({ options }: formatting.FormatContext, sourceFile: SourceFile): FormatCodeSettings { | ||
|
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. moved to utilities |
||
| const shouldAutoDetectSemicolonPreference = !options.semicolons || options.semicolons === SemicolonPreference.Ignore; | ||
| const shouldRemoveSemicolons = options.semicolons === SemicolonPreference.Remove || shouldAutoDetectSemicolonPreference && !probablyUsesSemicolons(sourceFile); | ||
| return { | ||
| ...options, | ||
| semicolons: shouldRemoveSemicolons ? SemicolonPreference.Remove : SemicolonPreference.Ignore, | ||
| }; | ||
| } | ||
|
|
||
| /** Note: this may mutate `nodeIn`. */ | ||
| function getFormattedTextOfNode(nodeIn: Node, sourceFile: SourceFile, pos: number, { indentation, prefix, delta }: InsertNodeOptions, newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): string { | ||
| const { node, text } = getNonformattedText(nodeIn, sourceFile, newLineCharacter); | ||
|
|
@@ -1110,7 +1101,7 @@ namespace ts.textChanges { | |
| return skipTrivia(s, 0) === s.length; | ||
| } | ||
|
|
||
| function assignPositionsToNode(node: Node): Node { | ||
| export function assignPositionsToNode(node: Node): Node { | ||
| const visited = visitEachChild(node, assignPositionsToNode, nullTransformationContext, assignPositionsToNodeArray, assignPositionsToNode); | ||
| // create proxy node for non synthesized nodes | ||
| const newNode = nodeIsSynthesized(visited) ? visited : Object.create(visited) as Node; | ||
|
|
@@ -1131,7 +1122,7 @@ namespace ts.textChanges { | |
|
|
||
| interface TextChangesWriter extends EmitTextWriter, PrintHandlers {} | ||
|
|
||
| function createWriter(newLine: string): TextChangesWriter { | ||
| export function createWriter(newLine: string): TextChangesWriter { | ||
| let lastNonTriviaPosition = 0; | ||
|
|
||
| const writer = createTextWriter(newLine); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.