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

Non-copyable analysis for fields and auto-properties #4798

Merged
merged 1 commit into from
Feb 8, 2021
Merged

Non-copyable analysis for fields and auto-properties #4798

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
11 changes: 11 additions & 0 deletions src/Roslyn.Diagnostics.Analyzers/CSharp/CSharpDoNotCopyValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ public sealed class CSharpDoNotCopyValue : AbstractDoNotCopyValue
protected override NonCopyableWalker CreateWalker(OperationBlockAnalysisContext context, NonCopyableTypesCache cache)
=> new CSharpNonCopyableWalker(context, cache);

protected override NonCopyableSymbolWalker CreateSymbolWalker(SymbolAnalysisContext context, NonCopyableTypesCache cache)
=> new CSharpNonCopyableSymbolWalker(context, cache);

private sealed class CSharpNonCopyableWalker : NonCopyableWalker
{
public CSharpNonCopyableWalker(OperationBlockAnalysisContext context, NonCopyableTypesCache cache)
Expand Down Expand Up @@ -47,5 +50,13 @@ protected override bool CheckForEachGetEnumerator(IForEachLoopOperation operatio
return false;
}
}

private sealed class CSharpNonCopyableSymbolWalker : NonCopyableSymbolWalker
{
public CSharpNonCopyableSymbolWalker(SymbolAnalysisContext context, NonCopyableTypesCache cache)
: base(context, cache)
{
}
}
}
}
198 changes: 197 additions & 1 deletion src/Roslyn.Diagnostics.Analyzers/Core/AbstractDoNotCopyValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ public abstract class AbstractDoNotCopyValue : DiagnosticAnalyzer
private static readonly LocalizableString s_localizableNoUnboxingMessage = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.DoNotCopyValueNoUnboxingMessage), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));
private static readonly LocalizableString s_localizableNoUnboxingDescription = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.DoNotCopyValueNoUnboxingDescription), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));

private static readonly LocalizableString s_localizableNoFieldOfCopyableTypeMessage = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.DoNotCopyValueNoFieldOfCopyableTypeMessage), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));
private static readonly LocalizableString s_localizableNoFieldOfCopyableTypeDescription = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.DoNotCopyValueNoFieldOfCopyableTypeDescription), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));

private static readonly LocalizableString s_localizableNoAutoPropertyMessage = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.DoNotCopyValueNoAutoPropertyMessage), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));
private static readonly LocalizableString s_localizableNoAutoPropertyDescription = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.DoNotCopyValueNoAutoPropertyDescription), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));

internal static DiagnosticDescriptor Rule = new(
RoslynDiagnosticIds.DoNotCopyValueRuleId,
s_localizableTitle,
Expand Down Expand Up @@ -118,10 +124,34 @@ public abstract class AbstractDoNotCopyValue : DiagnosticAnalyzer
helpLinkUri: null,
customTags: WellKnownDiagnosticTags.Telemetry);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule, UnsupportedUseRule, NoBoxingRule, NoUnboxingRule);
internal static DiagnosticDescriptor NoFieldOfCopyableTypeRule = new(
RoslynDiagnosticIds.DoNotCopyValueRuleId,
s_localizableTitle,
s_localizableNoFieldOfCopyableTypeMessage,
DiagnosticCategory.RoslynDiagnosticsReliability,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: s_localizableNoFieldOfCopyableTypeDescription,
helpLinkUri: null,
customTags: WellKnownDiagnosticTags.Telemetry);

internal static DiagnosticDescriptor NoAutoPropertyRule = new(
RoslynDiagnosticIds.DoNotCopyValueRuleId,
s_localizableTitle,
s_localizableNoAutoPropertyMessage,
DiagnosticCategory.RoslynDiagnosticsReliability,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: s_localizableNoAutoPropertyDescription,
helpLinkUri: null,
customTags: WellKnownDiagnosticTags.Telemetry);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule, UnsupportedUseRule, NoBoxingRule, NoUnboxingRule, NoFieldOfCopyableTypeRule, NoAutoPropertyRule);

protected abstract NonCopyableWalker CreateWalker(OperationBlockAnalysisContext context, NonCopyableTypesCache cache);

