Skip to content

Commit

Permalink
Merge pull request #3882 from Evangelink/CA1812-vbnet
Browse files Browse the repository at this point in the history
Update CA1812 to not report on vbnet static-like holder type
  • Loading branch information
mavasani authored Aug 10, 2020
2 parents ccbb33e + 8e12e55 commit 0a95f9e
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines
/// even though the title of CA1053 is "Static holder types should not have constructors".
/// Like FxCop, this analyzer does emit a diagnostic when the type has a private default
/// constructor, even though the documentation of CA1053 says it should only trigger for public
/// or protected default constructor. Like FxCop, this analyzer does not emit a diagnostic when
/// or protected default constructor. Like FxCop, this analyzer does not emit a diagnostic when
/// class has a base class, however the diagnostic is emitted if class supports empty interface.
/// </para>
/// <para>
Expand Down Expand Up @@ -73,13 +73,18 @@ public override void Initialize(AnalysisContext context)
private static void AnalyzeSymbol(SymbolAnalysisContext context)
{
var symbol = (INamedTypeSymbol)context.Symbol;

if (!symbol.IsStatic &&
symbol.MatchesConfiguredVisibility(context.Options, Rule, context.Compilation, context.CancellationToken) &&
!symbol.IsAbstract &&
!IsSealedAndVisualBasic(symbol) &&
symbol.IsStaticHolderType() &&
!symbol.IsAbstract)
symbol.MatchesConfiguredVisibility(context.Options, Rule, context.Compilation, context.CancellationToken))
{
context.ReportDiagnostic(symbol.CreateDiagnostic(Rule, symbol.Name));
}

static bool IsSealedAndVisualBasic(INamedTypeSymbol symbol)
=> symbol.IsSealed && symbol.Language == LanguageNames.VisualBasic;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1511,6 +1511,87 @@ public void M()
}.RunAsync();
}

[Fact, WorkItem(1878, "https://github.com/dotnet/roslyn-analyzers/issues/1878")]
public async Task CA1812_VisualBasic_StaticLikeClass_NoDiagnostic()
{
await VerifyVB.VerifyAnalyzerAsync(@"
Imports System
Friend NotInheritable Class C1
Private Sub New()
End Sub
Public Shared Function GetSomething(o As Object) As String
Return o.ToString()
End Function
End Class
Public Class Helpers
Private NotInheritable Class C2
Private Sub New()
End Sub
Public Shared Function GetSomething(o As Object) As String
Return o.ToString()
End Function
End Class
End Class
Friend NotInheritable Class C3
Private Const SomeConstant As String = ""Value""
Private Shared f As Integer
Private Sub New()
End Sub
Public Shared Sub M()
End Sub
Public Shared Property P As Integer
Public Shared Event ThresholdReached As EventHandler
End Class
Friend Class C4
Private Sub New()
End Sub
Public Shared Function GetSomething(o As Object) As String
Return o.ToString()
End Function
End Class
Friend NotInheritable Class C5
Public Sub New()
End Sub
Public Shared Function GetSomething(o As Object) As String
Return o.ToString()
End Function
End Class");
}

[Fact, WorkItem(1878, "https://github.com/dotnet/roslyn-analyzers/issues/1878")]
public async Task CA1812_VisualBasic_NotStaticLikeClass_Diagnostic()
{
await VerifyVB.VerifyAnalyzerAsync(@"
Imports System
Friend NotInheritable Class [|C1|]
Private Sub New()
End Sub
Public Function GetSomething(o As Object) As String
Return o.ToString()
End Function
End Class");
}

private static DiagnosticResult GetCSharpResultAt(int line, int column, string className)
=> VerifyCS.Diagnostic()
.WithLocation(line, column)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public static IEnumerable<INamedTypeSymbol> GetBaseTypesAndThis(this INamedTypeS
/// <see cref="Stack{T}"/>.
/// </para>
/// <para>
/// Similarly, if <paramref name="parentType"/> is the interface <see cref="IList{T}"/>,
/// Similarly, if <paramref name="parentType"/> is the interface <see cref="IList{T}"/>,
/// then this method will return <see langword="true"/> for <c>List&gt;int></c>
/// or any other class that extends <see cref="IList{T}"/> or an class that implements it,
/// because <c>IList&gt;int></c> is constructed from <see cref="IList{T}"/>.
Expand Down Expand Up @@ -139,8 +139,10 @@ public static bool IsStaticHolderType(this INamedTypeSymbol symbol)
return false;
}

// Sealed objects are presumed to be non-static holder types
if (symbol.IsSealed)
// Sealed objects are presumed to be non-static holder types for C#.
// In VB.NET the type cannot be static and guidelines favor having a sealed (NotInheritable) type
// to act as static holder type.
if (symbol.IsSealed && symbol.Language == LanguageNames.CSharp)
{
return false;
}
Expand Down

0 comments on commit 0a95f9e

Please sign in to comment.