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

Add support for ExperimentalAttribute #6971

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Add support for ExperimentalAttribute #6971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 31 additions & 4 deletions src/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.Impl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,11 +589,33 @@ private static bool UsesOblivious(ISymbol symbol)

private ApiName GetApiName(ISymbol symbol)
{
var experimentName = getExperimentName(symbol);

return new ApiName(
getApiString(symbol, s_publicApiFormat),
getApiString(symbol, s_publicApiFormatWithNullability));
getApiString(_compilation, symbol, experimentName, s_publicApiFormat),
getApiString(_compilation, symbol, experimentName, s_publicApiFormatWithNullability));

static string? getExperimentName(ISymbol symbol)
{
for (var current = symbol; current is not null; current = current.ContainingSymbol)
{
foreach (var attribute in current.GetAttributes())
{
if (attribute.AttributeClass is { Name: "ExperimentalAttribute", ContainingSymbol: INamespaceSymbol { Name: nameof(System.Diagnostics.CodeAnalysis), ContainingNamespace: { Name: nameof(System.Diagnostics), ContainingNamespace: { Name: nameof(System), ContainingNamespace.IsGlobalNamespace: true } } } })
{
if (attribute.ConstructorArguments is not [{ Kind: TypedConstantKind.Primitive, Type.SpecialType: SpecialType.System_String, Value: string diagnosticId }])
return "???";

return diagnosticId;

}
}
}

return null;
}

string getApiString(ISymbol symbol, SymbolDisplayFormat format)
static string getApiString(Compilation compilation, ISymbol symbol, string? experimentName, SymbolDisplayFormat format)
{
string publicApiName = symbol.ToDisplayString(format);

Expand Down Expand Up @@ -625,11 +647,16 @@ string getApiString(ISymbol symbol, SymbolDisplayFormat format)
return string.Empty;
}

if (symbol.ContainingAssembly != null && !symbol.ContainingAssembly.Equals(_compilation.Assembly))
if (symbol.ContainingAssembly != null && !symbol.ContainingAssembly.Equals(compilation.Assembly))
{
publicApiName += $" (forwarded, contained in {symbol.ContainingAssembly.Name})";
}

if (experimentName != null)
{
publicApiName = "[" + experimentName + "]" + publicApiName;
}

return publicApiName;
}
}
Expand Down
76 changes: 76 additions & 0 deletions src/PublicApiAnalyzers/UnitTests/DeclarePublicAPIAnalyzerTestsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,11 @@ private async Task VerifyNet50CSharpAdditionalFileFixAsync(string source, string
await VerifyAdditionalFileFixAsync(LanguageNames.CSharp, source, shippedApiText, oldUnshippedApiText, newUnshippedApiText, ReferenceAssemblies.Net.Net50);
}

private async Task VerifyNet80CSharpAdditionalFileFixAsync(string source, string? shippedApiText, string? oldUnshippedApiText, string newUnshippedApiText)
{
await VerifyAdditionalFileFixAsync(LanguageNames.CSharp, source, shippedApiText, oldUnshippedApiText, newUnshippedApiText, AdditionalMetadataReferences.Net80);
}

private async Task VerifyAdditionalFileFixAsync(string language, string source, string? shippedApiText, string? oldUnshippedApiText, string newUnshippedApiText,
ReferenceAssemblies? referenceAssemblies = null)
{
Expand Down Expand Up @@ -3219,6 +3224,77 @@ virtual R.PrintMembers(System.Text.StringBuilder! builder) -> bool
await VerifyNet50CSharpAdditionalFileFixAsync(source, shippedText, unshippedText, fixedUnshippedText);
}

[Fact]
[WorkItem(6759, "https://github.com/dotnet/roslyn-analyzers/issues/6759")]
public async Task TestExperimentalApiAsync()
{
var source = $$"""
using System.Diagnostics.CodeAnalysis;

[Experimental("ID1")]
{{EnabledModifierCSharp}} class {|{{AddNewApiId}}:{|{{AddNewApiId}}:C|}|}
{
}
""";

var shippedText = @"";
var unshippedText = @"";
var fixedUnshippedText = @"[ID1]C
[ID1]C.C() -> void";

await VerifyNet80CSharpAdditionalFileFixAsync(source, shippedText, unshippedText, fixedUnshippedText);
}
Copy link
Member

Choose a reason for hiding this comment

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

Consider also testing removal of Experimental attribute (shipped file contains the experiment, but API no longer marked as experimental) and change of Experimental attribute (however I'm not sure it's a realistic/common scenario)

Copy link
Member Author

@sharwell sharwell Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Currently this will be treated as simply removing an old API and adding a new one. It's not the best, but it should work for now. We can work to improve this experience in the future based on how things go with Roslyn experiments.


[Theory]
[InlineData("")]
[InlineData("null")]
[InlineData("1")]
[InlineData("1, 2")]
[WorkItem(6759, "https://github.com/dotnet/roslyn-analyzers/issues/6759")]
public async Task TestExperimentalApiWithInvalidArgumentAsync(string invalidArgument)
{
var source = $$"""
using System.Diagnostics.CodeAnalysis;

[Experimental({{invalidArgument}})]
{{EnabledModifierCSharp}} class {|{{AddNewApiId}}:{|{{AddNewApiId}}:C|}|}
{
}
""";

var shippedText = @"";
var unshippedText = @"";
var fixedUnshippedText = @"[???]C
[???]C.C() -> void";

var test = new CSharpCodeFixTest<DeclarePublicApiAnalyzer, DeclarePublicApiFix, XUnitVerifier>()
{
ReferenceAssemblies = AdditionalMetadataReferences.Net80,
CompilerDiagnostics = CompilerDiagnostics.None,
TestState =
{
Sources = { source },
AdditionalFiles =
{
(ShippedFileName, shippedText),
(UnshippedFileName, unshippedText),
},
},
FixedState =
{
AdditionalFiles =
{
(ShippedFileName, shippedText),
(UnshippedFileName, fixedUnshippedText),
},
},
};

test.DisabledDiagnostics.AddRange(DisabledDiagnostics);

await test.RunAsync();
}

#endregion
}
}