protected abstract NonCopyableSymbolWalker CreateSymbolWalker(SymbolAnalysisContext context, NonCopyableTypesCache cache);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
Expand All @@ -131,6 +161,30 @@ public override void Initialize(AnalysisContext context)
{
var cache = new NonCopyableTypesCache(context.Compilation);
context.RegisterOperationBlockAction(context => AnalyzeOperationBlock(context, cache));
context.RegisterSymbolAction(
context => AnalyzeSymbol(context, cache),
//SymbolKind.Alias,
//SymbolKind.ArrayType,
//SymbolKind.Assembly,
//SymbolKind.Discard,
//SymbolKind.DynamicType,
//SymbolKind.ErrorType,
SymbolKind.Event,
SymbolKind.Field,
//SymbolKind.FunctionPointerType,
//SymbolKind.Label,
//SymbolKind.Local,
SymbolKind.Method,
SymbolKind.NamedType,
SymbolKind.Namespace,
//SymbolKind.NetModule,
SymbolKind.Parameter,
//SymbolKind.PointerType,
//SymbolKind.Preprocessing,
SymbolKind.Property
//SymbolKind.RangeVariable,
//SymbolKind.TypeParameter
);
});
}

Expand All @@ -143,6 +197,12 @@ private void AnalyzeOperationBlock(OperationBlockAnalysisContext context, NonCop
}
}

private void AnalyzeSymbol(SymbolAnalysisContext context, NonCopyableTypesCache cache)
{
var walker = CreateSymbolWalker(context, cache);
walker.Visit(context.Symbol);
}

private static VisitReleaser<T> TryAddForVisit<T>(HashSet<T> set, T? value, out bool added)
where T : class
{
Expand Down Expand Up @@ -178,6 +238,142 @@ public void Dispose()
}
}

protected abstract class NonCopyableSymbolWalker : SymbolVisitor
{
private readonly SymbolAnalysisContext _context;
//private readonly HashSet<ISymbol> _handledSymbols = new();

protected NonCopyableSymbolWalker(SymbolAnalysisContext context, NonCopyableTypesCache cache)
{
_context = context;
Cache = cache;
}

protected NonCopyableTypesCache Cache { get; }

public sealed override void Visit(ISymbol? symbol)
{
base.Visit(symbol);
}

public override void DefaultVisit(ISymbol symbol)
{
base.DefaultVisit(symbol);
}

public override void VisitAlias(IAliasSymbol symbol)
{
base.VisitAlias(symbol);
}
Copy link
Member

Choose a reason for hiding this comment

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

i don't quite get the purpose of the overrides. is it just to be explicit that this is the behavior you want? (if so, that's fine. just trying to understand).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this analyzer overrides everything.


public override void VisitArrayType(IArrayTypeSymbol symbol)
{
base.VisitArrayType(symbol);
}

public override void VisitAssembly(IAssemblySymbol symbol)
{
base.VisitAssembly(symbol);
}

public override void VisitDiscard(IDiscardSymbol symbol)
{
base.VisitDiscard(symbol);
}

public override void VisitDynamicType(IDynamicTypeSymbol symbol)
{
base.VisitDynamicType(symbol);
}

public override void VisitEvent(IEventSymbol symbol)
{
base.VisitEvent(symbol);
}

public override void VisitField(IFieldSymbol symbol)
{
// Fields of copyable value types must be copyable. Copying a value type makes a shallow copy of the
// fields, which implicitly copies any value type fields.
if (Cache.IsNonCopyableType(symbol.Type)
&& !Cache.IsNonCopyableType(symbol.ContainingType)
&& symbol.ContainingType.IsValueType)
{
_context.ReportDiagnostic(symbol.CreateDiagnostic(NoFieldOfCopyableTypeRule, symbol.Type, symbol));
}

base.VisitField(symbol);
}

public override void VisitFunctionPointerType(IFunctionPointerTypeSymbol symbol)
{
base.VisitFunctionPointerType(symbol);
}

public override void VisitLabel(ILabelSymbol symbol)
{
base.VisitLabel(symbol);
}

