Skip to content

Support for Property getter/setter and event add/remove annotations #4074

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

Merged
buyaa-n merged 6 commits into from
Aug 28, 2020
Merged

Support for Property getter/setter and event add/remove annotations #4074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bcaa1de
Support for Property getter and setter annotation
buyaa-n Aug 27, 2020
f06c579
Addressing review comments
buyaa-n Aug 28, 2020
a96b05e
Apply more feedback
buyaa-n Aug 28, 2020
e99ab92
Update VB diagnostic use its own format
buyaa-n Aug 28, 2020
cfbdad0
Add Event accessor support
buyaa-n Aug 28, 2020
57db642
Apply feedback
buyaa-n Aug 28, 2020
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
137 changes: 123 additions & 14 deletions ...alyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatabilityAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
Expand Down Expand Up @@ -175,7 +176,7 @@ private void AnalyzeOperationBlock(

if (guardMethods.IsEmpty || !(context.OperationBlocks.GetControlFlowGraph(out var topmostBlock) is { } cfg))
{
ReportDiagnosticsForAll(platformSpecificOperations, context);
ReportDiagnosticsForAll(platformSpecificOperations, context, platformSpecificMembers);
return;
}

Expand All @@ -201,7 +202,7 @@ private void AnalyzeOperationBlock(
continue;
}

ReportDiagnostics(platformSpecificOperation, attributes, context);
ReportDiagnostics(platformSpecificOperation, attributes, context, platformSpecificMembers);
}
}
finally
Expand Down Expand Up @@ -431,15 +432,17 @@ static void RemoveOtherSupportsOnDifferentPlatforms(SmallDictionary<string, Plat
private static bool IsEmptyVersion(Version version) => version.Major == 0 && version.Minor == 0;

private static void ReportDiagnosticsForAll(PooledConcurrentDictionary<IOperation,
SmallDictionary<string, PlatformAttributes>> platformSpecificOperations, OperationBlockAnalysisContext context)
SmallDictionary<string, PlatformAttributes>> platformSpecificOperations, OperationBlockAnalysisContext context,
ConcurrentDictionary<ISymbol, SmallDictionary<string, PlatformAttributes>?> platformSpecificMembers)
{
foreach (var platformSpecificOperation in platformSpecificOperations)
{
ReportDiagnostics(platformSpecificOperation.Key, platformSpecificOperation.Value, context);
ReportDiagnostics(platformSpecificOperation.Key, platformSpecificOperation.Value, context, platformSpecificMembers);
}
}

private static void ReportDiagnostics(IOperation operation, SmallDictionary<string, PlatformAttributes> attributes, OperationBlockAnalysisContext context)
private static void ReportDiagnostics(IOperation operation, SmallDictionary<string, PlatformAttributes> attributes,
OperationBlockAnalysisContext context, ConcurrentDictionary<ISymbol, SmallDictionary<string, PlatformAttributes>?> platformSpecificMembers)
{
var symbol = operation is IObjectCreationOperation creation ? creation.Constructor.ContainingType : GetOperationSymbol(operation);

Expand All @@ -448,7 +451,20 @@ private static void ReportDiagnostics(IOperation operation, SmallDictionary<stri
return;
}

var operationName = symbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat);
if (symbol is IPropertySymbol property)
{
symbol = GetAccessorMethod(platformSpecificMembers, symbol, GetPropertyAccessors(property, operation));
}

if (symbol is IEventSymbol iEvent)
{
var accessor = GetEventAccessor(iEvent, operation);
if (accessor != null)
{
symbol = accessor;
}
}
var operationName = symbol.ToDisplayString(GetLanguageSpecificFormat(operation));

