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.

Sign up for GitHub

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

Jump to bottom

Display standalone parameter names by default #67857

Merged
merged 7 commits into from
May 12, 2023
Merged

Display standalone parameter names by default #67857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fc572f9
Add a test
jjonescz Apr 18, 2023
173c26a
Display standalone parameter names by default
jjonescz Apr 18, 2023
2fc98dd
Document breaking change
jjonescz Apr 18, 2023
5e7ae18
Update tests
jjonescz Apr 18, 2023
b985376
Merge branch 'main' into 67464-StandaloneParameterName-ByDefault
jjonescz Apr 20, 2023
8bb51cd
Merge branch 'main' into 67464-StandaloneParameterName-ByDefault
jjonescz May 11, 2023
f7d0f66
Merge branch 'main' into 67464-StandaloneParameterName-ByDefault
jjonescz May 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion docs/Breaking API Changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,16 @@ All property setters now throw an exception.
The `Workspace` and `DocumentId` parameters of `TextLoader.LoadTextAndVersionAsync(Workspace, DocumentId, CancellationToken)` are deprecated.
The method now receives `null` `Workspace` and `DocumentId`.

# Unreleased
# Version 4.5.0

`SymbolDisplayFormat.CSharpErrorMessageFormat` and `CSharpShortErrorMessageFormat` now include parameter names by default if used on a standalone `IParameterSymbol`.
For example, parameter `p` in `void M(ref int p)` was previously formatted as `"ref int"` and now it is formatted as `"ref int p"`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, we called these 'Display' apis intentionally so that we coudl change them without it being considered a breaking change. I'm fine documenting this though, just wanted to reiterate our position that things that exist for 'display purposes' absolutely can and will change depending on what we think is best for displaying them :)


# Version 4.7.0

### `SymbolDisplayFormat` includes parameter name when invoked on `IParameterSymbol`

All `SymbolDisplayFormat`s (predefined and user-created) now include parameter names by default if used on a standalone `IParameterSymbol` for consistency with predefined formats (see the breaking change for version 4.5.0 above).

### Changed `IncrementalStepRunReason` when a modified input produced a new output
Copy link
Contributor

@AlekseyTs AlekseyTs May 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this section is completely unrelated to the goal of this PR #Closed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this section is completely unrelated to the goal of this PR

I guess it came from the parent branch with the merge.

Copy link
Contributor Author

@jjonescz jjonescz May 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this PR hasn't touched that section.


Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ public override void VisitParameter(IParameterSymbol symbol)

var includeType = format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeType);
var includeName = symbol.Name.Length != 0 && (format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeName) ||
(format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.IncludeParameterNameIfStandalone) && builder.Count == 0));
(!format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.ExcludeParameterNameIfStandalone) && builder.Count == 0));
var includeBrackets = format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeOptionalBrackets);
var includeDefaultValue = format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeDefaultValue) &&
format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeName) &&
Expand Down
67 changes: 67 additions & 0 deletions src/Compilers/CSharp/Test/Symbol/SymbolDisplay/SymbolDisplayTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8222,6 +8222,73 @@ void M(string s!!)
SymbolDisplayPartKind.Punctuation);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67464")]
public void Parameter_Standalone()
{
var source = """
class C
{
void M(ref int p) { }
}
""";
var comp = CreateCompilation(source);
var methodSymbol = comp.GetMember<MethodSymbol>("C.M").GetPublicSymbol();
var parameterSymbol = methodSymbol.Parameters.Single();

var format = s_memberSignatureDisplayFormat.RemoveParameterOptions(SymbolDisplayParameterOptions.IncludeName);

Verify(methodSymbol.ToDisplayParts(format), "void C.M(ref int)",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ClassName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.MethodName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Punctuation);

Verify(methodSymbol.ToDisplayParts(SymbolDisplayFormat.CSharpErrorMessageFormat), "C.M(ref int)",
SymbolDisplayPartKind.ClassName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.MethodName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Punctuation);

Verify(methodSymbol.ToDisplayParts(SymbolDisplayFormat.CSharpErrorMessageNoParameterNamesFormat), "C.M(ref int)",
SymbolDisplayPartKind.ClassName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.MethodName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Punctuation);

Verify(parameterSymbol.ToDisplayParts(format), "ref int p",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ParameterName);

Verify(parameterSymbol.ToDisplayParts(SymbolDisplayFormat.CSharpErrorMessageFormat), "ref int p",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ParameterName);

Verify(parameterSymbol.ToDisplayParts(SymbolDisplayFormat.CSharpErrorMessageNoParameterNamesFormat), "ref int",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword);
}

[Fact]
public void TestRequiredProperty()
{
Expand Down
5 changes: 2 additions & 3 deletions src/Compilers/Core/Portable/SymbolDisplay/SymbolDisplayCompilerInternalOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,9 @@ internal enum SymbolDisplayCompilerInternalOptions
IncludeContainingFileForFileTypes = 1 << 8,

/// <summary>
/// Equivalent to <see cref="SymbolDisplayParameterOptions.IncludeName"/>
/// but only if the parameter is displayed on its own
/// Does not include parameter name if the parameter is displayed on its own
/// (i.e., not as part of a method, delegate, or indexer).
/// </summary>
IncludeParameterNameIfStandalone = 1 << 9,
ExcludeParameterNameIfStandalone = 1 << 9,
}
}
8 changes: 3 additions & 5 deletions src/Compilers/Core/Portable/SymbolDisplay/SymbolDisplayFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@ public class SymbolDisplayFormat
SymbolDisplayMiscellaneousOptions.UseSpecialTypes |
SymbolDisplayMiscellaneousOptions.UseAsterisksInMultiDimensionalArrays |
SymbolDisplayMiscellaneousOptions.UseErrorTypeSymbolName |
SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier,
compilerInternalOptions: SymbolDisplayCompilerInternalOptions.IncludeParameterNameIfStandalone);
SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier);

internal static SymbolDisplayFormat CSharpErrorMessageNoParameterNamesFormat { get; } = CSharpErrorMessageFormat
.RemoveCompilerInternalOptions(SymbolDisplayCompilerInternalOptions.IncludeParameterNameIfStandalone);
.AddCompilerInternalOptions(SymbolDisplayCompilerInternalOptions.ExcludeParameterNameIfStandalone);

/// <summary>
/// Formats a symbol description as in a C# compiler short error message.
Expand All @@ -59,8 +58,7 @@ public class SymbolDisplayFormat
SymbolDisplayMiscellaneousOptions.UseSpecialTypes |
SymbolDisplayMiscellaneousOptions.UseAsterisksInMultiDimensionalArrays |
SymbolDisplayMiscellaneousOptions.UseErrorTypeSymbolName |
SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier,
compilerInternalOptions: SymbolDisplayCompilerInternalOptions.IncludeParameterNameIfStandalone);
SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier);

/// <summary>
/// Formats a symbol description as in a Visual Basic compiler error message.
Expand Down
Loading
Loading