Debug Protocol: Include display hints on evaluation results

[andrewcrawley]

The VS UI shows icons for locals, watches, etc to indicate the type of the evaluated value. We need to plumb this information through the protocol so debug adapters can indicate to the UI which icons and other visual states to apply.

Our proposal (implemented and verified in the VS Debug Adapter Host and VsDbg):

export interface EvaluateResponse extends Response {
    body: {
        // ...

        /** Optional attributes describing the type to help with display.*/
        evaluationAttribute?: EvaluationAttributes;

        // ...
    };
}

export interface Variable {
    // ...

    /** Optional attributes describing the type to help with display.*/
    evaluationAttribute?: EvaluationAttributes;

    // ...
}

export interface EvaluationAttributes {
    /** Evaluation of the expression failed. Allows additional information to be passed during an evaluation failure.*/
    isFailedEvaluation?: boolean;
    /** Indicates that the object is read only.*/
    isReadOnly?: boolean;
    /** Indicates that the object is a raw string.*/
    isRawString?: boolean;
    /** Indicates that the object can have an Object ID created for it.*/
    hasObjectId?: boolean;
    /** Indicates that the object has an Object ID associated with it.*/
    canHaveObjectId?: boolean;
    /** Indicates that the evaluation had side effects.*/
    hasSideEffects?: boolean;
    /** Indicates that the object is static.*/
    isStatic?: boolean;
    /** Indicates that the object is a constant.*/
    isConstant?: boolean;
    /** Type of the expression: property, method, etc.*/
    expressionType?: 'property' | 'method' | 'class' | 'data' | 'event' | 'baseClass' | 'innerClass' | 'interface' | 'mostDerivedClass';
    /** Access type of the expression: public, private, etc.*/
    accessLevel?: 'public' | 'private' | 'protected' | 'internal' | 'final';
}

We considered implementing this via the Kind field on Variable (and adding that field to EvaluateResult), e.g. "kind"="public;property;static;readonly", but we didn't like that for two reasons:

1. It allows you to specify "impossible" combinations, e.g. public;private
2. It requires a bunch of extra logic to generate and parse these strings, and we felt that it was more appropriate to use JSON.

Namedrops: @weinand @richardstanton @gregg-miskelly @pieandcakes @digeff

[gregg-miskelly]

LGTM

[weinand]

The kind attribute on type Variable was introduced to address exactly this issue (see #30).

Currently 'kind' is of type 'string' because we could not agree on a better type.
(Follow-up item #35 had been created to address this.)

I've reviewed @andrewcrawley's proposal and here is my feedback/proposal:

"open" enums

• since the existing property kind is not used by any frontend, I suggest that we replace it by a new property presentationHint. This name is already used in other places where we have a 'hint' that "helps with the display in the UI". The corresponding type will become VariablePresentationHint.
• since it is missing on type EvaluateResponse, we have to add it there too.
• The fields and value sets of the VariablePresentationHint (previously named EvaluationAttributes) type are more or less a 1:1 mapping of https://msdn.microsoft.com/en-us/library/bb145595.aspx and appear to have a strong C#/C++ bias. Since the DAP has to support a wide range of debuggers, I tried to "open" the definition of the hint type and make it less rigid. I suggest three properties: kind, attributes, and visibility. Instead of using "real" enums (which have a closed value set) for kind and visibility I'm using "open ended" enums, that is basically a string type with an "_enum" hint. For TypeScript the schema is transformed into a string property and the enum values are listed in the property comment (this pattern is already used in many places in the DAP). For the various boolean properties I propose to use an array of "open ended" enums.

The resulting typescript looks like:

/** Response to 'evaluate' request. */
export interface EvaluateResponse extends Response {
    body: {
        // ...
        /** Properties of a evaluate result that can be used to determine how to render the result in the UI. */
        presentationHint?: VariablePresentationHint;
        // ...
    };
}

export interface Variable {
    // ...
    /** Properties of a variable that can be used to determine how to render the variable in the UI. */
    presentationHint?: VariablePresentationHint;
    // ...
}

/** Optional properties of a variable that can be used to determine how to render the variable in the UI. */
export interface VariablePresentationHint {
    /** The kind of variable. Before introducing additional values, try to use the listed values.
        Values: 'property', 'method', 'class', 'data', 'event', 'baseClass', 'innerClass', 'interface', 'mostDerivedClass', etc.
    */
    kind?: string;
    /** Set of attributes represented as an array of strings. Before introducing additional values, try to use the listed values.
        Values: 
        'static': Indicates that the object is static.
        'constant': Indicates that the object is a constant.
        'readOnly': Indicates that the object is read only.
        'rawString': Indicates that the object is a raw string.
        'hasObjectId': Indicates that the object can have an Object ID created for it.
        'canHaveObjectId': Indicates that the object has an Object ID associated with it.
        'hasSideEffects': Indicates that the evaluation had side effects.
        etc.
    */
    attributes?: string[];
    /** Visibility of variable. Before introducing additional values, try to use the listed values.
        Values: 'public', 'private', 'protected', 'internal', 'final', etc.
    */
    visibility?: string;
}

and here is the JSON schema:

"VariablePresentationHint": {
    "type": "object",
    "description": "Optional properties of a variable that can be used to determine how to render the variable in the UI.",
    "properties": {
        "kind": {
            "description": "The kind of variable. Before introducing additional values, try to use the listed values.",
            "type": "string",
            "_enum": [ "property", "method", "class", "data", "event", "baseClass", "innerClass", "interface", "mostDerivedClass" ]
        },
        "attributes": {
            "description": "Set of attributes represented as an array of strings. Before introducing additional values, try to use the listed values.",
            "type": "array",
            "items": {
                "type": "string",
                "_enum": [ "static", "constant", "readOnly", "rawString", "hasObjectId", "canHaveObjectId", "hasSideEffects" ],
                "enumDescriptions": [
                    "Indicates that the object is static.",
                    "Indicates that the object is a constant.",
                    "Indicates that the object is read only.",
                    "Indicates that the object is a raw string.",
                    "Indicates that the object can have an Object ID created for it.",
                    "Indicates that the object has an Object ID associated with it.",
                    "Indicates that the evaluation had side effects."
                ]
            }
        },
        "visibility": {
            "description": "Visibility of variable. Before introducing additional values, try to use the listed values.",
            "type": "string",
            "_enum": [ "public", "private", "protected", "internal", "final" ]
        }
    }
}

@richardstanton @gregg-miskelly @pieandcakes @digeff @andrewcrawley what do you think?

[andrewcrawley]

I'm OK with merging the bool fields into an array like you propose.

I'm not sure what the objection is to using actual enums instead of strings? As you noted in #30, VSCode doesn't show icons, so it probably doesn't care about these values, and VS only assigns meaning to the listed values (which are not C++ / C# specific, btw - dozens of debuggers in VS use these values). If I'm writing a Java debug adapter and I specify a visibility of "package-private", how should a UI interpret this value? I think it's better to limit it to the set of values that have meaning to the UI, and adapter authors can pick the closest value, or file an issue to add a new enum value that we could properly support.

[weinand]

We do not want to hardcode a very specific value set into this specification.

If one (not yet known) debugger frontend decides to support a specific visibility value in its UI for their Simula-67 DA, we want to support that without requiring them to have to ask us for a protocol addition. So it would be ideal if we could directly express an "open" enum in the schema and all target languages would support that.

Please note that you are free to transform the JSON-schema into C# by using real enums! All the information is there. You don't have to use strings. But you need to fail gracefully when receiving an unknown enum value from the Simula-67 DA.