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

Is it possible to add intellisense to dynamic object with something like JSDoc? #15974

Closed
Thaina opened this issue Dec 17, 2016 · 50 comments
Closed
Labels
Area-IDE IDE-IntelliSense Completion, Signature Help, Quick Info Resolution-By Design The behavior reported in the issue matches the current design Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Milestone

Comments

@Thaina
Copy link

Thaina commented Dec 17, 2016

I want to feel like working with javascript object when using dynamic type in C#. And jsdoc approach like salsa from typescript is very neat in my opinion

So I wish C# could be able to do something the same

Is it possible for roslyn to support this feature?

@foxbot
Copy link

foxbot commented Dec 19, 2016

Supporting low-effort libraries that return dynamic objects, what else could go wrong?

@Thaina
Copy link
Author

Thaina commented Dec 19, 2016

Well, I heard VisualStudio was support something alike, intellisense on ExpandoObject like this

image

But it not supported on roslyn. And it not covering my use case

@DustinCampbell
Copy link
Member

Note that C# IntelliSense in Visual Studio is part of Roslyn. So, without support in Roslyn, there's no hope for ExpandoObject IntelliSense.

@jveselka
Copy link

jveselka commented Dec 20, 2016

What I would do is probably something like this

public class MyDynamicBase : DynamicObject
    {
        private Dictionary<string, object> _properties = new Dictionary<string, object>();

        public override bool TryGetMember(GetMemberBinder binder, out object result)
        {
            return _properties.TryGetValue(binder.Name, out result);
        }

        public override bool TrySetMember(SetMemberBinder binder, object value)
        {
            if (GetType().GetProperty(binder.Name) != null)
            {
                //you want this to stay sane
                throw new InvalidOperationException("Setting member of incompatible type");
            }
            _properties[binder.Name] = value;
            return true;
        }

        public dynamic More => this;
    }

    public class MyDynamicObject : MyDynamicBase
    {
        public string MyFixedProperty { get; set; }
    }

All statically typed stuff goes to MyDynamicObject - Intellisense works as expected. I can set additional properties via myObject.More.Anything = anything (or by casting to dynamic if you wish) and when I pass it into dynamic API I know that myObject.MyFixedProperty and myObject.Anything are both accessible just fine.

@DustinCampbell
Copy link
Member

FWIW, there is an extensibility point for the completion list that you can use to add members to the list: CompletionProvider

@Thaina
Copy link
Author

Thaina commented Jan 18, 2017

I would like to propose one idea, using interface and generic attribute

Not sure it possible but would be something like this

interface IUser
{
    string UserID { get; set; }
    string Name { get; set; }
}

[DynamicProperty<IUser>]
dynamic obj = LoadUserFromDB();
obj.User :|[intellisense show UserID]|:

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

@DustinCampbell If I write my own custom CompletionProvider how can I register it to roslyn?

@DustinCampbell
Copy link
Member

@Thaina: You don't so much register it with Roslyn as you'd register it with a particular host (such as VS, OmniSharp, etc.), which can be different depending on the host. In VS, integrating a new CompletionProvider would require adding an [ExportCompletionProvider] attribute to it and creating a VS extension that includes it as a MEF component.

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

@DustinCampbell I use vscode and omnisharp. Could you please envisioned me how should I get start with?

@DustinCampbell
Copy link
Member

Today, there isn't a mechanism for adding anything to OmniSharp's MEF composition without recompiling. You'd need to specifically add your assembly into the set of host assemblies used by OmniSharp. That's set is constructed here. You can provide an IHostExportProvider like they do here

That said, if the feature turns out well, I would hope you'd eventually submit it as a PR to Roslyn. That way, everyone can enjoy it! 😄

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

Thanks for your guide @DustinCampbell

I really wish roslyn itself should be able to add plugin or modulate some parts such as completion and formatting

@DustinCampbell
Copy link
Member

How and if extensibility works is really a decision that should be made by the host. Roslyn takes the position that it shouldn't impose an extensibility pattern on an application that hosts Roslyn.

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