public override void VisitLocal(ILocalSymbol symbol)
{
base.VisitLocal(symbol);
}

public override void VisitMethod(IMethodSymbol symbol)
{
base.VisitMethod(symbol);
}

public override void VisitModule(IModuleSymbol symbol)
{
base.VisitModule(symbol);
}

public override void VisitNamedType(INamedTypeSymbol symbol)
{
base.VisitNamedType(symbol);
}

public override void VisitNamespace(INamespaceSymbol symbol)
{
base.VisitNamespace(symbol);
}

public override void VisitParameter(IParameterSymbol symbol)
{
base.VisitParameter(symbol);
}

public override void VisitPointerType(IPointerTypeSymbol symbol)
{
base.VisitPointerType(symbol);
}

public override void VisitProperty(IPropertySymbol symbol)
{
// Auto-properties cannot have non-copyable types. The getter always returns the backing field by value,
// which requires making a copy.
if (symbol.IsAutoProperty()
&& Cache.IsNonCopyableType(symbol.Type))
{
_context.ReportDiagnostic(symbol.CreateDiagnostic(NoAutoPropertyRule, symbol.Type, symbol));
}

base.VisitProperty(symbol);
}

public override void VisitRangeVariable(IRangeVariableSymbol symbol)
{
base.VisitRangeVariable(symbol);
}

public override void VisitTypeParameter(ITypeParameterSymbol symbol)
{
base.VisitTypeParameter(symbol);
}
}

