Add "obsolete inheritance" analyzer (#5858)

This allows us to make it obsolete to *inherit* from a class, and only that.

Intended so people stop inheriting UI controls for no good reason.

Fixes #5856
This commit is contained in:
Pieter-Jan Briers
2025-04-19 09:29:17 +02:00
committed by GitHub
parent 191d7ab81c
commit 72d893dec5
7 changed files with 200 additions and 0 deletions

View File

@@ -0,0 +1,86 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp.Testing;
using Microsoft.CodeAnalysis.Testing;
using NUnit.Framework;
using VerifyCS =
Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<Robust.Analyzers.ObsoleteInheritanceAnalyzer, Microsoft.CodeAnalysis.Testing.DefaultVerifier>;
namespace Robust.Analyzers.Tests;
/// <summary>
/// Analyzer that implements <c>[ObsoleteInheritance]</c> checking, to give obsoletion warnings for inheriting types
/// that should never have been virtual.
/// </summary>
[Parallelizable(ParallelScope.All | ParallelScope.Fixtures)]
[TestFixture]
public sealed class ObsoleteInheritanceAnalyzerTest
{
private static Task Verifier(string code, params DiagnosticResult[] expected)
{
var test = new CSharpAnalyzerTest<ObsoleteInheritanceAnalyzer, DefaultVerifier>()
{
TestState =
{
Sources = { code },
},
};
TestHelper.AddEmbeddedSources(
test.TestState,
"Robust.Shared.Analyzers.ObsoleteInheritanceAttribute.cs"
);
// ExpectedDiagnostics cannot be set, so we need to AddRange here...
test.TestState.ExpectedDiagnostics.AddRange(expected);
return test.RunAsync();
}
[Test]
public async Task TestBasic()
{
const string code = """
using Robust.Shared.Analyzers;
[ObsoleteInheritance]
public class Base;
public class NotAllowed : Base;
""";
await Verifier(code,
// /0/Test0.cs(6,14): warning RA0034: Type 'NotAllowed' inherits from 'Base', which has obsoleted inheriting from itself
VerifyCS.Diagnostic(ObsoleteInheritanceAnalyzer.Rule).WithSpan(6, 14, 6, 24).WithArguments("NotAllowed", "Base")
);
}
[Test]
public async Task TestMessage()
{
const string code = """
using Robust.Shared.Analyzers;
[ObsoleteInheritance("Sus")]
public class Base;
public class NotAllowed : Base;
""";
await Verifier(code,
// /0/Test0.cs(6,14): warning RA0034: Type 'NotAllowed' inherits from 'Base', which has obsoleted inheriting from itself: "Sus"
VerifyCS.Diagnostic(ObsoleteInheritanceAnalyzer.RuleWithMessage).WithSpan(6, 14, 6, 24).WithArguments("NotAllowed", "Base", "Sus")
);
}
[Test]
public async Task TestNormal()
{
const string code = """
public class Base;
public class AllowedAllowed : Base;
""";
await Verifier(code);
}
}

View File

@@ -14,6 +14,7 @@
<EmbeddedResource Include="..\Robust.Shared\Analyzers\PreferNonGenericVariantForAttribute.cs" LogicalName="Robust.Shared.Analyzers.PreferNonGenericVariantForAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\Analyzers\PreferOtherTypeAttribute.cs" LogicalName="Robust.Shared.Analyzers.PreferOtherTypeAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\Analyzers\ForbidLiteralAttribute.cs" LogicalName="Robust.Shared.Analyzers.ForbidLiteralAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\Analyzers\ObsoleteInheritanceAttribute.cs" LogicalName="Robust.Shared.Analyzers.ObsoleteInheritanceAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\IoC\DependencyAttribute.cs" LogicalName="Robust.Shared.IoC.DependencyAttribute.cs" LinkBase="Implementations" />
<EmbeddedResource Include="..\Robust.Shared\GameObjects\EventBusAttributes.cs" LogicalName="Robust.Shared.GameObjects.EventBusAttributes.cs" LinkBase="Implementations" />
</ItemGroup>

View File

@@ -0,0 +1,75 @@
#nullable enable
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Robust.Roslyn.Shared;
namespace Robust.Analyzers;
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class ObsoleteInheritanceAnalyzer : DiagnosticAnalyzer
{
private const string Attribute = "Robust.Shared.Analyzers.ObsoleteInheritanceAttribute";
public static readonly DiagnosticDescriptor Rule = new(
Diagnostics.IdObsoleteInheritance,
"Parent type has obsoleted inheritance",
"Type '{0}' inherits from '{1}', which has obsoleted inheriting from itself",
"Usage",
DiagnosticSeverity.Warning,
true);
public static readonly DiagnosticDescriptor RuleWithMessage = new(
Diagnostics.IdObsoleteInheritanceWithMessage,
"Parent type has obsoleted inheritance",
"Type '{0}' inherits from '{1}', which has obsoleted inheriting from itself: \"{2}\"",
"Usage",
DiagnosticSeverity.Warning,
true);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [Rule, RuleWithMessage];
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterSymbolAction(CheckClass, SymbolKind.NamedType);
}
private static void CheckClass(SymbolAnalysisContext context)
{
if (context.Symbol is not INamedTypeSymbol typeSymbol)
return;
if (typeSymbol.IsValueType || typeSymbol.BaseType is not { } baseType)
return;
if (!AttributeHelper.HasAttribute(baseType, Attribute, out var data))
return;
var location = context.Symbol.Locations[0];
if (GetMessageFromAttributeData(data) is { } message)
{
context.ReportDiagnostic(Diagnostic.Create(
RuleWithMessage,
location,
[typeSymbol.Name, baseType.Name, message]));
}
else
{
context.ReportDiagnostic(Diagnostic.Create(
Rule,
location,
[typeSymbol.Name, baseType.Name]));
}
}
private static string? GetMessageFromAttributeData(AttributeData data)
{
if (data.ConstructorArguments is not [var message, ..])
return null;
return message.Value as string;
}
}

