Skip to content

JS autocomplete doesn't work for object literal shorthands #41259

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

Closed
jurajmatus opened this issue Oct 24, 2020 · 7 comments · Fixed by #41539
Closed

JS autocomplete doesn't work for object literal shorthands #41259

jurajmatus opened this issue Oct 24, 2020 · 7 comments · Fixed by #41539
Assignees
Labels
Bug A bug in TypeScript Domain: Completion Lists The issue relates to showing completion lists in an editor Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue Help Wanted You can do this

Comments

@jurajmatus
Copy link

jurajmatus commented Oct 24, 2020

TS Template added by @mjbvz

TypeScript Version: 4.1.0-dev.20201026

Search Terms

  • suggest / suggestion
  • intellisense
  • object short hand

  • VSCode Version: 1.51.0-insider (7a3bdf4)
  • OS Version: Arch Linux

Steps to Reproduce:

  1. Suppose there are variables in the current context (be it local or global)
  2. Create an object literal and try to use a variable in a shorthand style
  3. No semantic autocompletion is offered, and textual autocomplete is only offered after 3 characters

GIF file to better demonstrate:
vscode

Does this issue occur when all extensions are disabled?: Yes

@mjbvz mjbvz transferred this issue from microsoft/vscode Oct 26, 2020
@mjbvz mjbvz removed their assignment Oct 26, 2020
@mjbvz mjbvz changed the title JS autocomplete doesn't work in object literals JS autocomplete doesn't work for object literal shorthands Oct 26, 2020
@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this Domain: Completion Lists The issue relates to showing completion lists in an editor labels Oct 27, 2020
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.2.0 milestone Oct 27, 2020
@orange4glace
Copy link
Contributor

orange4glace commented Nov 2, 2020

If you don't mind, I'd like to work with this issue. FYI @andrewbranch

@andrewbranch
Copy link
Member

Sounds good @orange4glace, let me know if you get stuck. You have about a month and a half before we need to get a fix in.

orange4glace added a commit to orange4glace/TypeScript that referenced this issue Nov 13, 2020
orange4glace added a commit to orange4glace/TypeScript that referenced this issue Nov 13, 2020
@orange4glace
Copy link
Contributor

orange4glace commented Nov 13, 2020

@andrewbranch Thanks for the reply! I have one question for this.
I'm planning to change the code like this.
Every tests are passed except completionListAtIdentifierDefinitionLocations_destructuring, which is,

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

//// var [x/*variable1*/

//// var [x, y/*variable2*/

//// var [./*variable3*/

//// var [x, ...z/*variable4*/

//// var {x/*variable5*/

//// var {x, y/*variable6*/

//// function func1({ a/*parameter1*/

//// function func2({ a, b/*parameter2*/
                         ~
                         Getting Global Autocompletion

verify.completions({ marker: test.markers(), exact: undefined });

As l understood, the parenthesis { of the last line following by the func2( is recognized as ObjectLiteral with any type rather than ObjectBinding since the line is getting following error,

var [x/*variable1*/

var [x, y/*variable2*/

var [./*variable3*/

var [x, ...z/*variable4*/

var {x/*variable5*/

var {x, y/*variable6*/

function func1({ a/*parameter1*/

function func2({ a, b/*parameter2*/



'}' expected.ts(1005)
41259.ts(15, 16): The parser expected to find a '}' to match the '{' token here.
41259.ts(13, 16): The parser expected to find a '}' to match the '{' token here.

I'm curious that,

  1. Is the test case valid one even though there's an syntax error? (Original PR for testcase : Less aggresive completion list #1767)
  2. If so, Fixing the code to satisfy both original issue and the testcase seems pretty hard as I think.

I'm very new to this community and anything that can be helped would be much appreciated!

@andrewbranch
Copy link
Member

We definitely want test cases that have syntax errors because they reflect half-written code as though a user is actively editing, and we want completions to work in those cases. That said, any particular assertion in a test like this might become wrong and need to be updated. What completion is showing up there?

@orange4glace
Copy link
Contributor

orange4glace commented Nov 14, 2020

With my modified version, it autocompletes global symbols at,

....
var {x, y/*variable6*/
function func1({ a/*parameter1*/
function func2({ a, b/*parameter2*/
                    ~
                    Global symbols Autocompletion

whereas with Typescript v4.0.5, it autocompletes nothing. (Playground)

Currently, the compiler parses the testcase code like this,

/* Statement #0 Starts (VariableStatement) */
var [x
/* Statement #1 Starts (VariableStatement) */
var [x, y
/* Statement #2 Starts (VariableStatement) */
var [.
/* Statement #3 Starts (VariableStatement) */
var [x, ...z
/* Statement #4 Starts (VariableStatement) */
var {x
var {x, y
function func1/* Statement #5 Starts (ExpressionStatement) */({ a
function func2({ a, b

The opening bracket { in the func2({ a, b is recognized as an ObjectLiteralExpression even though it looks like (or it should be) an ObjectBindingPattern, finally it autocompletes with global symbols.

@andrewbranch
Copy link
Member

I think that’s probably alright. Can you go ahead and open a draft PR so we can make a playground build?

@orange4glace
Copy link
Contributor

Sure! I've just opened it. :)

orange4glace added a commit to orange4glace/TypeScript that referenced this issue Nov 15, 2020
andrewbranch pushed a commit that referenced this issue Dec 22, 2020
…ds (#41539)

* fix: #41259

* fix: #41259

* fix: fourslash

* fix: remove nested if

* fix: change tc result for #41259

* fix: less restrictive shorthand completion rules

* fix: use typeMembers to find out whether properties are empty

* fix: typo

* fix: lint

* fix: exclude Object in completion

* fix: test

* fix: testcase tidy up

* fix: apply completions for unclosed literal and missing comma

* fix: ignore auto-imports

* fix: use exact to ensure the order of completions

* fix: use exact to ensure the order of completions

* fix: add new lines so it can easy to be distinguished
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Completion Lists The issue relates to showing completion lists in an editor Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fix Available A PR has been opened for this issue Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants