Support for inlayHints user preferences in language server

[sebthom]

Please support user configuration of the inlayHints support for the Dart language server similar to the JS/TS language server or the Haxe Language Servers .

This allows end-users to configure for which elements they want inlay hints to be displayed.

This seems currently not possible according https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/tool/lsp_spec/README.md

[bwilkerson]

@DanTup

[DanTup]

I like this idea. We'd need to decide exactly what settings to provide.. If we copy TypeScript (but exclude enumMemberValues because I don't think that's useful for Dart because you generally never reference enums using the index) we'd have:

• dart.inlayHints.returnTypes.enabled - whether to include return types
• dart.inlayHints.parameterNames.enabled:
  • none: don't show any parameter names
  • literal: only show parameter names for literal values (x: true) and not those using variables (myParam: myVariable)
  • all: show all parameter names (what we do today)
• dart.inlayHints.parameterNames.suppressWhenArgumentMatchesName - whether to suppress parameter name labels if the name matches the value (this only applies to all above and prevents myFoo: myFoo)
• dart.inlayHints.parameterTypes.enabled - whether to include inferred parameter types (eg. stringList.map((/*String*/ s))
• dart.inlayHints.propertyDeclarationTypes.enabled - whether to include inferred types for property declarations (/*String*/ get foo => '';)
• dart.inlayHints.variableTypes.enabled - whether to include inferred types for variable declarations (var /*int*/ a = 1)
• dart.inlayHints.variableTypes.suppressWhenTypeMatchesName - whether to suppress variable types when the type matches the name (var myFoo = MyFoo())

Two questions I have:

• Do we include class fields as "propertyDeclarations" or "variables"?
• Should we add a dart.inlayHints.propertyDeclarationTypes.suppressWhenTypeMatchesName because it's not clear to me why that shouldn't have the same option as variables (for example get myFoo => getMyFooInstance() seems like it would be also useful to suppress the duplicate MyFoo?

[bwilkerson]

dart.inlayHints.parameterNames.suppressWhenArgumentMatchesName

Should we just add a value between literal and all in the values for dart.inlayHints.parameterNames.enabled? Seems like it would be more discoverable, and would reduce the number of options for users to have to consider.

dart.inlayHints.variableTypes.suppressWhenTypeMatchesName

Do we want this to be a fuzzy match? For example, what if the type is String and the variable name is firstString? Or if the type is UniversallyUniqueIdentifier and the variable name is uuid?

Do we include class fields as "propertyDeclarations" or "variables"?

Semantically they're property declarations, so that would be my recommendation.

Should we add a dart.inlayHints.propertyDeclarationTypes.suppressWhenTypeMatchesName ...

Or just generalize the two into a single option that controls any location in which type information might be displayed? What about return types, such as a return type of Customer from a method named getCustomer? What I don't know is whether users want to control all such locations independently.

For that matter, should returnTypes be different than propertyDeclarationTypes? I always think of getters as having a return type, but maybe that's the wrong mental model.

[DanTup]

Should we just add a value between literal and all in the values for dart.inlayHints.parameterNames.enabled? Seems like it would be more discoverable, and would reduce the number of options for users to have to consider.

I wondered about that too (this is how TS does it and I was copying them for some consistency). I don't know if they had the suppressWhenTypeMatchesName booleans first, or they wanted to use this consistently for all values without making the others enums (I couldn't find the source for this or any discussion).

If we did this, what should it be called, and should we convert the other bools to be an enum for this? (none, onlyIfDifferentFromName(?), [literals], all)

Do we want this to be a fuzzy match? For example, what if the type is String and the variable name is firstString? Or if the type is UniversallyUniqueIdentifier and the variable name is uuid?
[...]
What about return types, such as a return type of Customer from a method named getCustomer?

My feeling is we should be fuzzy only for things like casing/underscores to avoid hiding any potentially important context?

Semantically they're property declarations, so that would be my recommendation.

I think for semantic tokens we map them as "instance variable declarations" or something (although references to them are properties because they are references to the getter/setter), but I'm not sure if the logic for that is sound. The reason I wondered about variables is that anything we put together in a category can only be togged together, and I wonder if a class field is something more likely to want to be toggled with variables (since they're of a similar form) compared to getter/setters. (Ofc, we could give them their own category and avoid choosing).

Or just generalize the two into a single option that controls any location in which type information might be displayed?

hmm, I think I prefer this to the idea above. I feel like if you've chosen not to include "duplicate" labels, probably you'd want that everywhere?

For that matter, should returnTypes be different than propertyDeclarationTypes? I always think of getters as having a return type, but maybe that's the wrong mental model.

Good question. I copied this from TS and didn't really question it, but I wonder if there's much value in separating properties at all (eg. getters could use returnTypes and setters parameterTypes).

Looking at the Haxe list posted above, that does also seem much simpler than TS with just a few bools:

inlayHints: {
			variableTypes: false,
			parameterNames: false,
			parameterTypes: false,
			functionReturnTypes: false,
			conditionals: false // (I'm not sure what this one does)
		},

So maybe something like:

• dart.inlayHints.returnTypes.enabled - (functions, getters)
• dart.inlayHints.parameterNames.enabled:
  none (or false?): don't show any parameter names
  literal: only show parameter names for literal values (x: true) and not those using variables (myParam: myVariable)
  all (or true?): show all parameter names (what we do today)
• dart.inlayHints.parameterTypes.enabled - include inferred parameter types (eg. stringList.map((/*String*/ s))
• dart.inlayHints.variableTypes.enabled - inferred types for variable + field declarations (var /*int*/ a = 1)
• dart.inlayHints.suppressWhenTypeMatchesName - exclude any where the textual part (ignoring underscores, casing) is the same as the adjacent identifier
  By keeping the enabled suffix for the option, if we wanted to support suppressWhenTypeMatchesName on an individual category in future, we could add that (and just use the parent level one as a default if unspecified).

[bwilkerson]

Do we want this to be a fuzzy match?

My feeling is we should be fuzzy only for things like casing/underscores to avoid hiding any potentially important context?

The reason this is hard is that it isn't possible (in general) to determine what context, if any, might be sufficient.

I assume we'd consider a name to match as long as the type name is somewhere in the name, such as a variable named highlightColor and a type named Color. What about the other direction, such as a variable named box whose type is BoundingBox? (I'm obviously concerned that it's going to be hard to get the right definition of "matching".)

I know that Erik has done some work to define 'obvious' for types. I wonder whether that might be another factor to take into account. Do we want to inline the type of a variable if the type is obvious based on the initialization expression? For example, consider var x = <String>[];. Or would that be another separate option, such as dart.inlayHints.suppressWhenTypeIsObvious? (I'm also a bit concerned about feature creep.)

To be clear, I'm not opposed to making the feature be configurable. I just don't want it to get out of hand. I like the second list of possible options more than the first.

[DanTup]

I assume we'd consider a name to match as long as the type name is somewhere in the name, such as a variable named highlightColor and a type named Color

For some reason I only thought about this the other way around. But I think you're right that the type is probably redundant in this case. I'm less sure about the other way (BoundingBox adds something that isn't visible in box). For ex.:

• BoudingBox box - the type name still adds context (what kind of box?)
• Color highlightColor - the type name doesn't add any context
• A bar - here the type name adds context, but the name does contain it (allowing for casing differences)

I wonder if we should split names into "segments" (where there are capitals, underscores, start/end of identifier, etc.) and then do only complete matches on those? (eg. highlightColor would be split into ["highlight", "color"] and then we'd match color)

I know that Erik has done some work to define 'obvious' for types.

I don't know much about this, but if we have an existing concept for this and we could reuse it here, I think that could be nice (and having its own option makes sense). Perhaps it's something to look at separately though - perhaps with a little configurability (as noted above) the feature would be more useful and we'd get better feedback on additional tweaks like that.

[bwilkerson]

I find the lack of a good definition of what we mean by "matching" to be concerning. I'd like to hold off on that particular preference until we have a more concrete definition of what it does, but the test of the preferences seem reasonable to me.

I know that Erik has done some work to define 'obvious' for types.

For more context, see #58773.

"Obvious" isn't the same for everyone, but the concept is, I think, well defined for the lints.

[sebthom]

I really don’t understand why this seemingly gets turned into a PhD dissertation. Couldn’t this just be done similarly to how the TypeScript language server handles it - just without the settings that don’t make sense for Dart?

Holding off for some “concrete” definitions basically feels like this will never be solved. Is there a dedicated, trackable task whose goal is to figure out these ominous definitions?

Sorry if this comes off as rude, that’s not my intention - I’m just genuinely confused why this has to be so complicated.