foreach (var platformName in attributes.Keys)
{
Expand Down Expand Up @@ -480,6 +496,22 @@ static void ReportSupportedDiagnostic(IOperation operation, OperationBlockAnalys
static void ReportUnsupportedDiagnostic(IOperation operation, OperationBlockAnalysisContext context, string name, string platformName, string? version = null) =>
context.ReportDiagnostic(version == null ? operation.CreateDiagnostic(UnsupportedOsRule, name, platformName) :
operation.CreateDiagnostic(UnsupportedOsVersionRule, name, platformName, version));

static SymbolDisplayFormat GetLanguageSpecificFormat(IOperation operation) =>
operation.Language == LanguageNames.CSharp ? SymbolDisplayFormat.CSharpShortErrorMessageFormat : SymbolDisplayFormat.VisualBasicShortErrorMessageFormat;

static ISymbol GetAccessorMethod(ConcurrentDictionary<ISymbol, SmallDictionary<string, PlatformAttributes>?> platformSpecificMembers, ISymbol symbol, IEnumerable<ISymbol> accessors)
{
foreach (var accessor in accessors)
{
if (accessor != null && platformSpecificMembers.TryGetValue(accessor, out var attribute) && attribute != null)
{
return accessor;
}
}

return symbol;
}
}

private static string? VersionToString(Version version) => IsEmptyVersion(version) ? null : version.ToString();
Expand All @@ -494,6 +526,43 @@ static void ReportUnsupportedDiagnostic(IOperation operation, OperationBlockAnal
_ => null,
};

private static IEnumerable<ISymbol> GetPropertyAccessors(IPropertySymbol property, IOperation operation)
{
var usageInfo = operation.GetValueUsageInfo(property.ContainingSymbol);

// not checking/using ValueUsageInfo.Reference related values as property cannot be used as ref or out parameter
// not using ValueUsageInfo.Name too, it only use name of the property
if (usageInfo == ValueUsageInfo.ReadWrite)
{
yield return property.GetMethod;
yield return property.SetMethod;
}
else if (usageInfo.IsWrittenTo())
{
yield return property.SetMethod;
}
else if (usageInfo.IsReadFrom())
{
yield return property.GetMethod;
}
else
{
yield return property;
}
}

private static ISymbol GetEventAccessor(IEventSymbol iEvent, IOperation operation)
{
if (operation.Parent is IEventAssignmentOperation eventAssignment)
{
if (eventAssignment.Adds)
return iEvent.AddMethod;
else
return iEvent.RemoveMethod;
}
return iEvent;
}

private static void AnalyzeOperation(IOperation operation, OperationAnalysisContext context,
PooledConcurrentDictionary<IOperation, SmallDictionary<string, PlatformAttributes>> platformSpecificOperations,
ConcurrentDictionary<ISymbol, SmallDictionary<string, PlatformAttributes>?> platformSpecificMembers, ImmutableArray<string> msBuildPlatforms)
Expand All @@ -505,20 +574,53 @@ private static void AnalyzeOperation(IOperation operation, OperationAnalysisCont
return;
}

if (TryGetOrCreatePlatformAttributes(symbol, platformSpecificMembers, out var operationAttributes))
if (symbol is IPropertySymbol property)
{
if (TryGetOrCreatePlatformAttributes(context.ContainingSymbol, platformSpecificMembers, out var callSiteAttributes))
foreach (var accessor in GetPropertyAccessors(property, operation))
{
if (IsNotSuppressedByCallSite(operationAttributes, callSiteAttributes, msBuildPlatforms, out var notSuppressedAttributes))
if (accessor != null)
{
platformSpecificOperations.TryAdd(operation, notSuppressedAttributes);
CheckOperationAttributes(operation, context, platformSpecificOperations, platformSpecificMembers, msBuildPlatforms, accessor);
}
}
}
else if (symbol is IEventSymbol iEvent)
{
var accessor = GetEventAccessor(iEvent, operation);

if (accessor != null)
{
CheckOperationAttributes(operation, context, platformSpecificOperations, platformSpecificMembers, msBuildPlatforms, accessor);
}
else
{
if (TryCopyAttributesNotSuppressedByMsBuild(operationAttributes, msBuildPlatforms, out var copiedAttributes))
CheckOperationAttributes(operation, context, platformSpecificOperations, platformSpecificMembers, msBuildPlatforms, iEvent);
}
}
else
{
CheckOperationAttributes(operation, context, platformSpecificOperations, platformSpecificMembers, msBuildPlatforms, symbol);
}

static void CheckOperationAttributes(IOperation operation, OperationAnalysisContext context, PooledConcurrentDictionary<IOperation,
SmallDictionary<string, PlatformAttributes>> platformSpecificOperations,
ConcurrentDictionary<ISymbol, SmallDictionary<string, PlatformAttributes>?> platformSpecificMembers, ImmutableArray<string> msBuildPlatforms, ISymbol symbol)
{
if (TryGetOrCreatePlatformAttributes(symbol, platformSpecificMembers, out var operationAttributes))
{
if (TryGetOrCreatePlatformAttributes(context.ContainingSymbol, platformSpecificMembers, out var callSiteAttributes))
{
if (IsNotSuppressedByCallSite(operationAttributes, callSiteAttributes, msBuildPlatforms, out var notSuppressedAttributes))
{
platformSpecificOperations.TryAdd(operation, notSuppressedAttributes);
}
}
else
{
platformSpecificOperations.TryAdd(operation, copiedAttributes);
if (TryCopyAttributesNotSuppressedByMsBuild(operationAttributes, msBuildPlatforms, out var copiedAttributes))
{
platformSpecificOperations.TryAdd(operation, copiedAttributes);
}
}
}
}
Expand Down Expand Up @@ -808,21 +910,28 @@ private static bool TryGetOrCreatePlatformAttributes(

AddPlatformAttributes(symbol.GetAttributes(), ref attributes);

if (symbol is IMethodSymbol method && method.IsAccessorMethod())
{
// Add attributes for the associated Property
AddPlatformAttributes(method.AssociatedSymbol.GetAttributes(), ref attributes);
}

attributes = platformSpecificMembers.GetOrAdd(symbol, attributes);
}

return attributes != null;

static bool AddPlatformAttributes(ImmutableArray<AttributeData> immediateAttributes, [NotNullWhen(true)] ref SmallDictionary<string, PlatformAttributes>? attributes)
{
bool added = false;
foreach (AttributeData attribute in immediateAttributes)
{
if (s_osPlatformAttributes.Contains(attribute.AttributeClass.Name))
{
TryAddValidAttribute(ref attributes, attribute);
added |= TryAddValidAttribute(ref attributes, attribute);
}
}
return attributes != null;
return added;
}
}