View File

@@ -0,0 +1,7 @@
namespace Robust.Client.UserInterface;
internal static class UIConstants
{
public const string ObsoleteInheritanceMessage =
"Do not inherit from standard UI controls, compose via nesting instead";
}

View File

@@ -37,6 +37,8 @@ public static class Diagnostics
public const string IdPreferOtherType = "RA0031";
public const string IdDuplicateDependency = "RA0032";
public const string IdForbidLiteral = "RA0033";
public const string IdObsoleteInheritance = "RA0034";
public const string IdObsoleteInheritanceWithMessage = "RA0035";
public static SuppressionDescriptor MeansImplicitAssignment =>
new SuppressionDescriptor("RADC1000", "CS0649", "Marked as implicitly assigned.");

View File

@@ -0,0 +1,28 @@
using System;
namespace Robust.Shared.Analyzers;
/// <summary>
/// Indicates that the ability to <i>inherit</i> this type is obsolete, and attempting to do so should give a warning.
/// </summary>
/// <remarks>
/// This is useful to gracefully deal with types that should never have had <see cref="VirtualAttribute"/>.
/// </remarks>
/// <seealso cref="VirtualAttribute"/>
[AttributeUsage(AttributeTargets.Class, Inherited = false)]
public sealed class ObsoleteInheritanceAttribute : Attribute
{
/// <summary>
/// An optional message provided alongside this obsoletion.
/// </summary>
public string? Message { get; }
public ObsoleteInheritanceAttribute()
{
}
public ObsoleteInheritanceAttribute(string message)
{
Message = message;
}
}

View File

@@ -9,6 +9,7 @@ namespace Robust.Shared.Analyzers;
/// Robust uses analyzers to prevent accidental usage of non-sealed classes:
/// a class must be either marked [Virtual], abstract, or sealed.
/// </remarks>
/// <seealso cref="ObsoleteInheritanceAttribute"/>
[AttributeUsage(AttributeTargets.Class, Inherited = false)]
public sealed class VirtualAttribute : Attribute
{