I'm not sure but I was skim through roslyn and omnisharp source code once and found that completion list was generated from roslyn, am I wrong?

At first I was thinking I could write some plugin on vscode if the list of completion item give me the declaration type of each item and I could get attribute of the field. But it seem omnisharp was proxy all those to roslyn and no additional data was sent to omnisharp, so omnisharp member said that I need to have roslyn add more feature

@DustinCampbell
Copy link
Member

Well, I'm both an OmniSharp member and have worked on Roslyn since the beginning. 😄 There are some changes that would need to be done to be able to extend OmniSharp for this purpose. Roslyn provides an API to retrieve that data to display from a completion list. By default, that API will aggregate together the results of a set of completion providers. OmniSharp uses this API today, but only for providing keywords to the completion list. Today, OmniSharp provides symbols to the completion list differently, though that will change in the future (OmniSharp/omnisharp-roslyn#78).

All of that said, this is all open source, so you can feel free to have a go at it.

@DustinCampbell
Copy link
Member

Not necessarily. It could be done in many ways, each with different levels of reach:

  • It could be coded specifically into OmniSharp's IntelliSense service.
  • It could be written as a CompletionProvider in a separate assembly and included in OmniSharp's MEF composition. Once Use new Roslyn Completion List API for IntelliSense OmniSharp/omnisharp-roslyn#78 is implemented in the future (I'll get back to that someday, I promise!), it would flow through the CompletionService into OmniSharp.
  • It could be written as a CompletionProvider and contributed to Roslyn as a PR. This would allow the feature to appear in OmniSharp, VS, and other Roslyn hosts.

That said, whether or not this is doable is unclear. Providing "dynamic" completion statically is super-hard.

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

@DustinCampbell My idea is not to analyze all of possible dynamic member statically. Just take the idea from typescript salsa for js that we could mark dynamic object with the known member we want it to be

In vscode we can write js like this

/** @type {{key:string,value:number}[]} */
var keyValuePairs = [];
keyValuePairs[0].va // show intellisense for value as number

So if we could mark dynamic with something

[Dynamic(typeof(IEnumerable<KeyValuePair<string,float>>))]
dynamic keyValuePairs = new Dictionary<string,float>();
keyValuePair.FirstOrDefault().Va // show intellisense for value as float

So I think it doable in the service that

  • I can get attribute of that dynamic variable
  • I can use reflection to get members of types that implemented on the project
  • I can inject those members into completion item list

Which I think roslyn would be a place to do. But would it possible on just @omnisharp-roslyn or just @omnisharp-vscode ?

@DustinCampbell
Copy link
Member

Your code example above is a bit confusing. Is keyValuePairs a field? If it's a local, how are you intending to fix an attribute to it? What happens when you continue dotting into members? Will you show extension methods for KeyValuePair<string,float>?

It would be more work to do this in omnisharp-vscode. Because omnisharp-vscode is written in TypeScript and runs on nodejs, you wouldn't be able to write this in C#, or use Roslyn or other managed libraries without other shenanigans. Adding it to omnisharp-roslyn would be the way to go.

@DustinCampbell
Copy link
Member

I hope somebody will build a cool tuple-based serialization library for simple data-only cases like this.

interface IUser
{
    string UserID { get; set; }
    string Name { get; set; }
}

dynamic obj = LoadUserFromDB();

I'd rather write like this:

(string usedId, string name) = LoadUserFromDB();

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

@DustinCampbell The problem that I wish it must be dynamic because loading object from DB or anywhere dynamic is. It not always contains just the parameter you think it exist

Again please understand this example

In the db there are json like this

{
    UserID: "TT",
    Name: "ABC",
    CustomData:"123",
}

Whenever you do ANYTHING that parse this json into static type in C#. You will lose the data you don't know about. And when you pass that object to another system. You destroy the data we load from DB

I see you all never get my point why I request dynamic intellisense. Please understand the problem first

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

To put it simply

In most NoSQL database we use today let we save document in json format

It require us to work with json object by loading whole document out from DB. Modified some field, and save it back by posting whole document back to database

But whenever you load any json object into static type. Any field that named difference from what you have will gone missing

The best way to handle this situation is using dictionary and wrap it as dynamic object. That what we use in almost all of json plugin. NewtonSoft is one popular example

But whenever that object is large. Most of the time it has some field that it always have. If it is data of user it always have id and name for example. And salsa typescript is the best solution to deal with this scenario. Please go use it once and you will understand

@eyalsk As I said. Please go use typescript or js salsa with nosql db and you will understand. you still trying to mess with this issue without understanding and that is what frustrate me. I cannot appreciate your intervention because you never ever try to understand the problem

dynamic object can change. But we always have something in mind that it would not or should not change even if it is dynamic object

We want it to be dynamic because it has something that can change. But something is not everything. We may want it to have 100 fields that could change but only 10 field that will be present or null. And that's all we need to deal with dynamic object in the world

@DustinCampbell
Copy link
Member

@Thaina: I do understand the scenario you're talking about, which is why I've asked for more detail. I've spent a lot of time thinking about IntelliSense for dynamic in the past.

To implement dynamic IntelliSense reliably (for some definition of "reliable" 😄), you must either use the runtime type of an object (which isn't possible to determine statically) or add features to the type system (a la TypeScript) to allow hints to be provided about what a type might be.

The proposal you're making is similar to the latter approach: adding hints via attributes. The problem is that the examples you've shown are a little too simplistic and confusing. Consider your own example:

[Dynamic(typeof(IEnumerable<KeyValuePair<string,float>>))]
dynamic keyValuePairs = new Dictionary<string,float>();
keyValuePair.FirstOrDefault().Va // show intellisense for value as float

First, the use of an attribute limits the feature so that it this will only work if keyValuePairs is a field (since attributes can't be attached to locals).

Second, the example falls apart as soon as it is passed to another method.

[Dynamic(typeof(IEnumerable<KeyValuePair<string,float>>))]
dynamic keyValuePairs = new Dictionary<string,float>();

void M2()
{
    M1(keyValuePairs);
}

void M1(dynamic d)
{
    d.FirstOrDefault().Va // show intellisense for value as float
}

You could achieve this, but it would take a significant amount of work to perform the flow analysis necessary to determine the type hint for d.

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

@DustinCampbell What I request from the start is exactly latter case, as I mention JSDoc of typescript

I don't think we could be ever to reliably generate intellisense on the fly. Instead, as programmer, I just want to control what it will be present to me by myself

The ability to control what to hint is all I need. In fact I think that's all we should need. To let compiler guess your dynamic is too lazy

But you right, sorry I just forget that attribute cannot put on local variable. So it must be comment or some other mechanism. But the point is just to add custom hint into the code for dynamic object. I just use attribute because it feel familiar

Also its fine that it cannot work out of its scope. For any new things came in scope you should and must apply attribute mark it. using typescript salsa is also like that

[Dynamic(typeof(IEnumerable<KeyValuePair<string,float>>))]
dynamic keyValuePairs = new Dictionary<string,float>();

void M2()
{
    M1(keyValuePairs);
}

void M1([Dynamic(typeof(IEnumerable<KeyValuePair<string,float>>))]dynamic d)
{
    d.FirstOrDefault().Va // show intellisense for value as float
}

or to make this possible with local var

/// <var type="IEnumerable<KeyValuePair<string,float>>" />
dynamic keyValuePairs = new Dictionary<string,float>();

void M2()
{
    M1(keyValuePairs);
}

/// <param name="d" type="IEnumerable<KeyValuePair<string,float>>" />
void M1(dynamic d)
{
    /// <var type="IEnumerable<KeyValuePair<string,float>>" />
    dynamic dict = new Dict();
    d.FirstOrDefault().Va // show intellisense for value as float
}

@DustinCampbell
Copy link
Member

To my eye, the annotations are already getting a bit out of control. I can see this appealing to a certain sort of developer, but it's probably not something that would necessarily appeal to everybody. As I mentioned, you can definitely do this if you want to invest the time and are OK with the limitations.

We've considered allowing custom CompletionProviders to be included in a NuGet package in the same way that a Roslyn analyzer can be today. To me, that would be the ideal way to deliver a feature like this. That way, a user could simply reference the NuGet package for a particular project where they want to use this enhanced completion behavior.

@Pilchie, what do you think?

@DustinCampbell
Copy link
Member

I'm not concerned who started the dispute, but I would like it to end. https://opensource.microsoft.com/codeofconduct/

@iam3yal
Copy link

iam3yal commented Feb 4, 2017

@Thaina It's okay, I will unsubscribe from all your discussions and won't join any of them in the future.

Have a nice day.

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

@eyalsk Wow. After my threads became uncleanable mess you still think I can have a nice day with?
I should ask you are you feel good now?

Good day sir

@svick
Copy link
Contributor

svick commented Feb 4, 2017

If what you want is to have something that is dynamic, but also has some static type information, then proposals that suggest exactly that might be of interest: #3012 and #5306.

@Thaina
Copy link
Author

Thaina commented Feb 4, 2017

@svick Thank you recommending that for I can subscribed to it

But it seem those thread has no movement so long. And I think it overkill to add language feature just for intellisense. All we need for dynamic is intellisense so it should not be syntax to compiled

If we need to extend compiler to support new syntax would it be harder to implement?
That being said, I like those syntax too if it possible to implement

@kzu
Copy link
Contributor

kzu commented Oct 1, 2018

So, to chime in super late @DustinCampbell, regarding your comment on allowing completion providers via nugets, is that something we support already? /cc @Pilchie

Thanks!

@Pilchie
Copy link
Member

Pilchie commented Oct 1, 2018

@dpoeschl
Copy link
Contributor

dpoeschl commented Oct 2, 2018

@kzu No, that ability does not exist today. The only way to do this currently is the mechanisms Dustin described earlier (VS: export as [ExportCompletionProvider]; Omnisharp: more complicated as Dustin explained although maybe that's changed and I'm unaware).

I don't see an active issue for this... If this is something you think you'd make use of, please file an issue for it in this repo. Thanks!

@kzu
Copy link
Contributor

kzu commented Oct 2, 2018

Done, thanks! #30270

@VBAndCs
Copy link

VBAndCs commented Mar 25, 2020

So, what is the status of this now?

@CyrusNajmabadi
Copy link
Member

@VBAndCs please stop pinging issues asking for status. If there has been no movement, then there as been no movement. If you would like to contribute toward improvements here reach out and let us know. We can let you know if we'd take PR contributions.

@jmarolf
Copy link
Contributor

jmarolf commented Mar 25, 2020

My understanding of this thread is that we would like to do something to make intellisense for dynamic work in more places than it does today. It is unclear what the correct design for that feature would be though as attributes has several drawbacks. No clear design for this feature would be one of the main reasons the status has not changed.

@CyrusNajmabadi
Copy link
Member

Note: i'm going to close this out. There is an existing mechanism to support this scenario. Namly, the public CompletionProvider api.

With that people can build completion that htey want for advanced scenarios like this. Specifically, the thread has stated numerous times that supporting something like a jsdoc approach like salsa from typescript would be sufficient.

Such an approach could absolutely be built externally using our public api using whatever mechanism the author deems fit. i.e. they could use attributes on data for this, or they could add special sigils in doc comments or regular comments to indicate this information.

I do not see anything additional that roslyn has to add here.

@CyrusNajmabadi CyrusNajmabadi added IDE-IntelliSense Completion, Signature Help, Quick Info Resolution-By Design The behavior reported in the issue matches the current design Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented and removed 0 - Backlog Feature Request labels Mar 25, 2020
@CyrusNajmabadi
Copy link
Member

Marking as fixed as we now have a public, 100% supported mechanism for 3rd parties to enhance completion lists with this sort of information.

@VBAndCs

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE IDE-IntelliSense Completion, Signature Help, Quick Info Resolution-By Design The behavior reported in the issue matches the current design Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Development

No branches or pull requests