Expand Down
27 changes: 24 additions & 3 deletions ...NetCore.Analyzers/InteropServices/PlatformCompatabilityAnalyzerTests.GuardedCallsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,17 +1109,28 @@ public class Test
{
[UnsupportedOSPlatform(""Windows8.1"")]
public string RemovedProperty { get; set;}

public static bool WindowsOnlyPropertyGetter
{
[SupportedOSPlatform(""windows"")]
get { return true; }
set { }
}

public void M1()
{
if(OperatingSystemHelper.IsWindows() && !OperatingSystemHelper.IsWindowsVersionAtLeast(8, 0, 19222))
{
WindowsOnlyPropertyGetter = true;
var val = WindowsOnlyPropertyGetter;
RemovedProperty = ""Hello"";
string s = RemovedProperty;
M2(RemovedProperty);
}
else
{
WindowsOnlyPropertyGetter = true;
var val = [|WindowsOnlyPropertyGetter|];
[|RemovedProperty|] = ""Hello"";
string s = [|RemovedProperty|];
M2([|RemovedProperty|]);
Expand Down Expand Up @@ -1972,7 +1983,7 @@ End Sub
End Class
" + MockRuntimeApiSourceVb + MockAttributesVbSource;
await VerifyAnalyzerAsyncVb(vbSource, s_msBuildPlatforms,
VerifyCS.Diagnostic(PlatformCompatabilityAnalyzer.SupportedOsRule).WithLocation(10, 13).WithMessage("'Test.M2()' is unsupported on 'Windows'"));
VerifyCS.Diagnostic(PlatformCompatabilityAnalyzer.SupportedOsRule).WithLocation(10, 13).WithMessage("'Private Sub M2()' is unsupported on 'Windows'"));
}

[Fact]
Expand Down Expand Up @@ -2026,25 +2037,35 @@ void M2()
public async Task GuardedWith_DebugAssertAnalysisTest()
{
var source = @"
using System;
using System.Diagnostics;
using System.Runtime.Versioning;
using System;
using System.Runtime.InteropServices;

class Test
{
void M1()
{
Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows));
M3();

// Should still warn for Windows10.1.2.3
[|M2()|];

Debug.Assert(OperatingSystemHelper.IsOSPlatformVersionAtLeast(""Windows"", 10, 2));

M2();
M3();
}

[SupportedOSPlatform(""Windows10.1.2.3"")]
void M2()
{
}

[SupportedOSPlatform(""Windows"")]
void M3()
{
}
}" + MockAttributesCsSource + MockOperatingSystemApiSource;
await VerifyAnalyzerAsyncCs(source);

Expand Down
Loading