Skip to content

Conversation

@orta
Copy link
Contributor

@orta orta commented Jul 3, 2019

Fixes #30536 - handles both the case where you start writing on a newline directly, and when you are continuing to write inside an identifier.

Signed-off-by: Andrew Branch <andrew.branch@microsoft.com>
if (!isFromObjectTypeDeclaration(contextToken)) return undefined;
if (!isFromObjectTypeDeclaration(contextToken)) {
// class c extends React.Component { a: () => 1\n| }
if (getLineAndCharacterOfPosition(sourceFile, contextToken.getEnd()).line !== getLineAndCharacterOfPosition(sourceFile, position).line && isObjectTypeDeclaration(location)) {
Copy link
Member

Choose a reason for hiding this comment

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

Adding the isObjectTypeDeclaration(location) miiiight have been overly conservative, but I suggested it to err on the side of caution—the smallest reasonable net I could think of that would catch the repros provided.

if (!isFromObjectTypeDeclaration(contextToken)) return undefined;
if (!isFromObjectTypeDeclaration(contextToken)) {
// class c extends React.Component { a: () => 1\n| }
if (getLineAndCharacterOfPosition(sourceFile, contextToken.getEnd()).line !== getLineAndCharacterOfPosition(sourceFile, position).line && isObjectTypeDeclaration(location)) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you please reverse the order of checks here (that is checking isObjectTypeDeclaration(location) before line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switching the if ordering changes the behavior from what it previously was, and starts to trigger behavior which I think we don't want.

This fourslash test fails:

/// <reference path="fourslash.ts" />

////function /*a*/ ;
////function* /*b*/ ;
////interface I {
////    abstract baseMethod(): Iterable<number>;
////}
////class C implements I {
////    */*c*/ ;
////    public */*d*/
////}
////const o: I = {
////    */*e*/
////};
////1 * /*f*/

verify.completions(
    { marker: ["a", "b"], exact: undefined, isNewIdentifierLocation: true },
    { marker: ["c", "d"], exact: ["baseMethod"], isNewIdentifierLocation: true },
    { marker: "e", exact: ["baseMethod"] },
    { marker: "f", includes: [{ name: "Number", sortText: completion.SortText.GlobalsOrKeywords }] },
);

With

  1) fourslash tests
       tests/cases/fourslash/completionsGeneratorFunctions.ts
         fourslash test completionsGeneratorFunctions.ts runs correctly:
     Error: At f: Expected 'isNewIdentifierLocation' to be false, got true

Which is returning an empty array of completions

@orta
Copy link
Contributor Author

orta commented Jul 15, 2019

Alright I'm merging this 👍

@orta orta merged commit 2c26ac2 into microsoft:master Jul 15, 2019
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No member suggestions after class property definition without semicolon

5 participants