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

Add support for System.Diagnostics.CodeAnalysis.ExperimentalAttribute #68702

Merged
merged 8 commits into from
Jul 6, 2023
Merged

Add support for System.Diagnostics.CodeAnalysis.ExperimentalAttribute #68702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
58f7312
Add support for ExperimentalAttribute
jcouv Jun 18, 2023
eb8b887
Rename
jcouv Jun 20, 2023
4b36e8b
Add test
jcouv Jun 21, 2023
2608051
formatting
jcouv Jun 21, 2023
1b61429
Address feedback
jcouv Jun 22, 2023
1226862
Address feedback
jcouv Jul 5, 2023
38dbb3f
Address feedback
jcouv Jul 6, 2023
bddcfb3
Error format
jcouv Jul 6, 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
16 changes: 14 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ internal static ObsoleteDiagnosticKind GetObsoleteDiagnosticKind(Symbol symbol,
{
case ObsoleteAttributeKind.None:
return ObsoleteDiagnosticKind.NotObsolete;
case ObsoleteAttributeKind.WindowsExperimental:
case ObsoleteAttributeKind.Experimental:
return ObsoleteDiagnosticKind.Diagnostic;
case ObsoleteAttributeKind.Uninitialized:
Expand Down Expand Up @@ -154,12 +155,23 @@ static DiagnosticInfo createObsoleteDiagnostic(Symbol symbol, BinderFlags locati
return null;
}

if (data.Kind == ObsoleteAttributeKind.Experimental)
if (data.Kind == ObsoleteAttributeKind.WindowsExperimental)
{
Debug.Assert(data.Message == null);
Debug.Assert(!data.IsError);
// Provide an explicit format for fully-qualified type names.
Copy link
Member

@cston cston Jul 3, 2023 •
edited by jcouv
Loading

Choose a reason for hiding this comment

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

Provide an explicit format for fully-qualified type names.

Are we testing that type names are fully-qualified? (The format used in MessageProvider.cs is CSharpShortErrorMessageFormat which may drop the namespace.) #Resolved

return new CSDiagnosticInfo(ErrorCode.WRN_Experimental, new FormattedSymbol(symbol, SymbolDisplayFormat.CSharpErrorMessageFormat));
return new CSDiagnosticInfo(ErrorCode.WRN_Experimental,
new FormattedSymbol(symbol, SymbolDisplayFormat.CSharpErrorMessageFormat));
}

if (data.Kind == ObsoleteAttributeKind.Experimental)
{
Debug.Assert(data.Message is null);
Debug.Assert(!data.IsError);

// Provide an explicit format for fully-qualified type names.
return new CustomObsoleteDiagnosticInfo(MessageProvider.Instance, (int)ErrorCode.WRN_Experimental, data,
new FormattedSymbol(symbol, SymbolDisplayFormat.CSharpErrorMessageFormat));
}

// Issue a specialized diagnostic for add methods of collection initializers
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Symbols/Symbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1320,6 +1320,7 @@ internal ThreeState ObsoleteState
switch (ObsoleteKind)
{
case ObsoleteAttributeKind.None:
case ObsoleteAttributeKind.WindowsExperimental:
case ObsoleteAttributeKind.Experimental:
return ThreeState.False;
case ObsoleteAttributeKind.Uninitialized:
Expand Down
4 changes: 4 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Symbol_Attributes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ internal static bool EarlyDecodeDeprecatedOrExperimentalOrObsoleteAttribute(
{
kind = ObsoleteAttributeKind.Deprecated;
}
else if (CSharpAttributeData.IsTargetEarlyAttribute(type, syntax, AttributeDescription.WindowsExperimentalAttribute))
{
kind = ObsoleteAttributeKind.WindowsExperimental;
}
else if (CSharpAttributeData.IsTargetEarlyAttribute(type, syntax, AttributeDescription.ExperimentalAttribute))
{
kind = ObsoleteAttributeKind.Experimental;
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_Experimental.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace Microsoft.CodeAnalysis.CSharp.UnitTests
{
public class AttributeTests_Experimental : CSharpTestBase
public class AttributeTests_WindowsExperimental : CSharpTestBase
Copy link
Member

Choose a reason for hiding this comment

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

Also rename the file?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was intentionally not planning to rename the file. That unfortunately degrades the history and doesn't seem critical for this case.

Copy link
Member

Choose a reason for hiding this comment

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

History is only degraded when the changes to the file exceed the threshold of similarity. This one-line change would be detected by Git as a rename and features like blame would not be impacted.

Copy link
Member

Choose a reason for hiding this comment

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

Note that the rename can be implemented in a separate commit to ensure Git tracks the rename correctly. The squash-merge anti-feature works hard to undermine core Git functionality, but again this file would not exceed the standard similarity threshold.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have an example where blame on GitHub shows the proper history after a file rename?
I've seen git show a rename locally, but never saw blame work.

{
private const string DeprecatedAttributeSource =
@"using System;
Expand Down
Loading