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

[API proposal] Option for displaying standalone parameter names #67478

Closed
jjonescz opened this issue Mar 24, 2023 · 4 comments · Fixed by #67857
Closed

[API proposal] Option for displaying standalone parameter names #67478

jjonescz opened this issue Mar 24, 2023 · 4 comments · Fixed by #67857
Assignees
Labels
Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Milestone

Comments

@jjonescz
Copy link
Contributor

Background and Motivation

PR #65606 added an internal option for displaying standalone parameter names and this option is enabled in SymbolDisplayFormat.CSharpErrorMessageFormat. However, since it's internal, users cannot disable the option (reported in #67464) unless they create their own SymbolDisplayFormat from scratch. Here we propose to make the option public.

Proposed API

 namespace Microsoft.CodeAnalysis
 {
     /// <summary>
     /// Specifies how parameters are displayed in the description of a (member, property/indexer, or delegate) symbol.
     /// </summary>
     [Flags]
     public enum SymbolDisplayParameterOptions
     {
         /// <summary>
         /// Includes parameter names in symbol descriptions.
         /// </summary>
+        /// <seealso cref="IncludeNameIfStandalone" />
         IncludeName = 1 << 3,
         
+        /// <summary>
+        /// Includes parameter name in top-level parameter symbol description.
+        /// </summary>
+        /// <seealso cref="IncludeName" />
+        IncludeNameIfStandalone = 1 << 6,
     }
 }

Usage Examples

var formatBare = new SymbolDisplayFormat(
    memberOptions:
        SymbolDisplayMemberOptions.IncludeParameters |
        SymbolDisplayMemberOptions.IncludeContainingType,
    parameterOptions: SymbolDisplayParameterOptions.IncludeType);
var formatNested = formatBare.AddParameterOptions(SymbolDisplayParameterOptions.IncludeName);
var formatStandalone = formatBare.AddParameterOptions(SymbolDisplayParameterOptions.IncludeNameIfStandalone);
var formatBoth = formatBare.AddParameterOptions(
    SymbolDisplayParameterOptions.IncludeName |
    SymbolDisplayParameterOptions.IncludeNameIfStandalone);

var comp = CreateCompilation("class C { void M(int p) { } }");
var m = comp.GetTypeByMetadataName("C").GetMembers("M").OfType<IMethodSymbol>().Single();
IParameterSymbol p = m.Parameters.Single();

Assert.Equal("C.M(Int32)", m.ToDisplayString(formatBare));
Assert.Equal("C.M(Int32 p)", m.ToDisplayString(formatNested));
Assert.Equal("C.M(Int32)", m.ToDisplayString(formatStandalone));
Assert.Equal("C.M(Int32 p)", m.ToDisplayString(formatBoth));

Assert.Equal("Int32", p.ToDisplayString(formatBare));
Assert.Equal("Int32 p", p.ToDisplayString(formatNested));
Assert.Equal("Int32 p", p.ToDisplayString(formatStandalone));
Assert.Equal("Int32 p", p.ToDisplayString(formatBoth));

// using SymbolDisplayFormat.CSharpErrorMessageFormat:

// before
Assert.Equal("int p", p.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat));
// - we cannot force displaying only parameter types using just the format easily
// - we can special-case parameter symbols - won't work if we want to display modifiers:
Assert.Equal("int", (p is IParameterSymbol ? p.Type : p).ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat));
// - we can create custom format from scratch which won't have the internal option:
Assert.Equal("int", p.ToDisplayString(new SymbolDisplayFormat(
    globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.OmittedAsContaining,
    typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces,
    propertyStyle: SymbolDisplayPropertyStyle.NameOnly,
    genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters,
    memberOptions:
        SymbolDisplayMemberOptions.IncludeParameters |
        SymbolDisplayMemberOptions.IncludeContainingType |
        SymbolDisplayMemberOptions.IncludeExplicitInterface,
    parameterOptions:
        SymbolDisplayParameterOptions.IncludeParamsRefOut |
        SymbolDisplayParameterOptions.IncludeType,
    miscellaneousOptions:
        SymbolDisplayMiscellaneousOptions.EscapeKeywordIdentifiers |
        SymbolDisplayMiscellaneousOptions.UseSpecialTypes |
        SymbolDisplayMiscellaneousOptions.UseAsterisksInMultiDimensionalArrays |
        SymbolDisplayMiscellaneousOptions.UseErrorTypeSymbolName |
        SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier)));

// after
Assert.Equal("int p", p.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat));
Assert.Equal("int", p.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat.RemoveParameterOptions(SymbolDisplayParameterOptions.IncludeNameIfStandalone)));

Alternative Designs

  • Add the option to SymbolDisplayMiscellaneousOptions or elsewhere.
@jjonescz jjonescz added Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request labels Mar 24, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 24, 2023
@CyrusNajmabadi
Copy link
Member

What's the scenario where you do not want the parameter name when standing alone?

@jjonescz
Copy link
Contributor Author

What's the scenario where you do not want the parameter name when standing alone?

I think this should be configurable even if there is no scenario. new SymbolDisplayFormat today doesn't print standalone parameter names - making it a default would be a breaking change.

@CyrusNajmabadi
Copy link
Member

I'm ok with a break if there's no real use case. Do we support all the other symbols behaving differently wrt name of they're standalone or not?

@jcouv jcouv added this to the 17.7 milestone Mar 31, 2023
@jcouv jcouv removed the untriaged Issues and PRs which have not yet been triaged by a lead label Mar 31, 2023
@jjonescz
Copy link
Contributor Author

jjonescz commented Apr 18, 2023

Do we support all the other symbols behaving differently wrt name of they're standalone or not?

You're right, we don't support this for other symbols.

I'm ok with a break if there's no real use case.

Here are the use cases for standalone parameter displaying without name inside the compiler: CSharpErrorMessageNoParameterNamesFormat references
And here in IDE: 5e7ae18 (test change demonstrating where IDE displayed standalone parameter symbols without name)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants