Function hover ignores static keyword

[anthroid]

Bug type: Language Service
Version: 1.60.1 (Universal)
Commit: 83bd43bc519d15e50c4272c6cf5c1479df196a4d
Date: 2021-09-10T17:09:14.403Z
Electron: 13.1.8
Chrome: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Darwin arm64 20.5.0
C/C++ Extension Version: v1.6.0

Bug description

When hovering over a function call or a function definition, the static keyword is ignored and not displayed in the tooltip.

Steps to reproduce

1. Declare a static function inside a C translation unit.
2. Call the function from another location inside the same translation unit.
3. Hover the instance of the function call, note the static keyword is dropped.
4. Hover the function definition itself, the static keyword is dropped.

Expected behavior

The static keyword is important and should not be dropped in the hover tooltip. This can be misleading until you actually navigate to the function definition itself, where you see that the function is declared static.

Screenshots

[michelleangela]

@anthroid
Could you further clarify the importance of seeing the static keyboard in the context of the current translation unit?

[anthroid]

Assume you're working with a C API (a driver, library, or something that may have some exposed functions and some static functions), and your project is broken up into several translation units. You might have functions within the API that will autocomplete and be visible with Intellisense, making them appear to be functions that are usable, but then they don't compile if you try to call them. You check the function prototype in the hover tooltip and everything looks fine. Then you navigate to the source file itself (within the driver, library, etc) and see that the function is actually declared static, so you can't actually use it in your own code.

There are probably two angles to look at this:

1. Maybe Intellisense shouldn't be including static functions in C translation units if they're not callable from the current context, and therefore not offer these functions as autocomplete suggestions or provide hover tooltips. However, these features are helpful for a lot of reasons, so personally, I would opt for this to stay as-is (just don't drop keywords).
2. Keep things as-is, but don't strip keywords, making it clear when a function is declared with static, inline, or restrict.

[michelleangela]

Sorry to ask for more info, could you provide some sample code where this is occurring:

Maybe Intellisense shouldn't be including static functions in C translation units if they're not callable from the current context, and therefore not offer these functions as autocomplete suggestions or provide hover tooltips.

[anthroid]

Please see an example here:
https://github.com/anthroid/vscode-static-test

[michelleangela]

Thank you for the sample code, we now understand the scenario where this feature will be valuable.

This feature is not implemented yet and so this issue is set as "Feature Request".

[sean-mcmanus]

However, this wouldn't fix the fact that the static function still appears in auto-complete since IntelliSense isn't aware of linking issues such as functions that are never defined in a TU -- ideally, the static functions wouldn't be declared in the public headers.

For example, there could be other static functions that are defined, and the only way to tell would be some naming convention that allows you to know the function is from some external library.

[anthroid]

Hi Sean, thank you for your reply.

However, this wouldn't fix the fact that the static function still appears in auto-complete since IntelliSense isn't aware of linking issues such as functions that are never defined in a TU

Personally, I don't think this is the problem. I understand that it would be more 'technically correct' for any non-reachable functions to be excluded from auto-complete, but the purpose of opening the issue (per the title) was not to change the fact that a static function will show up in auto-complete in some cases (which itself, I think is harmless), it was to report that the declaration is wrong when it's displayed, because it drops keywords.

So the problem (as I see it) is that you have a declaration, and you expect that when you see that declaration elsewhere (tooltips, etc) that it will be accurate, but it's not. The hover and auto-complete tooltips are displaying something different than the actual declaration in the file. I don't know why these wouldn't just be left alone and displayed as-is. I can see dropping compiler-specific extensions for example, but these are core language keywords. The embedded documentation markup is all parsed out from the comments, and the whitespace is formatted for display in the tooltip, but for some reason we just throw away keywords between the last preceding comment and the return type?

ideally, the static functions wouldn't be declared in the public headers.

I agree. For instance, removing the declaration from the header in the linked example does actually cause the static function to no longer appear in auto-complete, but many times, all functions within a translation unit will be declared in a common header regardless of whether they are all public, and the user of the API has no control over that, so it would be helpful if the declaration was actually true to the file wherever it was shown.

For what it's worth, I've also run into this with C++, where important keywords like virtual are dropped:

Yet some aren't, like for example constexpr is actually displayed here:

And modifiers always seem to be displayed for variables, but not functions?

Maybe this could be something as simple as a user preference?

• Display storage-class specifiers in tooltips

Or better yet, since the default behavior should be the most predictable, opt-in to hide them?

• Hide storage-class specifiers in tooltips

In any case, I don't think this is a feature request, it seems like a bug by definition (unexpected, incorrect, or unintended behavior), especially given the inconsistencies. Adding a switch to enable/disable it would be a feature request, but I personally think VS Code should not be dropping parts of the function declaration (ever) to begin with, and giving the user the ability to choose their preferred behavior would just be a bonus.

[sean-mcmanus]

1. The distinction between bug, enhancement, and feature request isn’t particularly relevant in regards to getting whether it gets fixed/implemented or not. Our team just considers it a bug if it was something that was not intentional (i.e. not implemented yet). I changed it to "enhancement".
2. TypeScript has the “same” issue, most likely “by design” and VS Code would likely also call it a feature request to add it:
   
3. Other features of our product also don’t show ALL the modifiers, such as the outline view in which we don’t even show the return type (it’s “by design” to show only a subset of the info).

[sean-mcmanus]

The fix is available in https://github.com/microsoft/vscode-cpptools/releases/tag/1.8.0-insiders .