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

Do not report on awaitable-awaiter patterns #4639

Merged
merged 3 commits into from
Jan 5, 2021
Merged

Do not report on awaitable-awaiter patterns #4639

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9bea835
Small refactoring
Evangelink Dec 30, 2020
bc278da
CA1822: Do not report on awaitable-awaiter
Evangelink Dec 30, 2020
63c67a1
CA1024: Do not report on awaitable-awaiter
Evangelink Dec 30, 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
49 changes: 29 additions & 20 deletions ...etAnalyzers/Core/Microsoft.CodeQuality.Analyzers/QualityGuidelines/MarkMembersAsStatic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,7 @@ void OnOperationBlockStart(OperationBlockStartAnalysisContext blockStartContext)
}

// Don't run any other check for this method if it isn't a valid analysis context
if (!ShouldAnalyze(methodSymbol, wellKnownTypeProvider, skippedAttributes,
blockStartContext.Options, isWebProject, blockStartContext.CancellationToken))
{
return;
}

// Don't report methods which have a single throw statement
// with NotImplementedException or NotSupportedException
if (blockStartContext.IsMethodNotImplementedOrSupported())
if (!ShouldAnalyze(methodSymbol, wellKnownTypeProvider, skippedAttributes, isWebProject, blockStartContext))
{
return;
}
Expand Down Expand Up @@ -191,9 +183,10 @@ private static bool ShouldAnalyze(
IMethodSymbol methodSymbol,
WellKnownTypeProvider wellKnownTypeProvider,
ImmutableArray<INamedTypeSymbol> skippedAttributes,
AnalyzerOptions options,
bool isWebProject,
CancellationToken cancellationToken)
#pragma warning disable RS1012 // Start action has no registered actions
OperationBlockStartAnalysisContext blockStartContext)
#pragma warning restore RS1012 // Start action has no registered actions
{
// Modifiers that we don't care about
if (methodSymbol.IsStatic || methodSymbol.IsOverride || methodSymbol.IsVirtual ||
Expand All @@ -208,15 +201,31 @@ private static bool ShouldAnalyze(
return false;
}

// Do not analyze public APIs for web projects
// See https://github.com/dotnet/roslyn-analyzers/issues/3835 for details.
if (isWebProject && methodSymbol.IsExternallyVisible())
// Don't report methods which have a single throw statement
// with NotImplementedException or NotSupportedException
if (blockStartContext.IsMethodNotImplementedOrSupported())
{
return false;
}

// CA1000 says one shouldn't declare static members on generic types. So don't flag such cases.
if (methodSymbol.ContainingType.IsGenericType && methodSymbol.IsExternallyVisible())
if (methodSymbol.IsExternallyVisible())
Copy link
Member Author

Choose a reason for hiding this comment

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

Avoid doing this check twice.

{
// Do not analyze public APIs for web projects
// See https://github.com/dotnet/roslyn-analyzers/issues/3835 for details.
if (isWebProject)
{
return false;
}

// CA1000 says one shouldn't declare static members on generic types. So don't flag such cases.
if (methodSymbol.ContainingType.IsGenericType)
{
return false;
}
}

// We consider that auto-property have the intent to always be instance members so we want to workaround this issue.
if (methodSymbol.IsAutoPropertyAccessor())
{
return false;
}
Expand Down Expand Up @@ -248,14 +257,14 @@ private static bool ShouldAnalyze(
return false;
}

if (!options.MatchesConfiguredVisibility(Rule, methodSymbol, wellKnownTypeProvider.Compilation, cancellationToken,
defaultRequiredVisibility: SymbolVisibilityGroup.All))
var hasCorrectVisibility = blockStartContext.Options.MatchesConfiguredVisibility(Rule, methodSymbol, wellKnownTypeProvider.Compilation,
blockStartContext.CancellationToken, defaultRequiredVisibility: SymbolVisibilityGroup.All);
if (!hasCorrectVisibility)
{
return false;
}

// We consider that auto-property have the intent to always be instance members so we want to workaround this issue.
return !methodSymbol.IsAutoPropertyAccessor();
return true;
}

private static bool IsExplicitlyVisibleFromCom(IMethodSymbol methodSymbol, WellKnownTypeProvider wellKnownTypeProvider)
Expand Down