protected abstract class NonCopyableWalker : OperationWalker
{
private readonly OperationBlockAnalysisContext _context;
Expand Down
12 changes: 12 additions & 0 deletions src/Roslyn.Diagnostics.Analyzers/Core/RoslynDiagnosticsAnalyzersResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,4 +393,16 @@
<data name="ApplyTraitToContainingType" xml:space="preserve">
<value>Apply trait to containing type</value>
</data>
<data name="DoNotCopyValueNoFieldOfCopyableTypeDescription" xml:space="preserve">
<value>A field with a non-copyable type cannot be a member of a copyable type. The containing type can be made non-copyable or converted to a reference type, or the field can be removed or converted to a copyable type.</value>
</data>
<data name="DoNotCopyValueNoFieldOfCopyableTypeMessage" xml:space="preserve">
<value>Copyable field '{1}' cannot have non-copyable type '{0}'</value>
</data>
<data name="DoNotCopyValueNoAutoPropertyDescription" xml:space="preserve">
<value>Auto-properties always copy values, so they cannot be declared with non-copyable types.</value>
</data>
<data name="DoNotCopyValueNoAutoPropertyMessage" xml:space="preserve">
<value>Auto-property '{1}' cannot have non-copyable type '{0}'</value>
</data>
</root>
20 changes: 20 additions & 0 deletions src/Roslyn.Diagnostics.Analyzers/Core/xlf/RoslynDiagnosticsAnalyzersResources.cs.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@
<target state="translated">Hodnota z odkazu se nedá přiřadit typu {0}, který se nedá kopírovat.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoAutoPropertyDescription">
<source>Auto-properties always copy values, so they cannot be declared with non-copyable types.</source>
<target state="new">Auto-properties always copy values, so they cannot be declared with non-copyable types.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoAutoPropertyMessage">
<source>Auto-property '{1}' cannot have non-copyable type '{0}'</source>
<target state="new">Auto-property '{1}' cannot have non-copyable type '{0}'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoBoxingDescription">
<source>Do not box non-copyable value types.</source>
<target state="translated">Nebalte nekopírovatelné typy hodnot.</target>
Expand All @@ -127,6 +137,16 @@
<target state="translated">Nebalte nekopírovatelný typ hodnoty {0}.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoFieldOfCopyableTypeDescription">
<source>A field with a non-copyable type cannot be a member of a copyable type. The containing type can be made non-copyable or converted to a reference type, or the field can be removed or converted to a copyable type.</source>
<target state="new">A field with a non-copyable type cannot be a member of a copyable type. The containing type can be made non-copyable or converted to a reference type, or the field can be removed or converted to a copyable type.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoFieldOfCopyableTypeMessage">
<source>Copyable field '{1}' cannot have non-copyable type '{0}'</source>
<target state="new">Copyable field '{1}' cannot have non-copyable type '{0}'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoReturnValueFromReferenceDescription">
<source>Cannot return a value from a reference to a non-copyable type.</source>
<target state="translated">Hodnota z odkazu se nedá vrátit typu, který se nedá kopírovat.</target>
Expand Down
20 changes: 20 additions & 0 deletions src/Roslyn.Diagnostics.Analyzers/Core/xlf/RoslynDiagnosticsAnalyzersResources.de.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@
<target state="translated">Ein Wert kann aus einem Verweis kann dem nicht kopierbaren Typ "{0}" nicht zugewiesen werden.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoAutoPropertyDescription">
<source>Auto-properties always copy values, so they cannot be declared with non-copyable types.</source>
<target state="new">Auto-properties always copy values, so they cannot be declared with non-copyable types.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoAutoPropertyMessage">
<source>Auto-property '{1}' cannot have non-copyable type '{0}'</source>
<target state="new">Auto-property '{1}' cannot have non-copyable type '{0}'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoBoxingDescription">
<source>Do not box non-copyable value types.</source>
<target state="translated">Für nicht kopierbare Werttypen darf kein Boxing ausgeführt werden.</target>
Expand All @@ -127,6 +137,16 @@
<target state="translated">Für den nicht kopierbaren Typ "{0}" darf kein Boxing ausgeführt werden.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoFieldOfCopyableTypeDescription">
<source>A field with a non-copyable type cannot be a member of a copyable type. The containing type can be made non-copyable or converted to a reference type, or the field can be removed or converted to a copyable type.</source>
<target state="new">A field with a non-copyable type cannot be a member of a copyable type. The containing type can be made non-copyable or converted to a reference type, or the field can be removed or converted to a copyable type.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoFieldOfCopyableTypeMessage">
<source>Copyable field '{1}' cannot have non-copyable type '{0}'</source>
<target state="new">Copyable field '{1}' cannot have non-copyable type '{0}'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoReturnValueFromReferenceDescription">
<source>Cannot return a value from a reference to a non-copyable type.</source>
<target state="translated">Ein Wert aus einem Verweis kann nicht an einen nicht kopierbaren Typ zurückgegeben werden.</target>
Expand Down
20 changes: 20 additions & 0 deletions src/Roslyn.Diagnostics.Analyzers/Core/xlf/RoslynDiagnosticsAnalyzersResources.es.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@
<target state="translated">No se puede asignar un valor de una referencia a un tipo "{0}" que no se puede copiar.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoAutoPropertyDescription">
<source>Auto-properties always copy values, so they cannot be declared with non-copyable types.</source>
<target state="new">Auto-properties always copy values, so they cannot be declared with non-copyable types.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoAutoPropertyMessage">
<source>Auto-property '{1}' cannot have non-copyable type '{0}'</source>
<target state="new">Auto-property '{1}' cannot have non-copyable type '{0}'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoBoxingDescription">
<source>Do not box non-copyable value types.</source>
<target state="translated">No aplique conversión boxing a los tipos de valores que no se pueden copiar.</target>
Expand All @@ -127,6 +137,16 @@
<target state="translated">No aplicar conversión boxing al tipo "{0}" que no se puede copiar</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoFieldOfCopyableTypeDescription">
<source>A field with a non-copyable type cannot be a member of a copyable type. The containing type can be made non-copyable or converted to a reference type, or the field can be removed or converted to a copyable type.</source>
<target state="new">A field with a non-copyable type cannot be a member of a copyable type. The containing type can be made non-copyable or converted to a reference type, or the field can be removed or converted to a copyable type.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoFieldOfCopyableTypeMessage">
<source>Copyable field '{1}' cannot have non-copyable type '{0}'</source>
<target state="new">Copyable field '{1}' cannot have non-copyable type '{0}'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCopyValueNoReturnValueFromReferenceDescription">
<source>Cannot return a value from a reference to a non-copyable type.</source>
<target state="translated">No se puede devolver un valor de una referencia a un tipo que no se puede copiar.</target>
Expand Down
Loading